DisplayController should deep compare mPerDisplayBounds
- In CHANGE_SUPPORTED_BOUNDS check, it uses Map.equals, but as the value is a primitive array WindowBounds[], equals check does not deep compare the arrays and may result in false negative
- One example is when fontScale changes, DisplayController re-create bounds from WindowManagerProxy, result in new WindowBounds[] created, and the shallow compare results in unexpected CHANGE_SUPPORTED_BOUNDS
Fix: 282736623
Test: DisplayControllerTest
Change-Id: I3897595c58559192b951ecfee7c9f62a07dafe1f
diff --git a/quickstep/src/com/android/quickstep/util/SystemWindowManagerProxy.java b/quickstep/src/com/android/quickstep/util/SystemWindowManagerProxy.java
index a34888f..c82cdb7 100644
--- a/quickstep/src/com/android/quickstep/util/SystemWindowManagerProxy.java
+++ b/quickstep/src/com/android/quickstep/util/SystemWindowManagerProxy.java
@@ -28,6 +28,7 @@
import com.android.launcher3.util.window.CachedDisplayInfo;
import com.android.launcher3.util.window.WindowManagerProxy;
+import java.util.List;
import java.util.Set;
/**
@@ -53,15 +54,15 @@
}
@Override
- public ArrayMap<CachedDisplayInfo, WindowBounds[]> estimateInternalDisplayBounds(
+ public ArrayMap<CachedDisplayInfo, List<WindowBounds>> estimateInternalDisplayBounds(
Context displayInfoContext) {
- ArrayMap<CachedDisplayInfo, WindowBounds[]> result = new ArrayMap<>();
+ ArrayMap<CachedDisplayInfo, List<WindowBounds>> result = new ArrayMap<>();
WindowManager windowManager = displayInfoContext.getSystemService(WindowManager.class);
Set<WindowMetrics> possibleMaximumWindowMetrics =
windowManager.getPossibleMaximumWindowMetrics(DEFAULT_DISPLAY);
for (WindowMetrics windowMetrics : possibleMaximumWindowMetrics) {
CachedDisplayInfo info = getDisplayInfo(windowMetrics, Surface.ROTATION_0);
- WindowBounds[] bounds = estimateWindowBounds(displayInfoContext, info);
+ List<WindowBounds> bounds = estimateWindowBounds(displayInfoContext, info);
result.put(info, bounds);
}
return result;
diff --git a/quickstep/tests/src/com/android/quickstep/OrientationTouchTransformerTest.java b/quickstep/tests/src/com/android/quickstep/OrientationTouchTransformerTest.java
index 9c240f0..298dd6c 100644
--- a/quickstep/tests/src/com/android/quickstep/OrientationTouchTransformerTest.java
+++ b/quickstep/tests/src/com/android/quickstep/OrientationTouchTransformerTest.java
@@ -53,6 +53,8 @@
import org.junit.runner.RunWith;
import org.mockito.MockitoAnnotations;
+import java.util.List;
+
@SmallTest
@RunWith(AndroidJUnit4.class)
public class OrientationTouchTransformerTest {
@@ -296,7 +298,7 @@
WindowManagerProxy wmProxy = mock(WindowManagerProxy.class);
doReturn(cachedDisplayInfo).when(wmProxy).getDisplayInfo(any());
doReturn(windowBounds).when(wmProxy).getRealBounds(any(), any());
- ArrayMap<CachedDisplayInfo, WindowBounds[]> internalDisplayBounds = new ArrayMap<>();
+ ArrayMap<CachedDisplayInfo, List<WindowBounds>> internalDisplayBounds = new ArrayMap<>();
doReturn(internalDisplayBounds).when(wmProxy).estimateInternalDisplayBounds(any());
return new DisplayController.Info(
getApplicationContext(), wmProxy, new ArrayMap<>());
diff --git a/quickstep/tests/src/com/android/quickstep/util/TaskViewSimulatorTest.java b/quickstep/tests/src/com/android/quickstep/util/TaskViewSimulatorTest.java
index 83602be..a54dc2d 100644
--- a/quickstep/tests/src/com/android/quickstep/util/TaskViewSimulatorTest.java
+++ b/quickstep/tests/src/com/android/quickstep/util/TaskViewSimulatorTest.java
@@ -50,6 +50,9 @@
import org.junit.Test;
import org.junit.runner.RunWith;
+import java.util.ArrayList;
+import java.util.List;
+
@SmallTest
@RunWith(AndroidJUnit4.class)
public class TaskViewSimulatorTest {
@@ -150,7 +153,7 @@
WindowBounds wm = new WindowBounds(
new Rect(0, 0, mDisplaySize.x, mDisplaySize.y),
mDisplayInsets);
- WindowBounds[] allBounds = new WindowBounds[4];
+ List<WindowBounds> allBounds = new ArrayList<>(4);
for (int i = 0; i < 4; i++) {
Rect boundsR = new Rect(wm.bounds);
Rect insetsR = new Rect(wm.insets);
@@ -158,7 +161,7 @@
RotationUtils.rotateRect(insetsR, RotationUtils.deltaRotation(rotation, i));
RotationUtils.rotateRect(boundsR, RotationUtils.deltaRotation(rotation, i));
boundsR.set(0, 0, Math.abs(boundsR.width()), Math.abs(boundsR.height()));
- allBounds[i] = new WindowBounds(boundsR, insetsR);
+ allBounds.add(new WindowBounds(boundsR, insetsR));
}
WindowManagerProxy wmProxy = mock(WindowManagerProxy.class);
@@ -166,7 +169,7 @@
doReturn(wm).when(wmProxy).getRealBounds(any(), any());
doReturn(NavigationMode.NO_BUTTON).when(wmProxy).getNavigationMode(any());
- ArrayMap<CachedDisplayInfo, WindowBounds[]> perDisplayBoundsCache =
+ ArrayMap<CachedDisplayInfo, List<WindowBounds>> perDisplayBoundsCache =
new ArrayMap<>();
perDisplayBoundsCache.put(cdi.normalize(), allBounds);
diff --git a/src/com/android/launcher3/util/DisplayController.java b/src/com/android/launcher3/util/DisplayController.java
index 776fe40..68ed78a 100644
--- a/src/com/android/launcher3/util/DisplayController.java
+++ b/src/com/android/launcher3/util/DisplayController.java
@@ -53,8 +53,8 @@
import java.io.PrintWriter;
import java.util.ArrayList;
-import java.util.Arrays;
import java.util.Collections;
+import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
@@ -105,7 +105,8 @@
private final LauncherPrefs mPrefs;
- private DisplayController(Context context) {
+ @VisibleForTesting
+ protected DisplayController(Context context) {
mContext = context;
mDM = context.getSystemService(DisplayManager.class);
mPrefs = LauncherPrefs.get(context);
@@ -323,7 +324,7 @@
// WindowBounds
public final WindowBounds realBounds;
public final Set<WindowBounds> supportedBounds = new ArraySet<>();
- private final ArrayMap<CachedDisplayInfo, WindowBounds[]> mPerDisplayBounds =
+ private final ArrayMap<CachedDisplayInfo, List<WindowBounds>> mPerDisplayBounds =
new ArrayMap<>();
public Info(Context displayInfoContext) {
@@ -334,7 +335,7 @@
// Used for testing
public Info(Context displayInfoContext,
WindowManagerProxy wmProxy,
- Map<CachedDisplayInfo, WindowBounds[]> perDisplayBoundsCache) {
+ Map<CachedDisplayInfo, List<WindowBounds>> perDisplayBoundsCache) {
CachedDisplayInfo displayInfo = wmProxy.getDisplayInfo(displayInfoContext);
normalizedDisplayInfo = displayInfo.normalize();
rotation = displayInfo.rotation;
@@ -348,7 +349,7 @@
navigationMode = wmProxy.getNavigationMode(displayInfoContext);
mPerDisplayBounds.putAll(perDisplayBoundsCache);
- WindowBounds[] cachedValue = mPerDisplayBounds.get(normalizedDisplayInfo);
+ List<WindowBounds> cachedValue = mPerDisplayBounds.get(normalizedDisplayInfo);
realBounds = wmProxy.getRealBounds(displayInfoContext, displayInfo);
if (cachedValue == null) {
@@ -366,22 +367,20 @@
if (cachedValue != null) {
// Verify that the real bounds are a match
- WindowBounds expectedBounds = cachedValue[displayInfo.rotation];
+ WindowBounds expectedBounds = cachedValue.get(displayInfo.rotation);
if (!realBounds.equals(expectedBounds)) {
- WindowBounds[] clone = new WindowBounds[4];
- System.arraycopy(cachedValue, 0, clone, 0, 4);
- clone[displayInfo.rotation] = realBounds;
+ List<WindowBounds> clone = new ArrayList<>(cachedValue);
+ clone.set(displayInfo.rotation, realBounds);
mPerDisplayBounds.put(normalizedDisplayInfo, clone);
}
}
- mPerDisplayBounds.values().forEach(
- windowBounds -> Collections.addAll(supportedBounds, windowBounds));
+ mPerDisplayBounds.values().forEach(supportedBounds::addAll);
if (DEBUG) {
Log.d(TAG, "displayInfo: " + displayInfo);
Log.d(TAG, "realBounds: " + realBounds);
Log.d(TAG, "normalizedDisplayInfo: " + normalizedDisplayInfo);
mPerDisplayBounds.forEach((key, value) -> Log.d(TAG,
- "perDisplayBounds - " + key + ": " + Arrays.deepToString(value)));
+ "perDisplayBounds - " + key + ": " + value));
}
}
@@ -438,7 +437,7 @@
pw.println(" navigationMode=" + info.navigationMode.name());
pw.println(" currentSize=" + info.currentSize);
info.mPerDisplayBounds.forEach((key, value) -> pw.println(
- " perDisplayBounds - " + key + ": " + Arrays.deepToString(value)));
+ " perDisplayBounds - " + key + ": " + value));
}
/**
diff --git a/src/com/android/launcher3/util/MainThreadInitializedObject.java b/src/com/android/launcher3/util/MainThreadInitializedObject.java
index 6a4e528..0899a22 100644
--- a/src/com/android/launcher3/util/MainThreadInitializedObject.java
+++ b/src/com/android/launcher3/util/MainThreadInitializedObject.java
@@ -131,7 +131,8 @@
* Find a cached object from mObjectMap if we have already created one. If not, generate
* an object using the provider.
*/
- private <T> T getObject(MainThreadInitializedObject<T> object, ObjectProvider<T> provider) {
+ protected <T> T getObject(MainThreadInitializedObject<T> object,
+ ObjectProvider<T> provider) {
synchronized (mDestroyLock) {
if (mDestroyed) {
Log.e(TAG, "Static object access with a destroyed context");
diff --git a/src/com/android/launcher3/util/window/WindowManagerProxy.java b/src/com/android/launcher3/util/window/WindowManagerProxy.java
index 4093bc9..278a37e 100644
--- a/src/com/android/launcher3/util/window/WindowManagerProxy.java
+++ b/src/com/android/launcher3/util/window/WindowManagerProxy.java
@@ -57,6 +57,9 @@
import com.android.launcher3.util.ResourceBasedOverride;
import com.android.launcher3.util.WindowBounds;
+import java.util.ArrayList;
+import java.util.List;
+
/**
* Utility class for mocking some window manager behaviours
*/
@@ -90,11 +93,11 @@
* Returns a map of normalized info of internal displays to estimated window bounds
* for that display
*/
- public ArrayMap<CachedDisplayInfo, WindowBounds[]> estimateInternalDisplayBounds(
+ public ArrayMap<CachedDisplayInfo, List<WindowBounds>> estimateInternalDisplayBounds(
Context displayInfoContext) {
CachedDisplayInfo info = getDisplayInfo(displayInfoContext).normalize();
- WindowBounds[] bounds = estimateWindowBounds(displayInfoContext, info);
- ArrayMap<CachedDisplayInfo, WindowBounds[]> result = new ArrayMap<>();
+ List<WindowBounds> bounds = estimateWindowBounds(displayInfoContext, info);
+ ArrayMap<CachedDisplayInfo, List<WindowBounds>> result = new ArrayMap<>();
result.put(info, bounds);
return result;
}
@@ -200,7 +203,8 @@
/**
* Returns a list of possible WindowBounds for the display keyed on the 4 surface rotations
*/
- protected WindowBounds[] estimateWindowBounds(Context context, CachedDisplayInfo displayInfo) {
+ protected List<WindowBounds> estimateWindowBounds(Context context,
+ CachedDisplayInfo displayInfo) {
int densityDpi = context.getResources().getConfiguration().densityDpi;
int rotation = displayInfo.rotation;
Rect safeCutout = displayInfo.cutout;
@@ -243,7 +247,7 @@
? 0
: getDimenByName(systemRes, NAVBAR_LANDSCAPE_LEFT_RIGHT_SIZE);
- WindowBounds[] result = new WindowBounds[4];
+ List<WindowBounds> result = new ArrayList<>(4);
Point tempSize = new Point();
for (int i = 0; i < 4; i++) {
int rotationChange = deltaRotation(rotation, i);
@@ -274,7 +278,7 @@
} else {
insets.right = Math.max(insets.right, navbarWidth);
}
- result[i] = new WindowBounds(bounds, insets, i);
+ result.add(new WindowBounds(bounds, insets, i));
}
return result;
}
diff --git a/tests/src/com/android/launcher3/AbstractDeviceProfileTest.kt b/tests/src/com/android/launcher3/AbstractDeviceProfileTest.kt
index 01f494b..a1c4f90 100644
--- a/tests/src/com/android/launcher3/AbstractDeviceProfileTest.kt
+++ b/tests/src/com/android/launcher3/AbstractDeviceProfileTest.kt
@@ -196,7 +196,7 @@
isGestureMode: Boolean,
naturalX: Int,
naturalY: Int
- ): Array<WindowBounds> {
+ ): List<WindowBounds> {
val buttonsNavHeight = Utilities.dpToPx(48f, deviceSpec.densityDpi)
val rotation0Insets =
@@ -231,7 +231,7 @@
if (isGestureMode) deviceSpec.gesturePx else 0
)
- return arrayOf(
+ return listOf(
WindowBounds(Rect(0, 0, naturalX, naturalY), rotation0Insets, Surface.ROTATION_0),
WindowBounds(Rect(0, 0, naturalY, naturalX), rotation90Insets, Surface.ROTATION_90),
WindowBounds(Rect(0, 0, naturalX, naturalY), rotation180Insets, Surface.ROTATION_180),
@@ -243,11 +243,11 @@
deviceSpec: DeviceSpec,
naturalX: Int,
naturalY: Int
- ): Array<WindowBounds> {
+ ): List<WindowBounds> {
val naturalInsets = Rect(0, deviceSpec.statusBarNaturalPx, 0, 0)
val rotatedInsets = Rect(0, deviceSpec.statusBarRotatedPx, 0, 0)
- return arrayOf(
+ return listOf(
WindowBounds(Rect(0, 0, naturalX, naturalY), naturalInsets, Surface.ROTATION_0),
WindowBounds(Rect(0, 0, naturalY, naturalX), rotatedInsets, Surface.ROTATION_90),
WindowBounds(Rect(0, 0, naturalX, naturalY), naturalInsets, Surface.ROTATION_180),
@@ -256,7 +256,7 @@
}
private fun initializeCommonVars(
- perDisplayBoundsCache: Map<CachedDisplayInfo, Array<WindowBounds>>,
+ perDisplayBoundsCache: Map<CachedDisplayInfo, List<WindowBounds>>,
displayInfo: CachedDisplayInfo,
rotation: Int,
isGestureMode: Boolean = true,
diff --git a/tests/src/com/android/launcher3/util/DisplayControllerTest.kt b/tests/src/com/android/launcher3/util/DisplayControllerTest.kt
new file mode 100644
index 0000000..2f57634
--- /dev/null
+++ b/tests/src/com/android/launcher3/util/DisplayControllerTest.kt
@@ -0,0 +1,160 @@
+/*
+ * Copyright (C) 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.android.launcher3.util
+
+import android.content.Context
+import android.content.res.Configuration
+import android.content.res.Resources
+import android.graphics.Point
+import android.graphics.Rect
+import android.hardware.display.DisplayManager
+import android.util.ArrayMap
+import android.util.DisplayMetrics
+import android.view.Display
+import android.view.Surface
+import androidx.test.annotation.UiThreadTest
+import androidx.test.core.app.ApplicationProvider
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import androidx.test.filters.SmallTest
+import com.android.launcher3.LauncherPrefs
+import com.android.launcher3.util.DisplayController.CHANGE_DENSITY
+import com.android.launcher3.util.DisplayController.CHANGE_ROTATION
+import com.android.launcher3.util.DisplayController.DisplayInfoChangeListener
+import com.android.launcher3.util.MainThreadInitializedObject.SandboxContext
+import com.android.launcher3.util.window.CachedDisplayInfo
+import com.android.launcher3.util.window.WindowManagerProxy
+import kotlin.math.min
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.mockito.Mock
+import org.mockito.Mockito.doNothing
+import org.mockito.Mockito.verify
+import org.mockito.Mockito.`when` as whenever
+import org.mockito.MockitoAnnotations
+import org.mockito.stubbing.Answer
+
+/** Unit tests for {@link DisplayController} */
+@SmallTest
+@RunWith(AndroidJUnit4::class)
+class DisplayControllerTest {
+
+ private val appContext: Context = ApplicationProvider.getApplicationContext()
+
+ @Mock private lateinit var context: SandboxContext
+ @Mock private lateinit var windowManagerProxy: WindowManagerProxy
+ @Mock private lateinit var launcherPrefs: LauncherPrefs
+ @Mock private lateinit var displayManager: DisplayManager
+ @Mock private lateinit var display: Display
+ @Mock private lateinit var resources: Resources
+ @Mock private lateinit var displayInfoChangeListener: DisplayInfoChangeListener
+
+ private lateinit var displayController: DisplayController
+
+ private val width = 2208
+ private val height = 1840
+ private val inset = 110
+ private val densityDpi = 420
+ private val density = densityDpi / DisplayMetrics.DENSITY_DEFAULT.toFloat()
+ private val bounds =
+ arrayOf(
+ WindowBounds(Rect(0, 0, width, height), Rect(0, inset, 0, 0), Surface.ROTATION_0),
+ WindowBounds(Rect(0, 0, height, width), Rect(0, inset, 0, 0), Surface.ROTATION_90),
+ WindowBounds(Rect(0, 0, width, height), Rect(0, inset, 0, 0), Surface.ROTATION_180),
+ WindowBounds(Rect(0, 0, height, width), Rect(0, inset, 0, 0), Surface.ROTATION_270)
+ )
+ private val configuration =
+ Configuration(appContext.resources.configuration).apply {
+ densityDpi = this@DisplayControllerTest.densityDpi
+ screenWidthDp = (bounds[0].bounds.width() / density).toInt()
+ screenHeightDp = (bounds[0].bounds.height() / density).toInt()
+ smallestScreenWidthDp = min(screenWidthDp, screenHeightDp)
+ }
+
+ @Before
+ fun setUp() {
+ MockitoAnnotations.initMocks(this)
+ whenever(context.getObject(eq(WindowManagerProxy.INSTANCE), any()))
+ .thenReturn(windowManagerProxy)
+ whenever(context.getObject(eq(LauncherPrefs.INSTANCE), any())).thenReturn(launcherPrefs)
+
+ // Mock WindowManagerProxy
+ val displayInfo =
+ CachedDisplayInfo(Point(width, height), Surface.ROTATION_0, Rect(0, 0, 0, 0))
+ whenever(windowManagerProxy.getDisplayInfo(any())).thenReturn(displayInfo)
+ whenever(windowManagerProxy.estimateInternalDisplayBounds(any()))
+ .thenAnswer(
+ Answer {
+ // Always create a new copy of bounds
+ val perDisplayBounds = ArrayMap<CachedDisplayInfo, List<WindowBounds>>()
+ perDisplayBounds[displayInfo] = bounds.toList()
+ return@Answer perDisplayBounds
+ }
+ )
+ whenever(windowManagerProxy.getRealBounds(any(), any())).thenAnswer { i ->
+ bounds[i.getArgument<CachedDisplayInfo>(1).rotation]
+ }
+
+ // Mock context
+ whenever(context.createWindowContext(any(), any(), nullable())).thenReturn(context)
+ whenever(context.getSystemService(eq(DisplayManager::class.java)))
+ .thenReturn(displayManager)
+ doNothing().`when`(context).registerComponentCallbacks(any())
+
+ // Mock display
+ whenever(display.rotation).thenReturn(displayInfo.rotation)
+ whenever(context.display).thenReturn(display)
+ whenever(displayManager.getDisplay(any())).thenReturn(display)
+
+ // Mock resources
+ whenever(resources.configuration).thenReturn(configuration)
+ whenever(context.resources).thenReturn(resources)
+
+ // Initialize DisplayController
+ displayController = DisplayController(context)
+ displayController.addChangeListener(displayInfoChangeListener)
+ }
+
+ @Test
+ @UiThreadTest
+ fun testRotation() {
+ val displayInfo =
+ CachedDisplayInfo(Point(height, width), Surface.ROTATION_90, Rect(0, 0, 0, 0))
+ whenever(windowManagerProxy.getDisplayInfo(any())).thenReturn(displayInfo)
+ whenever(display.rotation).thenReturn(displayInfo.rotation)
+ val configuration =
+ Configuration(configuration).apply {
+ screenWidthDp = configuration.screenHeightDp
+ screenHeightDp = configuration.screenWidthDp
+ }
+ whenever(resources.configuration).thenReturn(configuration)
+
+ displayController.onConfigurationChanged(configuration)
+
+ verify(displayInfoChangeListener).onDisplayInfoChanged(any(), any(), eq(CHANGE_ROTATION))
+ }
+
+ @Test
+ @UiThreadTest
+ fun testFontScale() {
+ val configuration = Configuration(configuration).apply { fontScale = 1.2f }
+ whenever(resources.configuration).thenReturn(configuration)
+
+ displayController.onConfigurationChanged(configuration)
+
+ verify(displayInfoChangeListener).onDisplayInfoChanged(any(), any(), eq(CHANGE_DENSITY))
+ }
+}