Merge "ScreenDecorations: Fix stuck / delayed rotations" into pi-dev

4 files changed, 197 insertions, 46 deletions

diff --git a/packages/SystemUI/src/com/android/systemui/ScreenDecorations.java b/packages/SystemUI/src/com/android/systemui/ScreenDecorations.java
index 194679cc626e..71cf9b52a2db 100644
--- a/packages/SystemUI/src/com/android/systemui/ScreenDecorations.java
+++ b/packages/SystemUI/src/com/android/systemui/ScreenDecorations.java
@@ -39,10 +39,13 @@ import android.graphics.PixelFormat;
 import android.graphics.Rect;
 import android.graphics.Region;
 import android.hardware.display.DisplayManager;
+import android.os.Handler;
+import android.os.HandlerThread;
 import android.os.SystemProperties;
 import android.provider.Settings.Secure;
 import android.support.annotation.VisibleForTesting;
 import android.util.DisplayMetrics;
+import android.util.Log;
 import android.view.DisplayCutout;
 import android.view.DisplayInfo;
 import android.view.Gravity;
@@ -57,6 +60,7 @@ import android.view.WindowManager;
 import android.widget.FrameLayout;
 import android.widget.ImageView;
 
+import com.android.internal.util.Preconditions;
 import com.android.systemui.RegionInterceptingFrameLayout.RegionInterceptableView;
 import com.android.systemui.fragments.FragmentHostManager;
 import com.android.systemui.fragments.FragmentHostManager.FragmentListener;
