Merge "Fix out-of-order transactions (1/2)" into oc-dr1-dev
diff --git a/services/core/java/com/android/server/wm/WindowAnimator.java b/services/core/java/com/android/server/wm/WindowAnimator.java
index fe5b7f2..079ae40 100644
--- a/services/core/java/com/android/server/wm/WindowAnimator.java
+++ b/services/core/java/com/android/server/wm/WindowAnimator.java
@@ -33,7 +33,6 @@
 import android.view.SurfaceControl;
 import android.view.WindowManagerPolicy;
 
-import com.android.internal.view.SurfaceFlingerVsyncChoreographer;
 import com.android.server.AnimationThread;
 
 import java.io.PrintWriter;
@@ -134,26 +133,38 @@
      * sure other threads can make progress if this happens.
      */
     private void animate(long frameTimeNs) {
-        boolean transactionOpen = false;
-        try {
-            synchronized (mService.mWindowMap) {
-                if (!mInitialized) {
-                    return;
-                }
 
-                mCurrentTime = frameTimeNs / TimeUtils.NANOS_PER_MS;
-                mBulkUpdateParams = SET_ORIENTATION_CHANGE_COMPLETE;
-                mAnimating = false;
-                mAppWindowAnimating = false;
-                if (DEBUG_WINDOW_TRACE) {
-                    Slog.i(TAG, "!!! animate: entry time=" + mCurrentTime);
-                }
+        synchronized (mService.mWindowMap) {
+            if (!mInitialized) {
+                return;
+            }
 
-                if (SHOW_TRANSACTIONS) Slog.i(TAG, ">>> OPEN TRANSACTION animate");
-                mService.openSurfaceTransaction();
-                transactionOpen = true;
-                SurfaceControl.setAnimationTransaction();
+            // 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;
+            mAppWindowAnimating = false;
+            if (DEBUG_WINDOW_TRACE) {
+                Slog.i(TAG, "!!! animate: entry time=" + mCurrentTime);
+            }
+
+            if (SHOW_TRANSACTIONS) Slog.i(TAG, ">>> OPEN TRANSACTION animate");
+            mService.openSurfaceTransaction();
+            try {
                 final AccessibilityController accessibilityController =
                         mService.mAccessibilityController;
                 final int numDisplays = mDisplayContentsAnimators.size();
@@ -216,27 +227,20 @@
                     mAnimating |= mService.mDragState.stepAnimationLocked(mCurrentTime);
                 }
 
-                if (mAnimating) {
-                    mService.scheduleAnimationLocked();
+                if (!mAnimating) {
+                    cancelAnimation();
                 }
 
                 if (mService.mWatermark != null) {
                     mService.mWatermark.drawIfNeeded();
                 }
-            }
-        } catch (RuntimeException e) {
-            Slog.wtf(TAG, "Unhandled exception in Window Manager", e);
-        } finally {
-            if (transactionOpen) {
-
-                // 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 */);
+            } catch (RuntimeException e) {
+                Slog.wtf(TAG, "Unhandled exception in Window Manager", e);
+            } finally {
+                mService.closeSurfaceTransaction();
                 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 @@
         }
     }
 
+    private void cancelAnimation() {
+        if (mAnimationFrameCallbackScheduled) {
+            mAnimationFrameCallbackScheduled = false;
+            mChoreographer.removeFrameCallback(mAnimationFrameCallback);
+        }
+    }
+
     private class DisplayContentsAnimator {
         ScreenRotationAnimation mScreenRotationAnimation = null;
     }
diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java
index 52d3d95..8ba36d5 100644
--- a/services/core/java/com/android/server/wm/WindowManagerService.java
+++ b/services/core/java/com/android/server/wm/WindowManagerService.java
@@ -908,29 +908,16 @@
         }
     }
 
-    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();
-                }
-            }
-            if (!withLockHeld) {
                 SurfaceControl.closeTransaction();
             }
         } finally {
@@ -938,6 +925,34 @@
         }
     }
 
+    /**
+     * 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);
+        }
+    }
+
     /** Listener to notify activity manager about app transitions. */
     final WindowManagerInternal.AppTransitionListener mActivityManagerAppTransitionNotifier
             = new WindowManagerInternal.AppTransitionListener() {