From 8c3cf38592e0e197606cf0ce1b1d6e85b7e55711 Mon Sep 17 00:00:00 2001 From: Bryce Lee Date: Thu, 6 Jul 2017 19:47:10 -0700 Subject: [PATCH] Use configuration delta to determine if still in orientation change. We currently track this change in the WindowState with a variable called mOrientationChanging. This value is set and cleared in a variety of classes. It is possible for this value to be cleared before processing an orientation changing frame. This can lead to us prematurely unfreezing. Since the orientation is present in the configuration, we can use the orientation delta between the last and current frame's configuration to determine whether we are in the middle of an orientation change. The existing signal has been moved behind a setter/getter where the latter is combined with the configuration state. This reverts commit 31386ab88b48fd5475efc58155549ad4c7da7ad2. It addresses issues raised in b/63380251 where the orientation delta was being considered for WindowStates that were not visible. Bug: 62846907 Test: go/wm-smoke Change-Id: I574efb8f9a458cb295f2ef14e10dbb91e8875bf4 --- .../com/android/server/wm/DisplayContent.java | 6 ++-- .../server/wm/WindowManagerService.java | 2 +- .../com/android/server/wm/WindowState.java | 36 ++++++++++++++----- .../server/wm/WindowStateAnimator.java | 14 ++++---- 4 files changed, 39 insertions(+), 19 deletions(-) diff --git a/services/core/java/com/android/server/wm/DisplayContent.java b/services/core/java/com/android/server/wm/DisplayContent.java index 9fe73815b380d..05f4626259d78 100644 --- a/services/core/java/com/android/server/wm/DisplayContent.java +++ b/services/core/java/com/android/server/wm/DisplayContent.java @@ -1073,7 +1073,7 @@ class DisplayContent extends WindowContainer { - if (!w.mOrientationChanging) { + if (!w.getOrientationChanging()) { return; } - w.mOrientationChanging = false; + w.setOrientationChanging(false); w.mLastFreezeDuration = (int)(SystemClock.elapsedRealtime() - mService.mDisplayFreezeTime); Slog.w(TAG_WM, "Force clearing orientation change: " + w); diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java index ebfeac3390838..1b4c34d212571 100644 --- a/services/core/java/com/android/server/wm/WindowManagerService.java +++ b/services/core/java/com/android/server/wm/WindowManagerService.java @@ -5789,7 +5789,7 @@ public class WindowManagerService extends IWindowManager.Stub // orientation. if (!okToDisplay() && mWindowsFreezingScreen != WINDOWS_FREEZING_SCREENS_TIMEOUT) { if (DEBUG_ORIENTATION) Slog.v(TAG_WM, "Changing surface while display frozen: " + w); - w.mOrientationChanging = true; + w.setOrientationChanging(true); w.mLastFreezeDuration = 0; mRoot.mOrientationChangeComplete = false; if (mWindowsFreezingScreen == WINDOWS_FREEZING_SCREENS_NONE) { diff --git a/services/core/java/com/android/server/wm/WindowState.java b/services/core/java/com/android/server/wm/WindowState.java index 1ec8e54cad36a..f6464cc6a9109 100644 --- a/services/core/java/com/android/server/wm/WindowState.java +++ b/services/core/java/com/android/server/wm/WindowState.java @@ -441,7 +441,7 @@ class WindowState extends WindowContainer implements WindowManagerP * Set when the orientation is changing and this window has not yet * been updated for the new orientation. */ - boolean mOrientationChanging; + private boolean mOrientationChanging; /** * The orientation during the last visible call to relayout. If our @@ -1184,7 +1184,8 @@ class WindowState extends WindowContainer implements WindowManagerP // then we need to hold off on unfreezing the display until this window has been // redrawn; to do that, we need to go through the process of getting informed by the // application when it has finished drawing. - if (mOrientationChanging || dragResizingChanged || isResizedWhileNotDragResizing()) { + if (getOrientationChanging() || dragResizingChanged + || isResizedWhileNotDragResizing()) { if (DEBUG_SURFACE_TRACE || DEBUG_ANIM || DEBUG_ORIENTATION || DEBUG_RESIZE) { Slog.v(TAG_WM, "Orientation or resize start waiting for draw" + ", mDrawState=DRAW_PENDING in " + this @@ -1199,17 +1200,33 @@ class WindowState extends WindowContainer implements WindowManagerP if (DEBUG_RESIZE || DEBUG_ORIENTATION) Slog.v(TAG_WM, "Resizing window " + this); mService.mResizingWindows.add(this); } - } else if (mOrientationChanging) { + } else if (getOrientationChanging()) { if (isDrawnLw()) { if (DEBUG_ORIENTATION) Slog.v(TAG_WM, "Orientation not waiting for draw in " + this + ", surfaceController " + winAnimator.mSurfaceController); - mOrientationChanging = false; + setOrientationChanging(false); mLastFreezeDuration = (int)(SystemClock.elapsedRealtime() - mService.mDisplayFreezeTime); } } } + boolean getOrientationChanging() { + // In addition to the local state flag, we must also consider the difference in the last + // reported configuration vs. the current state. If the client code has not been informed of + // the change, logic dependent on having finished processing the orientation, such as + // unfreezing, could be improperly triggered. + // TODO(b/62846907): Checking against {@link mLastReportedConfiguration} could be flaky as + // 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); + } + + void setOrientationChanging(boolean changing) { + mOrientationChanging = changing; + } + DisplayContent getDisplayContent() { return mToken.getDisplayContent(); } @@ -2663,10 +2680,10 @@ class WindowState extends WindowContainer implements WindowManagerP mAppFreezing = false; - if (mHasSurface && !mOrientationChanging + if (mHasSurface && !getOrientationChanging() && mService.mWindowsFreezingScreen != WINDOWS_FREEZING_SCREENS_TIMEOUT) { if (DEBUG_ORIENTATION) Slog.v(TAG_WM, "set mOrientationChanging of " + this); - mOrientationChanging = true; + setOrientationChanging(true); mService.mRoot.mOrientationChangeComplete = false; } mLastFreezeDuration = 0; @@ -3085,7 +3102,7 @@ class WindowState extends WindowContainer implements WindowManagerP mWinAnimator.mSurfaceResized = false; mReportOrientationChanged = false; } catch (RemoteException e) { - mOrientationChanging = false; + setOrientationChanging(false); mLastFreezeDuration = (int)(SystemClock.elapsedRealtime() - mService.mDisplayFreezeTime); // We are assuming the hosting process is dead or in a zombie state. @@ -3444,10 +3461,13 @@ class WindowState extends WindowContainer implements WindowManagerP pw.print(" mDestroying="); pw.print(mDestroying); pw.print(" mRemoved="); pw.println(mRemoved); } - if (mOrientationChanging || mAppFreezing || mTurnOnScreen + if (getOrientationChanging() || mAppFreezing || mTurnOnScreen || mReportOrientationChanged) { pw.print(prefix); pw.print("mOrientationChanging="); pw.print(mOrientationChanging); + pw.print(" configOrientationChanging="); + pw.print(mLastReportedConfiguration.orientation + != getConfiguration().orientation); pw.print(" mAppFreezing="); pw.print(mAppFreezing); pw.print(" mTurnOnScreen="); pw.print(mTurnOnScreen); pw.print(" mReportOrientationChanged="); pw.println(mReportOrientationChanged); diff --git a/services/core/java/com/android/server/wm/WindowStateAnimator.java b/services/core/java/com/android/server/wm/WindowStateAnimator.java index cd55156a67a99..8f1065f756424 100644 --- a/services/core/java/com/android/server/wm/WindowStateAnimator.java +++ b/services/core/java/com/android/server/wm/WindowStateAnimator.java @@ -1527,11 +1527,11 @@ class WindowStateAnimator { // There is no need to wait for an animation change if our window is gone for layout // already as we'll never be visible. - if (w.mOrientationChanging && w.isGoneForLayoutLw()) { + if (w.getOrientationChanging() && w.isGoneForLayoutLw()) { if (DEBUG_ORIENTATION) { Slog.v(TAG, "Orientation change skips hidden " + w); } - w.mOrientationChanging = false; + w.setOrientationChanging(false); } return; } @@ -1564,8 +1564,8 @@ class WindowStateAnimator { // really hidden (gone for layout), there is no point in still waiting for it. // Note that this does introduce a potential glitch if the window becomes unhidden // before it has drawn for the new orientation. - if (w.mOrientationChanging && w.isGoneForLayoutLw()) { - w.mOrientationChanging = false; + if (w.getOrientationChanging() && w.isGoneForLayoutLw()) { + w.setOrientationChanging(false); if (DEBUG_ORIENTATION) Slog.v(TAG, "Orientation change skips hidden " + w); } @@ -1618,7 +1618,7 @@ class WindowStateAnimator { mAnimator.setPendingLayoutChanges(w.getDisplayId(), WindowManagerPolicy.FINISH_LAYOUT_REDO_ANIM); } else { - w.mOrientationChanging = false; + w.setOrientationChanging(false); } } if (hasSurface()) { @@ -1631,14 +1631,14 @@ class WindowStateAnimator { displayed = true; } - if (w.mOrientationChanging) { + if (w.getOrientationChanging()) { if (!w.isDrawnLw()) { mAnimator.mBulkUpdateParams &= ~SET_ORIENTATION_CHANGE_COMPLETE; mAnimator.mLastWindowFreezeSource = w; if (DEBUG_ORIENTATION) Slog.v(TAG, "Orientation continue waiting for draw in " + w); } else { - w.mOrientationChanging = false; + w.setOrientationChanging(false); if (DEBUG_ORIENTATION) Slog.v(TAG, "Orientation change complete in " + w); } }