Merge "Fix out-of-order transactions (1/2)" into oc-dr1-dev

services/core/java/com/android/server/wm/WindowAnimator.java

@@ -33,7 +33,6 @@ import android.view.Choreographer;
 import android.view.SurfaceControl;
 import android.view.WindowManagerPolicy;
 
-import com.android.internal.view.SurfaceFlingerVsyncChoreographer;
 import com.android.server.AnimationThread;
 
 import java.io.PrintWriter;
@@ -134,13 +133,27 @@ public class WindowAnimator {
      * sure other threads can make progress if this happens.
      */
     private void animate(long frameTimeNs) {
-        boolean transactionOpen = false;
+
-        try {
         synchronized (mService.mWindowMap) {
             if (!mInitialized) {
                 return;
             }
 
+            // Schedule next frame already such that back-pressure happens continuously
+            scheduleAnimation();
+        }
+
+        // Simulate back-pressure by opening and closing an empty animation transaction. This makes
+        // sure that an animation frame is at least presented once on the screen. We do this outside
+        // of the regular transaction such that we can avoid holding the window manager lock in case
+        // we receive back-pressure from SurfaceFlinger. Since closing an animation transaction
+        // without the window manager locks leads to ordering issues (as the transaction will be
+        // processed only at the beginning of the next frame which may result in another transaction
+        // that was executed later in WM side gets executed first on SF side), we don't update any
+        // Surface properties here such that reordering doesn't cause issues.
+        mService.executeEmptyAnimationTransaction();
+
+        synchronized (mService.mWindowMap) {
             mCurrentTime = frameTimeNs / TimeUtils.NANOS_PER_MS;
             mBulkUpdateParams = SET_ORIENTATION_CHANGE_COMPLETE;
             mAnimating = false;
@@ -151,9 +164,7 @@ public class WindowAnimator {
 
             if (SHOW_TRANSACTIONS) Slog.i(TAG, ">>> OPEN TRANSACTION animate");
             mService.openSurfaceTransaction();
-                transactionOpen = true;
+            try {
-                SurfaceControl.setAnimationTransaction();
-
                 final AccessibilityController accessibilityController =
                         mService.mAccessibilityController;
                 final int numDisplays = mDisplayContentsAnimators.size();
@@ -216,27 +227,20 @@ public class WindowAnimator {
                     mAnimating |= mService.mDragState.stepAnimationLocked(mCurrentTime);
                 }
 
-                if (mAnimating) {
+                if (!mAnimating) {
-                    mService.scheduleAnimationLocked();
+                    cancelAnimation();
                 }
 
                 if (mService.mWatermark != null) {
                     mService.mWatermark.drawIfNeeded();
                 }
-            }
             } catch (RuntimeException e) {
                 Slog.wtf(TAG, "Unhandled exception in Window Manager", e);
             } finally {
-            if (transactionOpen) {
+                mService.closeSurfaceTransaction();
-
-                // Do not hold window manager lock while closing the transaction, as this might be
-                // blocking until the next frame, which can lead to total lock starvation.
-                mService.closeSurfaceTransaction(false /* withLockHeld */);
                 if (SHOW_TRANSACTIONS) Slog.i(TAG, "<<< CLOSE TRANSACTION animate");
             }
-        }
 
-        synchronized (mService.mWindowMap) {
             boolean hasPendingLayoutChanges = mService.mRoot.hasPendingLayoutChanges(this);
             boolean doRequest = false;
             if (mBulkUpdateParams != 0) {
@@ -404,6 +408,13 @@ public class WindowAnimator {
         }
     }
 
+    private void cancelAnimation() {
+        if (mAnimationFrameCallbackScheduled) {
+            mAnimationFrameCallbackScheduled = false;
+            mChoreographer.removeFrameCallback(mAnimationFrameCallback);
+        }
+    }
+
     private class DisplayContentsAnimator {
         ScreenRotationAnimation mScreenRotationAnimation = null;
     }

services/core/java/com/android/server/wm/WindowManagerService.java

@@ -908,31 +908,46 @@ public class WindowManagerService extends IWindowManager.Stub
         }
     }
 
-    void closeSurfaceTransaction() {
-        closeSurfaceTransaction(true /* withLockHeld */);
-    }
-
     /**
      * Closes a surface transaction.
-     *
-     * @param withLockHeld Whether to acquire the window manager while doing so. In some cases
-     *                     holding the lock my lead to starvation in WM in case closeTransaction
-     *                     blocks and we call it repeatedly, like we do for animations.
      */
-    void closeSurfaceTransaction(boolean withLockHeld) {
+    void closeSurfaceTransaction() {
         try {
             Trace.traceBegin(TRACE_TAG_WINDOW_MANAGER, "closeSurfaceTransaction");
             synchronized (mWindowMap) {
                 if (mRoot.mSurfaceTraceEnabled) {
                     mRoot.mRemoteEventTrace.closeSurfaceTransaction();
                 }
-                if (withLockHeld) {
                 SurfaceControl.closeTransaction();
             }
+        } finally {
+            Trace.traceEnd(TRACE_TAG_WINDOW_MANAGER);
         }
-            if (!withLockHeld) {
+    }
+
+    /**
+     * Executes an empty animation transaction without holding the WM lock to simulate
+     * back-pressure. See {@link WindowAnimator#animate} why this is needed.
+     */
+    void executeEmptyAnimationTransaction() {
+        try {
+            Trace.traceBegin(TRACE_TAG_WINDOW_MANAGER, "openSurfaceTransaction");
+            synchronized (mWindowMap) {
+                if (mRoot.mSurfaceTraceEnabled) {
+                    mRoot.mRemoteEventTrace.openSurfaceTransaction();
+                }
+                SurfaceControl.openTransaction();
+                SurfaceControl.setAnimationTransaction();
+                if (mRoot.mSurfaceTraceEnabled) {
+                    mRoot.mRemoteEventTrace.closeSurfaceTransaction();
+                }
+            }
+        } finally {
+            Trace.traceEnd(TRACE_TAG_WINDOW_MANAGER);
+        }
+        try {
+            Trace.traceBegin(TRACE_TAG_WINDOW_MANAGER, "closeSurfaceTransaction");
             SurfaceControl.closeTransaction();
-            }
         } finally {
             Trace.traceEnd(TRACE_TAG_WINDOW_MANAGER);
         }