commit efb71afcbb5d8f57af6435391f5d4835696375cc
author Marissa Wall <marissaw@google.com> Thu Jun 27 14:45:53 2019 -0700
committer Marissa Wall <marissaw@google.com> Wed Jul 10 11:04:40 2019 -0700
tree 2d2ded52b05cc34c3717366279983b32991e593c
parent 7507a128ab55f325d11ccf258e4788fdaef4fbb6

blast: fix registering callbacks

This is a better fix for b/134194071 than ag/7998544. The previous
patch only protected the call to notify the binder thread to send a
callback.

This patch has both a start registration and end registration call.
During that time, the transaction callback cannot be sent. This is
closer to a long term fix for the bug.

Bug: 134194071
Test: Switch between front and back cameras to make sure the app
    doesn't crash.

Change-Id: I2d20c13cc1c8d13e5a1340dfaa8cbbaa4d3a30ab

services/surfaceflinger/SurfaceFlinger.cpp

diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index f53d3fa..7d8630f 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2183,14 +2183,7 @@
     }
 
     mTransactionCompletedThread.addPresentFence(mPreviousPresentFences[0]);
-
-    // Lock the mStateLock in case SurfaceFlinger is in the middle of applying a transaction.
-    // If we do not lock here, a callback could be sent without all of its SurfaceControls and
-    // metrics.
-    {
-        Mutex::Autolock _l(mStateLock);
-        mTransactionCompletedThread.sendCallbacks();
-    }
+    mTransactionCompletedThread.sendCallbacks();
 
     if (mLumaSampling && mRegionSamplingThread) {
         mRegionSamplingThread->notifyNewContent();
@@ -3790,8 +3783,8 @@
     if (!listenerCallbacks.empty()) {
         mTransactionCompletedThread.run();
     }
-    for (const auto& [listener, callbackIds] : listenerCallbacks) {
-        mTransactionCompletedThread.addCallback(listener, callbackIds);
+    for (const auto& listenerCallback : listenerCallbacks) {
+        mTransactionCompletedThread.startRegistration(listenerCallback);
     }
 
     uint32_t clientStateFlags = 0;
@@ -3800,6 +3793,10 @@
                                                  postTime, privileged);
     }
 
+    for (const auto& listenerCallback : listenerCallbacks) {
+        mTransactionCompletedThread.endRegistration(listenerCallback);
+    }
+
     // If the state doesn't require a traversal and there are callbacks, send them now
     if (!(clientStateFlags & eTraversalNeeded) && !listenerCallbacks.empty()) {
         mTransactionCompletedThread.sendCallbacks();
@@ -3937,7 +3934,7 @@
     sp<Layer> layer(client->getLayerUser(s.surface));
     if (layer == nullptr) {
         for (auto& listenerCallback : listenerCallbacks) {
-            mTransactionCompletedThread.addUnpresentedCallbackHandle(
+            mTransactionCompletedThread.registerUnpresentedCallbackHandle(
                     new CallbackHandle(listenerCallback.transactionCompletedListener,
                                        listenerCallback.callbackIds, s.surface));
         }