commit 470df5f0d6a272fd0c92f5967e43af0f29a5c866
author Dominik Laskowski <domlaskowski@google.com> Thu Apr 02 22:27:42 2020 -0700
committer Dominik Laskowski <domlaskowski@google.com> Mon Apr 13 22:15:04 2020 -0700
tree ff9ef7b3d038a5de6f9df9a081a3a4bdac281eca
parent 3e668fef1bc5cb4baeda1122d015a16c64964479

SF: Fix thread safety in ISurfaceComposer APIs

Fix a few cases of ISurfaceComposer functions looking up displays and
calling into HWC without holding mStateLock. Clean up error handling
for consistent messages and accurate status codes.

Bug: 123715322
Test: Boot (no repro, just an audit)
Change-Id: I6432e15c4166b27398137b681b4ea7c056fea5e4

services/surfaceflinger/SurfaceFlinger.h

diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 12efca1..72d1ffe 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -475,14 +475,13 @@
     status_t getCompositionPreference(ui::Dataspace* outDataspace, ui::PixelFormat* outPixelFormat,
                                       ui::Dataspace* outWideColorGamutDataspace,
                                       ui::PixelFormat* outWideColorGamutPixelFormat) const override;
-    status_t getDisplayedContentSamplingAttributes(const sp<IBinder>& display,
+    status_t getDisplayedContentSamplingAttributes(const sp<IBinder>& displayToken,
                                                    ui::PixelFormat* outFormat,
                                                    ui::Dataspace* outDataspace,
                                                    uint8_t* outComponentMask) const override;
-    status_t setDisplayContentSamplingEnabled(const sp<IBinder>& display, bool enable,
-                                              uint8_t componentMask,
-                                              uint64_t maxFrames) const override;
-    status_t getDisplayedContentSample(const sp<IBinder>& display, uint64_t maxFrames,
+    status_t setDisplayContentSamplingEnabled(const sp<IBinder>& displayToken, bool enable,
+                                              uint8_t componentMask, uint64_t maxFrames) override;
+    status_t getDisplayedContentSample(const sp<IBinder>& displayToken, uint64_t maxFrames,
                                        uint64_t timestamp,
                                        DisplayedFrameStats* outStats) const override;
     status_t getProtectedContentSupport(bool* outSupported) const override;
@@ -498,7 +497,7 @@
                                           float* outMaxRefreshRate) override;
     status_t getDisplayBrightnessSupport(const sp<IBinder>& displayToken,
                                          bool* outSupport) const override;
-    status_t setDisplayBrightness(const sp<IBinder>& displayToken, float brightness) const override;
+    status_t setDisplayBrightness(const sp<IBinder>& displayToken, float brightness) override;
     status_t notifyPowerHint(int32_t hintId) override;
     status_t setGlobalShadowSettings(const half4& ambientColor, const half4& spotColor,
                                      float lightPosY, float lightPosZ, float lightRadius) override;
@@ -581,11 +580,6 @@
             const std::optional<scheduler::RefreshRateConfigs::Policy>& policy, bool overridePolicy)
             EXCLUDES(mStateLock);
 
-    // called on the main thread in response to setAutoLowLatencyMode()
-    void setAutoLowLatencyModeInternal(const sp<IBinder>& displayToken, bool on);
-    // called on the main thread in response to setGameContentType()
-    void setGameContentTypeInternal(const sp<IBinder>& displayToken, bool on);
-
     // Returns whether the transaction actually modified any state
     bool handleMessageTransaction();
 
@@ -740,16 +734,6 @@
     // called when starting, or restarting after system_server death
     void initializeDisplays();
 
-    sp<const DisplayDevice> getDisplayDevice(const wp<IBinder>& displayToken) const {
-        Mutex::Autolock _l(mStateLock);
-        return getDisplayDeviceLocked(displayToken);
-    }
-
-    sp<DisplayDevice> getDisplayDevice(const wp<IBinder>& displayToken) {
-        Mutex::Autolock _l(mStateLock);
-        return getDisplayDeviceLocked(displayToken);
-    }
-
     // NOTE: can only be called from the main thread or with mStateLock held
     sp<const DisplayDevice> getDisplayDeviceLocked(const wp<IBinder>& displayToken) const {
         return const_cast<SurfaceFlinger*>(this)->getDisplayDeviceLocked(displayToken);