commit 3dad52d38519ef4ad6d13095a8fa756bb736ae85
author Marissa Wall <marissaw@google.com> Fri Mar 22 14:03:19 2019 -0700
committer Marissa Wall <marissaw@google.com> Wed Mar 27 09:32:42 2019 -0700
tree 6f95afff00b98cfd8d2ab0f9fbc708dca3525429
parent 78b7220f542826771223f9b5c0b876874dd398d2

blast: transaction callbacks should come in order

Previously, if no SurfaceControls where marked to get callbacks,
a callback was sent directly from SurfaceComposerClient so we could
save a binder call into SurfaceFlinger and a binder call for the
callback.

Although this saved us 2 binder calls, it made the transactions
callbacks come out of order. The public api guarantees that all
callbacks must come in order.

This patch moves the callback from SurfaceComposerClient into
SurfaceFlinger so the callbacks are in order.

Bug: 128519264
Test: SurfaceFlinger_test
Change-Id: Ia1cadb81adb69b58a4d6d43ae453c96a1572f833

libs/gui/SurfaceComposerClient.cpp

diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index 39cd62f..84aed5f 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -381,7 +381,7 @@
     s.state.parentHandleForChild = nullptr;
 
     composerStates.add(s);
-    sf->setTransactionState(composerStates, displayStates, 0, nullptr, {}, -1, {});
+    sf->setTransactionState(composerStates, displayStates, 0, nullptr, {}, -1, {}, {});
 }
 
 void SurfaceComposerClient::doUncacheBufferTransaction(uint64_t cacheId) {
@@ -391,7 +391,7 @@
     uncacheBuffer.token = BufferCache::getInstance().getToken();
     uncacheBuffer.cacheId = cacheId;
 
-    sf->setTransactionState({}, {}, 0, nullptr, {}, -1, uncacheBuffer);
+    sf->setTransactionState({}, {}, 0, nullptr, {}, -1, uncacheBuffer, {});
 }
 
 void SurfaceComposerClient::Transaction::cacheBuffers() {
@@ -434,6 +434,8 @@
 
     sp<ISurfaceComposer> sf(ComposerService::getComposerService());
 
+    std::vector<ListenerCallbacks> listenerCallbacks;
+
     // For every listener with registered callbacks
     for (const auto& [listener, callbackInfo] : mListenerCallbacks) {
         auto& [callbackIds, surfaceControls] = callbackInfo;
@@ -441,11 +443,7 @@
             continue;
         }
 
-        // If the listener does not have any SurfaceControls set on this Transaction, send the
-        // callback now
-        if (surfaceControls.empty()) {
-            listener->onTransactionCompleted(ListenerStats::createEmpty(listener, callbackIds));
-        }
+        listenerCallbacks.emplace_back(listener, std::move(callbackIds));
 
         // If the listener has any SurfaceControls set on this Transaction update the surface state
         for (const auto& surfaceControl : surfaceControls) {
@@ -454,8 +452,8 @@
                 ALOGE("failed to get layer state");
                 continue;
             }
-            s->what |= layer_state_t::eListenerCallbacksChanged;
-            s->listenerCallbacks.emplace_back(listener, std::move(callbackIds));
+            s->what |= layer_state_t::eHasListenerCallbacksChanged;
+            s->hasListenerCallbacks = true;
         }
     }
     mListenerCallbacks.clear();
@@ -494,7 +492,8 @@
     sp<IBinder> applyToken = IInterface::asBinder(TransactionCompletedListener::getIInstance());
     sf->setTransactionState(composerStates, displayStates, flags, applyToken, mInputWindowCommands,
                             mDesiredPresentTime,
-                            {} /*uncacheBuffer - only set in doUncacheBufferTransaction*/);
+                            {} /*uncacheBuffer - only set in doUncacheBufferTransaction*/,
+                            listenerCallbacks);
     mInputWindowCommands.clear();
     mStatus = NO_ERROR;
     return NO_ERROR;