Restrict when a window receives new Configurations.

It is possible that a window will relayout while it's being hidden,
but after the Configuration has changed. In this case, the window can
receive an updated Configuration incompatible with its settings.
Additionally, an Activity's window may relayout and receive a
Configuration during resume after its visibility has changed but
before the Configuration has been updated based on the current
AppWindowTokens.

This changelist addresses these issues by first only sending an
updated Configuration to the client if the associated AppWindowToken
is not requested to be hidden. In this case, the last reported
Configuration is returned instead. For the resume issue, we
address the race condition by making setting the visibility and
updating the configuration synchronized.

Fixes: 64916689
Test: go/wm-smoke
Test: place clock widget on launcher, launch landscape only activity,
      return to launcher, verify layout.

Change-Id: Ie07068be64120c2fdbe380d58af330372df6a1ab

diff --git a/services/core/java/com/android/server/am/ActivityStack.java b/services/core/java/com/android/server/am/ActivityStack.java
index a0817c4..a9ce927 100644 (file)
--- a/services/core/java/com/android/server/am/ActivityStack.java
+++ b/services/core/java/com/android/server/am/ActivityStack.java
@@ -2246,7 +2246,14 @@ class ActivityStack<T extends StackWindowController> extends ConfigurationContai
         try {
             // Protect against recursion.
             mStackSupervisor.inResumeTopActivity = true;
-            result = resumeTopActivityInnerLocked(prev, options);
+            // The contained logic must be synchronized, since we are both changing the visibility
+            // and updating the {@link Configuration}. {@link ActivityRecord#setVisibility} will
+            // ultimately cause the client code to schedule a layout. Since layouts retrieve the
+            // current {@link Configuration}, we must ensure that the below code updates it before
+            // the layout can occur.
+            synchronized (mWindowManager.getWindowManagerLock()) {
+                result = resumeTopActivityInnerLocked(prev, options);
+            }
         } finally {
             mStackSupervisor.inResumeTopActivity = false;
         }

diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java
index 1ac0019..f9d10ad 100644 (file)
--- a/services/core/java/com/android/server/wm/WindowManagerService.java
+++ b/services/core/java/com/android/server/wm/WindowManagerService.java
@@ -2186,8 +2186,18 @@ public class WindowManagerService extends IWindowManager.Stub
             // and needs process it before handling the corresponding window frame. the variable
             // {@code mergedConfiguration} is an out parameter that will be passed back to the
             // client over IPC and checked there.
-            win.getMergedConfiguration(mergedConfiguration);
-            win.setReportedConfiguration(mergedConfiguration);
+            // Note: in the cases where the window is tied to an activity, we should not send a
+            // configuration update when the window has requested to be hidden. Doing so can lead
+            // to the client erroneously accepting a configuration that would have otherwise caused
+            // an activity restart. We instead hand back the last reported
+            // {@link MergedConfiguration}.
+            if (win.mAppToken == null || !win.mAppToken.isClientHidden()) {
+                win.getMergedConfiguration(mergedConfiguration);
+            } else {
+                win.getLastReportedMergedConfiguration(mergedConfiguration);
+            }
+
+            win.setLastReportedMergedConfiguration(mergedConfiguration);
 
             outFrame.set(win.mCompatFrame);
             outOverscanInsets.set(win.mOverscanInsets);

diff --git a/services/core/java/com/android/server/wm/WindowState.java b/services/core/java/com/android/server/wm/WindowState.java
index 28dba73..77a8fec 100644 (file)
--- a/services/core/java/com/android/server/wm/WindowState.java
+++ b/services/core/java/com/android/server/wm/WindowState.java
@@ -243,7 +243,7 @@ class WindowState extends WindowContainer<WindowState> implements WindowManagerP
      * We'll send configuration to client only if it is different from the last applied one and
      * client won't perform unnecessary updates.
      */
