commit 6200eacdc927776483d775562db11cce284cc7e0
author Chia-I Wu <olv@google.com> Thu Oct 05 14:24:41 2017 -0700
committer Chia-I Wu <olv@google.com> Fri Oct 27 11:01:34 2017 -0700
tree 423759657354625c41872525fa093adab8cbb4a6
parent 6afce442eadb3b26d582815e7e14c93e3f487068

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
Merged-In: I7d7b98d1f57afc64af0f2065a9bc7c8ad004ca9f
Change-Id: I7d7b98d1f57afc64af0f2065a9bc7c8ad004ca9f

services/surfaceflinger/SurfaceFlinger.cpp

diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 29e7bd6..8a8fa01 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -515,7 +515,9 @@
 
     virtual void onInjectSyncEvent(nsecs_t when) {
         std::lock_guard<std::mutex> lock(mCallbackMutex);
-        mCallback->onVSyncEvent(when);
+        if (mCallback != nullptr) {
+            mCallback->onVSyncEvent(when);
+        }
     }
 
     virtual void setVSyncEnabled(bool) {}
@@ -988,13 +990,14 @@
     return NO_ERROR;
 }
 
-status_t SurfaceFlinger::enableVSyncInjections(bool enable) {
-    if (enable == mInjectVSyncs) {
-        return NO_ERROR;
+void SurfaceFlinger::enableVSyncInjectionsInternal(bool enable) {
+    Mutex::Autolock _l(mStateLock);
+
+    if (mInjectVSyncs == enable) {
+        return;
     }
 
     if (enable) {
-        mInjectVSyncs = enable;
         ALOGV("VSync Injections enabled");
         if (mVSyncInjector.get() == nullptr) {
             mVSyncInjector = new InjectVSyncSource();
@@ -1002,15 +1005,33 @@
         }
         mEventQueue.setEventThread(mInjectorEventThread);
     } else {
-        mInjectVSyncs = enable;
         ALOGV("VSync Injections disabled");
         mEventQueue.setEventThread(mSFEventThread);
-        mVSyncInjector.clear();
     }
+
+    mInjectVSyncs = enable;
+}
+
+status_t SurfaceFlinger::enableVSyncInjections(bool enable) {
+    class MessageEnableVSyncInjections : public MessageBase {
+        SurfaceFlinger* mFlinger;
+        bool mEnable;
+    public:
+        MessageEnableVSyncInjections(SurfaceFlinger* flinger, bool enable)
+            : mFlinger(flinger), mEnable(enable) { }
+        virtual bool handler() {
+            mFlinger->enableVSyncInjectionsInternal(mEnable);
+            return true;
+        }
+    };
+    sp<MessageBase> msg = new MessageEnableVSyncInjections(this, enable);
+    postMessageSync(msg);
     return NO_ERROR;
 }
 
 status_t SurfaceFlinger::injectVSync(nsecs_t when) {
+    Mutex::Autolock _l(mStateLock);
+
     if (!mInjectVSyncs) {
         ALOGE("VSync Injections not enabled");
         return BAD_VALUE;
@@ -3757,6 +3778,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();