commit 90f669f238cdc750483d0961efc61bbf551ae782
author Chia-I Wu <olv@google.com> Thu Oct 05 14:24:41 2017 -0700
committer Chia-I Wu <olv@google.com> Thu Oct 26 08:56:33 2017 -0700
tree c374182ff98f95e2eccba0d279e94195d8385d10
parent a6c443096cc63c31d920a04a22a9378ca25ae48c

surfaceflinger: make vsync injection more robust

There are more issues than I expected :)

 - no lock to synchronize enable/disable and injection
 - Every time injection is diabled and enabled, a new EventThread is
   created
 - mCallback might be nullptr
 - ENABLE_VSYNC_INJECTIONS/INJECT_VSYNC should require special
   permission
 - MessageQueue::setEventThread must be called from the main thread
 - MessageQueue::setEventThread does not handle EventThread switch
   well

Bug: 65483324
Test: manual
Change-Id: I7d7b98d1f57afc64af0f2065a9bc7c8ad004ca9f

services/surfaceflinger/MessageQueue.cpp

diff --git a/services/surfaceflinger/MessageQueue.cpp b/services/surfaceflinger/MessageQueue.cpp
index bca3430..0b1199c 100644
--- a/services/surfaceflinger/MessageQueue.cpp
+++ b/services/surfaceflinger/MessageQueue.cpp
@@ -91,6 +91,14 @@
 
 void MessageQueue::setEventThread(const sp<EventThread>& eventThread)
 {
+    if (mEventThread == eventThread) {
+        return;
+    }
+
+    if (mEventTube.getFd() >= 0) {
+        mLooper->removeFd(mEventTube.getFd());
+    }
+
     mEventThread = eventThread;
     mEvents = eventThread->createEventConnection();
     mEvents->stealReceiveChannel(&mEventTube);

services/surfaceflinger/SurfaceFlinger.cpp

diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 54c040d..05c76fc 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -570,7 +570,9 @@
 
     virtual void onInjectSyncEvent(nsecs_t when) {
         std::lock_guard<std::mutex> lock(mCallbackMutex);
-        mCallback->onVSyncEvent(when);
+        if (mCallback) {
+            mCallback->onVSyncEvent(when);
+        }
     }
 
     virtual void setVSyncEnabled(bool) {}
@@ -1066,28 +1068,34 @@
 }
 
 status_t SurfaceFlinger::enableVSyncInjections(bool enable) {
-    if (enable == mInjectVSyncs) {
-        return NO_ERROR;
-    }
+    sp<LambdaMessage> enableVSyncInjections = new LambdaMessage([&]() {
+        Mutex::Autolock _l(mStateLock);
 
-    if (enable) {
-        mInjectVSyncs = enable;
-        ALOGV("VSync Injections enabled");
-        if (mVSyncInjector.get() == nullptr) {
-            mVSyncInjector = new InjectVSyncSource();
-            mInjectorEventThread = new EventThread(mVSyncInjector, *this, false);
+        if (mInjectVSyncs == enable) {
+            return;
         }
-        mEventQueue.setEventThread(mInjectorEventThread);
-    } else {
+
+        if (enable) {
+            ALOGV("VSync Injections enabled");
+            if (mVSyncInjector.get() == nullptr) {
+                mVSyncInjector = new InjectVSyncSource();
+                mInjectorEventThread = new EventThread(mVSyncInjector, *this, false);
+            }
+            mEventQueue.setEventThread(mInjectorEventThread);
+        } else {
+            ALOGV("VSync Injections disabled");
+            mEventQueue.setEventThread(mSFEventThread);
+        }
+
         mInjectVSyncs = enable;
-        ALOGV("VSync Injections disabled");
-        mEventQueue.setEventThread(mSFEventThread);
-        mVSyncInjector.clear();
-    }
+    });
+    postMessageSync(enableVSyncInjections);
     return NO_ERROR;
 }
 
 status_t SurfaceFlinger::injectVSync(nsecs_t when) {
+    Mutex::Autolock _l(mStateLock);
+
     if (!mInjectVSyncs) {
         ALOGE("VSync Injections not enabled");
         return BAD_VALUE;
@@ -3983,6 +3991,8 @@
         case GET_ANIMATION_FRAME_STATS:
         case SET_POWER_MODE:
         case GET_HDR_CAPABILITIES:
+        case ENABLE_VSYNC_INJECTIONS:
+        case INJECT_VSYNC:
         {
             // codes that require permission check
             IPCThreadState* ipc = IPCThreadState::self();