@@ -74,6 +78,9 @@ import com.android.systemui.util.leak.RotationUtils;
  * for antialiasing and emulation purposes.
  */
 public class ScreenDecorations extends SystemUI implements Tunable {
+    private static final boolean DEBUG = false;
+    private static final String TAG = "ScreenDecorations";
+
     public static final String SIZE = "sysui_rounded_size";
     public static final String PADDING = "sysui_rounded_content_padding";
     private static final boolean DEBUG_SCREENSHOT_ROUNDED_CORNERS =
@@ -93,9 +100,24 @@ public class ScreenDecorations extends SystemUI implements Tunable {
     private DisplayCutoutView mCutoutTop;
     private DisplayCutoutView mCutoutBottom;
     private boolean mPendingRotationChange;
+    private Handler mHandler;
 
     @Override
     public void start() {
+        mHandler = startHandlerThread();
+        mHandler.post(this::startOnScreenDecorationsThread);
+        setupStatusBarPaddingIfNeeded();
+    }
+
+    @VisibleForTesting
+    Handler startHandlerThread() {
+        HandlerThread thread = new HandlerThread("ScreenDecorations");
+        thread.start();
+        return thread.getThreadHandler();
+    }
+
+    private void startOnScreenDecorationsThread() {
+        mRotation = RotationUtils.getExactRotation(mContext);
         mWindowManager = mContext.getSystemService(WindowManager.class);
         mRoundedDefault = mContext.getResources().getDimensionPixelSize(
                 R.dimen.rounded_corner_radius);
@@ -107,12 +129,6 @@ public class ScreenDecorations extends SystemUI implements Tunable {
             setupDecorations();
         }
 
-        int padding = mContext.getResources().getDimensionPixelSize(
-                R.dimen.rounded_corner_content_padding);
-        if (padding != 0) {
-            setupPadding(padding);
-        }
-
         mDisplayListener = new DisplayManager.DisplayListener() {
             @Override
             public void onDisplayAdded(int displayId) {
@@ -126,8 +142,8 @@ public class ScreenDecorations extends SystemUI implements Tunable {
 
             @Override
             public void onDisplayChanged(int displayId) {
-                if (mOverlay != null && mBottomOverlay != null
-                        && mRotation != RotationUtils.getExactRotation(mContext)) {
+                final int newRotation = RotationUtils.getExactRotation(mContext);
+                if (mOverlay != null && mBottomOverlay != null && mRotation != newRotation) {
                     // We cannot immediately update the orientation. Otherwise
                     // WindowManager is still deferring layout until it has finished dispatching
                     // the config changes, which may cause divergence between what we draw
@@ -136,20 +152,24 @@ public class ScreenDecorations extends SystemUI implements Tunable {
                     // - we are trying to redraw. This because WM resized our window and told us to.
                     // - the config change has been dispatched, so WM is no longer deferring layout.
                     mPendingRotationChange = true;
+                    if (DEBUG) {
+                        Log.i(TAG, "Rotation changed, deferring " + newRotation + ", staying at "
+                                + mRotation);
+                    }
+
                     mOverlay.getViewTreeObserver().addOnPreDrawListener(
-                            new RestartingPreDrawListener(mOverlay));
+                            new RestartingPreDrawListener(mOverlay, newRotation));
                     mBottomOverlay.getViewTreeObserver().addOnPreDrawListener(
-                            new RestartingPreDrawListener(mBottomOverlay));
-
+                            new RestartingPreDrawListener(mBottomOverlay, newRotation));
                 }
                 updateOrientation();
             }
         };
 
-        mRotation = -1;
         mDisplayManager = (DisplayManager) mContext.getSystemService(
                 Context.DISPLAY_SERVICE);
-        mDisplayManager.registerDisplayListener(mDisplayListener, null);
+        mDisplayManager.registerDisplayListener(mDisplayListener, mHandler);
+        updateOrientation();
     }
 
     private void setupDecorations() {
@@ -179,10 +199,11 @@ public class ScreenDecorations extends SystemUI implements Tunable {
         mWindowManager.getDefaultDisplay().getMetrics(metrics);
         mDensity = metrics.density;
 
-        Dependency.get(TunerService.class).addTunable(this, SIZE);
+        Dependency.get(Dependency.MAIN_HANDLER).post(
+                () -> Dependency.get(TunerService.class).addTunable(this, SIZE));
 
         // Watch color inversion and invert the overlay as needed.
-        SecureSetting setting = new SecureSetting(mContext, Dependency.get(Dependency.MAIN_HANDLER),
+        SecureSetting setting = new SecureSetting(mContext, mHandler,
                 Secure.ACCESSIBILITY_DISPLAY_INVERSION_ENABLED) {
             @Override
             protected void handleValueChanged(int value, boolean observedChange) {
@@ -215,18 +236,37 @@ public class ScreenDecorations extends SystemUI implements Tunable {
                         .start();
             }
         });
+
+        mOverlay.getViewTreeObserver().addOnPreDrawListener(
+                new ValidatingPreDrawListener(mOverlay));
+        mBottomOverlay.getViewTreeObserver().addOnPreDrawListener(
+                new ValidatingPreDrawListener(mBottomOverlay));
     }
 
     @Override
     protected void onConfigurationChanged(Configuration newConfig) {
-        mPendingRotationChange = false;
-        updateOrientation();
-        if (shouldDrawCutout() && mOverlay == null) {
-            setupDecorations();
-        }
+        mHandler.post(() -> {
+            int oldRotation = mRotation;
+            mPendingRotationChange = false;
+            updateOrientation();
+            if (DEBUG) Log.i(TAG, "onConfigChanged from rot " + oldRotation + " to " + mRotation);
+            if (shouldDrawCutout() && mOverlay == null) {
+                setupDecorations();
+            }
+            if (mOverlay != null) {
+                // Updating the layout params ensures that ViewRootImpl will call relayoutWindow(),
+                // which ensures that the forced seamless rotation will end, even if we updated
+                // the rotation before window manager was ready (and was still waiting for sending
+                // the updated rotation).
+                updateLayoutParams();
+            }
+        });
     }
 
-    protected void updateOrientation() {
+    private void updateOrientation() {
+        Preconditions.checkState(mHandler.getLooper().getThread() == Thread.currentThread(),
+                "must call on " + mHandler.getLooper().getThread()
+                        + ", but was " + Thread.currentThread());
         if (mPendingRotationChange) {
             return;
         }
@@ -306,7 +346,19 @@ public class ScreenDecorations extends SystemUI implements Tunable {
                 com.android.internal.R.bool.config_fillMainBuiltInDisplayCutout);
     }
 
-    private void setupPadding(int padding) {
+
+    private void setupStatusBarPaddingIfNeeded() {
+        // TODO: This should be moved to a more appropriate place, as it is not related to the
+        // screen decorations overlay.
+        int padding = mContext.getResources().getDimensionPixelSize(
+                R.dimen.rounded_corner_content_padding);
+        if (padding != 0) {
+            setupStatusBarPadding(padding);
+        }
+
+    }
+
+    private void setupStatusBarPadding(int padding) {
         // Add some padding to all the content near the edge of the screen.
         StatusBar sb = getComponent(StatusBar.class);
         View statusBar = (sb != null ? sb.getStatusBarWindow() : null);
@@ -375,30 +427,32 @@ public class ScreenDecorations extends SystemUI implements Tunable {
 
     @Override
     public void onTuningChanged(String key, String newValue) {
-        if (mOverlay == null) return;
-        if (SIZE.equals(key)) {
-            int size = mRoundedDefault;
-            int sizeTop = mRoundedDefaultTop;
-            int sizeBottom = mRoundedDefaultBottom;
-            if (newValue != null) {
-                try {
-                    size = (int) (Integer.parseInt(newValue) * mDensity);
-                } catch (Exception e) {
+        mHandler.post(() -> {
+            if (mOverlay == null) return;
+            if (SIZE.equals(key)) {
+                int size = mRoundedDefault;
+                int sizeTop = mRoundedDefaultTop;
+                int sizeBottom = mRoundedDefaultBottom;
+                if (newValue != null) {
+                    try {
+                        size = (int) (Integer.parseInt(newValue) * mDensity);
+                    } catch (Exception e) {
+                    }
                 }
-            }
 
-            if (sizeTop == 0) {
-                sizeTop = size;
-            }
-            if (sizeBottom == 0) {
-                sizeBottom = size;
-            }
+                if (sizeTop == 0) {
+                    sizeTop = size;
+                }
+                if (sizeBottom == 0) {
+                    sizeBottom = size;
+                }
 
-            setSize(mOverlay.findViewById(R.id.left), sizeTop);
-            setSize(mOverlay.findViewById(R.id.right), sizeTop);
-            setSize(mBottomOverlay.findViewById(R.id.left), sizeBottom);
-            setSize(mBottomOverlay.findViewById(R.id.right), sizeBottom);
-        }
+                setSize(mOverlay.findViewById(R.id.left), sizeTop);
+                setSize(mOverlay.findViewById(R.id.right), sizeTop);
+                setSize(mBottomOverlay.findViewById(R.id.left), sizeBottom);
+                setSize(mBottomOverlay.findViewById(R.id.right), sizeBottom);
+            }
+        });
     }
 
     private void setSize(View view, int pixelSize) {
@@ -457,6 +511,11 @@ public class ScreenDecorations extends SystemUI implements Tunable {
             mVisibilityChangedListener = visibilityChangedListener;
             mDecorations = decorations;
             setId(R.id.display_cutout);
+            if (DEBUG) {
+                getViewTreeObserver().addOnDrawListener(() -> Log.i(TAG,
+                        (mInitialStart ? "OverlayTop" : "OverlayBottom")
+                                + " drawn in rot " + mRotation));
+            }
         }
 
         public void setColor(int color) {
@@ -692,20 +751,66 @@ public class ScreenDecorations extends SystemUI implements Tunable {
     private class RestartingPreDrawListener implements ViewTreeObserver.OnPreDrawListener {
 
         private final View mView;
+        private final int mTargetRotation;
 
-        private RestartingPreDrawListener(View view) {
+        private RestartingPreDrawListener(View view, int targetRotation) {
             mView = view;
+            mTargetRotation = targetRotation;
         }
 
         @Override
         public boolean onPreDraw() {
-            mPendingRotationChange = false;
             mView.getViewTreeObserver().removeOnPreDrawListener(this);
+
+            if (mTargetRotation == mRotation) {
+                if (DEBUG) {
+                    Log.i(TAG, (mView == mOverlay ? "OverlayTop" : "OverlayBottom")
+                            + " already in target rot "
+                            + mTargetRotation + ", allow draw without restarting it");
+                }
+                return true;
+            }
+
+            mPendingRotationChange = false;
             // This changes the window attributes - we need to restart the traversal for them to
             // take effect.
             updateOrientation();
+            if (DEBUG) {
+                Log.i(TAG, (mView == mOverlay ? "OverlayTop" : "OverlayBottom")
+                        + " restarting listener fired, restarting draw for rot " + mRotation);
+            }
             mView.invalidate();
             return false;
         }
     }
+
+    /**
+     * A pre-draw listener, that validates that the rotation we draw in matches the displays
+     * rotation before continuing the draw.
+     *
+     * This is to prevent a race condition, where we have not received the display changed event
+     * yet, and would thus draw in an old orientation.
+     */
+    private class ValidatingPreDrawListener implements ViewTreeObserver.OnPreDrawListener {
+
+        private final View mView;
+
+        public ValidatingPreDrawListener(View view) {
+            mView = view;
+        }
+
+        @Override
+        public boolean onPreDraw() {
+            final int displayRotation = RotationUtils.getExactRotation(mContext);
+            if (displayRotation != mRotation && !mPendingRotationChange) {
+                if (DEBUG) {
+                    Log.i(TAG, "Drawing rot " + mRotation + ", but display is at rot "
+                            + displayRotation + ". Restarting draw");
+                }
+                mView.invalidate();
+                return false;
+            }
+            return true;
+        }
+    }
 }
diff --git a/packages/SystemUI/tests/src/com/android/systemui/ScreenDecorationsTest.java b/packages/SystemUI/tests/src/com/android/systemui/ScreenDecorationsTest.java
index f1bf31d7a58a..644c0b347bde 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/ScreenDecorationsTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/ScreenDecorationsTest.java
@@ -34,8 +34,10 @@ import static org.mockito.Mockito.when;
 
 import android.app.Fragment;
 import android.content.res.Configuration;
+import android.os.Handler;
 import android.support.test.filters.SmallTest;
 import android.testing.AndroidTestingRunner;
+import android.testing.TestableLooper;
 import android.testing.TestableLooper.RunWithLooper;
 import android.view.Display;
 import android.view.View;
@@ -60,6 +62,7 @@ import org.junit.runner.RunWith;
 @SmallTest
 public class ScreenDecorationsTest extends SysuiTestCase {
 
+    private TestableLooper mTestableLooper;
     private ScreenDecorations mScreenDecorations;
     private StatusBar mStatusBar;
     private WindowManager mWindowManager;
@@ -71,6 +74,10 @@ public class ScreenDecorationsTest extends SysuiTestCase {
 
     @Before
     public void setup() {
+        mTestableLooper = TestableLooper.get(this);
+        mDependency.injectTestDependency(Dependency.MAIN_HANDLER,
+                new Handler(mTestableLooper.getLooper()));
+
         mStatusBar = mock(StatusBar.class);
         mWindowManager = mock(WindowManager.class);
         mView = spy(new StatusBarWindowView(mContext, null));
@@ -88,7 +95,31 @@ public class ScreenDecorationsTest extends SysuiTestCase {
 
         mTunerService = mDependency.injectMockDependency(TunerService.class);
 
-        mScreenDecorations = new ScreenDecorations();
+
+        mScreenDecorations = new ScreenDecorations() {
+            @Override
+            public void start() {
+                super.start();
+                mTestableLooper.processAllMessages();
+            }
+
+            @Override
+            Handler startHandlerThread() {
+                return new Handler(mTestableLooper.getLooper());
+            }
+
+            @Override
+            protected void onConfigurationChanged(Configuration newConfig) {
+                super.onConfigurationChanged(newConfig);
+                mTestableLooper.processAllMessages();
+            }
+
+            @Override
+            public void onTuningChanged(String key, String newValue) {
+                super.onTuningChanged(key, newValue);
+                mTestableLooper.processAllMessages();
+            }
+        };
         mScreenDecorations.mContext = mContext;
         mScreenDecorations.mComponents = mContext.getComponents();
 
diff --git a/services/core/java/com/android/server/wm/ForcedSeamlessRotator.java b/services/core/java/com/android/server/wm/ForcedSeamlessRotator.java
index bebc5656c284..efb43a6b90cf 100644
--- a/services/core/java/com/android/server/wm/ForcedSeamlessRotator.java
+++ b/services/core/java/com/android/server/wm/ForcedSeamlessRotator.java
@@ -21,6 +21,7 @@ import static android.view.Surface.ROTATION_90;
 
 import android.graphics.Matrix;
 import android.view.DisplayInfo;
+import android.view.Surface.Rotation;
 
 import com.android.server.wm.utils.CoordinateTransforms;
 
@@ -65,6 +66,16 @@ public class ForcedSeamlessRotator {
     }
 
     /**
+     * Returns the rotation of the display before it started rotating.
+     *
+     * @return the old rotation of the display
+     */
+    @Rotation
+    public int getOldRotation() {
+        return mOldRotation;
+    }
+
+    /**
      * Removes the transform to the window token's surface that undoes the effect of the global
      * display rotation.
      *
diff --git a/services/core/java/com/android/server/wm/WindowState.java b/services/core/java/com/android/server/wm/WindowState.java
index 009f3930d02e..1ae680f793ca 100644
--- a/services/core/java/com/android/server/wm/WindowState.java
+++ b/services/core/java/com/android/server/wm/WindowState.java
@@ -681,6 +681,10 @@ class WindowState extends WindowContainer<WindowState> implements WindowManagerP
 
     void forceSeamlesslyRotateIfAllowed(int oldRotation, int rotation) {
         if (mForceSeamlesslyRotate) {
+            if (mPendingForcedSeamlessRotate != null) {
+                oldRotation = mPendingForcedSeamlessRotate.getOldRotation();
+            }
+
             mPendingForcedSeamlessRotate = new ForcedSeamlessRotator(
                     oldRotation, rotation, getDisplayInfo());
             mPendingForcedSeamlessRotate.unrotate(this.mToken);