fix another bug where screenshots could end-up all black

SF transactions were always handled on VSYNC which allowed
the screenshot to sneak-in between closing the transaction
and vsync (before it's latched), resulting in a screenshot
with the previous state.

we now always force transactions to happen immediately
before screenhots.

Bug: 7552304
Change-Id: I0afc86b7e8366173daff5b9988bbb4d2a0f43860

3 files changed, 37 insertions, 4 deletions

diff --git a/services/surfaceflinger/MessageQueue.cpp b/services/surfaceflinger/MessageQueue.cpp
index 3f77f74..c9c7b96 100644
--- a/services/surfaceflinger/MessageQueue.cpp
+++ b/services/surfaceflinger/MessageQueue.cpp
@@ -61,6 +61,12 @@ void MessageQueue::Handler::dispatchInvalidate() {
     }
 }
 
+void MessageQueue::Handler::dispatchTransaction() {
+    if ((android_atomic_or(eventMaskTransaction, &mEventMask) & eventMaskTransaction) == 0) {
+        mQueue.mLooper->sendMessage(this, Message(MessageQueue::TRANSACTION));
+    }
+}
+
 void MessageQueue::Handler::handleMessage(const Message& message) {
     switch (message.what) {
         case INVALIDATE:
@@ -71,6 +77,10 @@ void MessageQueue::Handler::handleMessage(const Message& message) {
             android_atomic_and(~eventMaskRefresh, &mEventMask);
             mQueue.mFlinger->onMessageReceived(message.what);
             break;
+        case TRANSACTION:
+            android_atomic_and(~eventMaskTransaction, &mEventMask);
+            mQueue.mFlinger->onMessageReceived(message.what);
+            break;
     }
 }
 
@@ -132,6 +142,7 @@ status_t MessageQueue::postMessage(
     return NO_ERROR;
 }
 
+
 /* when INVALIDATE_ON_VSYNC is set SF only processes
  * buffer updates on VSYNC and performs a refresh immediately
  * after.
@@ -143,6 +154,10 @@ status_t MessageQueue::postMessage(
  */
 #define INVALIDATE_ON_VSYNC 1
 
+void MessageQueue::invalidateTransactionNow() {
+    mHandler->dispatchTransaction();
+}
+
 void MessageQueue::invalidate() {
 #if INVALIDATE_ON_VSYNC
     mEvents->requestNextVsync();
diff --git a/services/surfaceflinger/MessageQueue.h b/services/surfaceflinger/MessageQueue.h
index 710b2c2..b77e08e 100644
--- a/services/surfaceflinger/MessageQueue.h
+++ b/services/surfaceflinger/MessageQueue.h
@@ -62,8 +62,9 @@ private:
 class MessageQueue {
     class Handler : public MessageHandler {
         enum {
-            eventMaskInvalidate = 0x1,
-            eventMaskRefresh    = 0x2
+            eventMaskInvalidate     = 0x1,
+            eventMaskRefresh        = 0x2,
+            eventMaskTransaction    = 0x4
         };
         MessageQueue& mQueue;
         int32_t mEventMask;
@@ -72,6 +73,7 @@ class MessageQueue {
         virtual void handleMessage(const Message& message);
         void dispatchRefresh();
         void dispatchInvalidate();
+        void dispatchTransaction();
     };
 
     friend class Handler;
@@ -89,8 +91,9 @@ class MessageQueue {
 
 public:
     enum {
-        INVALIDATE = 0,
-        REFRESH    = 1,
+        INVALIDATE  = 0,
+        REFRESH     = 1,
+        TRANSACTION = 2
     };
 
     MessageQueue();
@@ -100,8 +103,13 @@ public:
 
     void waitMessage();
     status_t postMessage(const sp<MessageBase>& message, nsecs_t reltime=0);
+
+    // sends INVALIDATE message at next VSYNC
     void invalidate();
+    // sends REFRESH message at next VSYNC
     void refresh();
+    // sends TRANSACTION message immediately
+    void invalidateTransactionNow();
 };
 
 // ---------------------------------------------------------------------------
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index a4426cd..32b97eb 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -744,6 +744,9 @@ void SurfaceFlinger::eventControl(int disp, int event, int enabled) {
 void SurfaceFlinger::onMessageReceived(int32_t what) {
     ATRACE_CALL();
     switch (what) {
+    case MessageQueue::TRANSACTION:
+        handleMessageTransaction();
+        break;
     case MessageQueue::INVALIDATE:
         handleMessageTransaction();
         handleMessageInvalidate();
@@ -2626,6 +2629,13 @@ status_t SurfaceFlinger::captureScreen(const sp<IBinder>& display,
         }
     };
 
+    // make sure to process transactions before screenshots -- a transaction
+    // might already be pending but scheduled for VSYNC; this guarantees we
+    // will handle it before the screenshot. When VSYNC finally arrives
+    // the scheduled transaction will be a no-op. If no transactions are
+    // scheduled at this time, this will end-up being a no-op as well.
+    mEventQueue.invalidateTransactionNow();
+
     sp<MessageBase> msg = new MessageCaptureScreen(this,
             display, producer, reqWidth, reqHeight, minLayerZ, maxLayerZ,
             isCpuConsumer);