-    private final Configuration mLastReportedConfiguration = new Configuration();
+    private final MergedConfiguration mLastReportedConfiguration = new MergedConfiguration();
 
     /**
      * Actual position of the surface shown on-screen (may be modified by animation). These are
@@ -1241,7 +1241,7 @@ class WindowState extends WindowContainer<WindowState> implements WindowManagerP
         //                   this is not necessarily what the client has processed yet. Find a
         //                   better indicator consistent with the client.
         return (mOrientationChanging || (isVisible()
-                && getConfiguration().orientation != mLastReportedConfiguration.orientation))
+                && getConfiguration().orientation != getLastReportedConfiguration().orientation))
                 && !mSeamlesslyRotated
                 && !mOrientationChangeTimedOut;
     }
@@ -1769,7 +1769,7 @@ class WindowState extends WindowContainer<WindowState> implements WindowManagerP
 
     /** Returns true if last applied config was not yet requested by client. */
     boolean isConfigChanged() {
-        return !mLastReportedConfiguration.equals(getConfiguration());
+        return !getLastReportedConfiguration().equals(getConfiguration());
     }
 
     void onWindowReplacementTimeout() {
@@ -2336,8 +2336,16 @@ class WindowState extends WindowContainer<WindowState> implements WindowManagerP
         outConfiguration.setConfiguration(globalConfig, overrideConfig);
     }
 
-    void setReportedConfiguration(MergedConfiguration config) {
-        mLastReportedConfiguration.setTo(config.getMergedConfiguration());
+    void setLastReportedMergedConfiguration(MergedConfiguration config) {
+        mLastReportedConfiguration.setTo(config);
+    }
+
+    void getLastReportedMergedConfiguration(MergedConfiguration config) {
+        config.setTo(mLastReportedConfiguration);
+    }
+
+    private Configuration getLastReportedConfiguration() {
+        return mLastReportedConfiguration.getMergedConfiguration();
     }
 
     void adjustStartingWindowFlags() {
@@ -3127,7 +3135,7 @@ class WindowState extends WindowContainer<WindowState> implements WindowManagerP
                     new MergedConfiguration(mService.mRoot.getConfiguration(),
                     getMergedOverrideConfiguration());
 
-            setReportedConfiguration(mergedConfiguration);
+            setLastReportedMergedConfiguration(mergedConfiguration);
 
             if (DEBUG_ORIENTATION && mWinAnimator.mDrawState == DRAW_PENDING)
                 Slog.i(TAG, "Resizing " + this + " WITH DRAW PENDING");
@@ -3494,7 +3502,7 @@ class WindowState extends WindowContainer<WindowState> implements WindowManagerP
             }
             pw.print(prefix); pw.print("mFullConfiguration="); pw.println(getConfiguration());
             pw.print(prefix); pw.print("mLastReportedConfiguration=");
-                    pw.println(mLastReportedConfiguration);
+                    pw.println(getLastReportedConfiguration());
         }
         pw.print(prefix); pw.print("mHasSurface="); pw.print(mHasSurface);
                 pw.print(" mShownPosition="); mShownPosition.printShortString(pw);
@@ -3555,7 +3563,7 @@ class WindowState extends WindowContainer<WindowState> implements WindowManagerP
             pw.print(prefix); pw.print("mOrientationChanging=");
                     pw.print(mOrientationChanging);
                     pw.print(" configOrientationChanging=");
-                    pw.print(mLastReportedConfiguration.orientation
+                    pw.print(getLastReportedConfiguration().orientation
                             != getConfiguration().orientation);
                     pw.print(" mAppFreezing="); pw.print(mAppFreezing);
                     pw.print(" mTurnOnScreen="); pw.print(mTurnOnScreen);

diff --git a/services/tests/servicestests/src/com/android/server/wm/WindowTestUtils.java b/services/tests/servicestests/src/com/android/server/wm/WindowTestUtils.java
index 7ff1110..272e451 100644 (file)
--- a/services/tests/servicestests/src/com/android/server/wm/WindowTestUtils.java
+++ b/services/tests/servicestests/src/com/android/server/wm/WindowTestUtils.java
@@ -32,6 +32,7 @@ import static android.content.pm.ActivityInfo.SCREEN_ORIENTATION_UNSPECIFIED;
 import static android.content.res.Configuration.EMPTY;
 import static com.android.server.wm.WindowContainer.POSITION_TOP;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 /**
  * A collection of static functions that can be referenced by other test packages to provide access
@@ -51,7 +52,10 @@ public class WindowTestUtils {
      * Retrieves an instance of a mock {@link WindowManagerService}.
      */
     public static WindowManagerService getMockWindowManagerService() {
-        return mock(WindowManagerService.class);
+        final WindowManagerService service = mock(WindowManagerService.class);
+        final WindowHashMap windowMap = new WindowHashMap();
+        when(service.getWindowManagerLock()).thenReturn(windowMap);
+        return service;
     }
 
     /**