From 2b17afd33075ddb81bbaa9616750cfeb5ee13665 Mon Sep 17 00:00:00 2001 From: Bryce Lee Date: Thu, 21 Sep 2017 10:38:20 -0700 Subject: 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 --- .../java/com/android/server/am/ActivityStack.java | 9 +++++++- .../android/server/wm/WindowManagerService.java | 14 +++++++++++-- .../java/com/android/server/wm/WindowState.java | 24 ++++++++++++++-------- .../src/com/android/server/wm/WindowTestUtils.java | 6 +++++- 4 files changed, 41 insertions(+), 12 deletions(-) diff --git a/services/core/java/com/android/server/am/ActivityStack.java b/services/core/java/com/android/server/am/ActivityStack.java index a0817c475918..a9ce927a56a5 100644 --- 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 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 1ac0019f1bae..f9d10ada0e31 100644 --- 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 28dba73db263..77a8fec7241b 100644 --- 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 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 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 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 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 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 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 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 7ff1110e00f7..272e451e74c9 100644 --- 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; } /** -- cgit v1.2.3-59-g8ed1b