Revert "Avoid a potential race condition on mDisplays"

This reverts commit 8d6c16dc3dc8b88a0046f53668a4e3be074507ff.

Bug: b/37282502
Change-Id: Ibf64607f9e14ede201510e2c1b502c49a31e9f2a

diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
index 32739b6..09434f6 100644 (file)
--- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp
@@ -206,7 +206,7 @@ void HWComposer::hotplug(const std::shared_ptr<HWC2::Display>& display,
         }
         disp = DisplayDevice::DISPLAY_EXTERNAL;
     }
-    mEventHandler->onHotplugReceived(this, disp,
+    mEventHandler->onHotplugReceived(disp,
             connected == HWC2::Connection::Connected);
 }
 

diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.h b/services/surfaceflinger/DisplayHardware/HWComposer.h
index 78d0307..81f1619 100644 (file)
--- a/services/surfaceflinger/DisplayHardware/HWComposer.h
+++ b/services/surfaceflinger/DisplayHardware/HWComposer.h
@@ -70,7 +70,7 @@ public:
         friend class HWComposer;
         virtual void onVSyncReceived(
             HWComposer* composer, int32_t disp, nsecs_t timestamp) = 0;
-        virtual void onHotplugReceived(HWComposer* composer, int32_t disp, bool connected) = 0;
+        virtual void onHotplugReceived(int32_t disp, bool connected) = 0;
         virtual void onInvalidateReceived(HWComposer* composer) = 0;
     protected:
         virtual ~EventHandler() {}

diff --git a/services/surfaceflinger/DisplayHardware/HWComposer_hwc1.cpp b/services/surfaceflinger/DisplayHardware/HWComposer_hwc1.cpp
index dcb2913..5b5f1cf 100644 (file)
--- a/services/surfaceflinger/DisplayHardware/HWComposer_hwc1.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWComposer_hwc1.cpp
@@ -315,7 +315,7 @@ void HWComposer::hotplug(int disp, int connected) {
     queryDisplayProperties(disp);
     // Do not teardown or recreate the primary display
     if (disp != HWC_DISPLAY_PRIMARY) {
-        mEventHandler.onHotplugReceived(this, disp, bool(connected));
+        mEventHandler.onHotplugReceived(disp, bool(connected));
     }
 }
 

diff --git a/services/surfaceflinger/DisplayHardware/HWComposer_hwc1.h b/services/surfaceflinger/DisplayHardware/HWComposer_hwc1.h
index 4bc63bb..f64d69a 100644 (file)
--- a/services/surfaceflinger/DisplayHardware/HWComposer_hwc1.h
+++ b/services/surfaceflinger/DisplayHardware/HWComposer_hwc1.h
@@ -62,7 +62,7 @@ public:
         friend class HWComposer;
         virtual void onVSyncReceived(
             HWComposer* composer, int32_t disp, nsecs_t timestamp) = 0;
-        virtual void onHotplugReceived(HWComposer* composer, int disp, bool connected) = 0;
+        virtual void onHotplugReceived(int disp, bool connected) = 0;
         virtual void onInvalidateReceived(HWComposer* composer) = 0;
     protected:
         virtual ~EventHandler() {}

diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 76a5d06..26baaae 100644 (file)
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -598,7 +598,7 @@ void SurfaceFlinger::init() {
 
     // make the GLContext current so that we can create textures when creating
     // Layers (which may happens before we render something)
-    getDefaultDisplayDeviceLocked()->makeCurrent(mEGLDisplay, mEGLContext);
+    getDefaultDisplayDevice()->makeCurrent(mEGLDisplay, mEGLContext);
 
     mEventControlThread = new EventControlThread(this);
     mEventControlThread->run("EventControl", PRIORITY_URGENT_DISPLAY);
@@ -714,11 +714,8 @@ status_t SurfaceFlinger::getDisplayConfigs(const sp<IBinder>& display,
             info.density = density;
 
             // TODO: this needs to go away (currently needed only by webkit)
-            {
-                Mutex::Autolock _l(mStateLock);
-                sp<const DisplayDevice> hw(getDefaultDisplayDeviceLocked());
-                info.orientation = hw->getOrientation();
-            }
+            sp<const DisplayDevice> hw(getDefaultDisplayDevice());
+            info.orientation = hw->getOrientation();
         } else {
             // TODO: where should this value come from?
             static const int TV_DENSITY = 213;
@@ -775,13 +772,10 @@ int SurfaceFlinger::getActiveConfig(const sp<IBinder>& display) {
         ALOGE("%s : display is NULL", __func__);
         return BAD_VALUE;
     }
-
-    Mutex::Autolock _l(mStateLock);
     sp<DisplayDevice> device(getDisplayDevice(display));
     if (device != NULL) {
         return device->getActiveConfig();
     }
-
     return BAD_VALUE;
 }
 
@@ -869,7 +863,6 @@ status_t SurfaceFlinger::getDisplayColorModes(const sp<IBinder>& display,
 }
 
 android_color_mode_t SurfaceFlinger::getActiveColorMode(const sp<IBinder>& display) {
-    Mutex::Autolock _l(mStateLock);
     sp<DisplayDevice> device(getDisplayDevice(display));
     if (device != nullptr) {
         return device->getActiveColorMode();
@@ -1131,60 +1124,55 @@ void SurfaceFlinger::getCompositorTiming(CompositorTiming* compositorTiming) {
     *compositorTiming = mCompositorTiming;
 }
 
-void SurfaceFlinger::createDefaultDisplayDevice() {
-    const int32_t type = DisplayDevice::DISPLAY_PRIMARY;
-    wp<IBinder> token = mBuiltinDisplays[type];
+void SurfaceFlinger::onHotplugReceived(int32_t disp, bool connected) {
+    ALOGV("onHotplugReceived(%d, %s)", disp, connected ? "true" : "false");
+    if (disp == DisplayDevice::DISPLAY_PRIMARY) {
+        Mutex::Autolock lock(mStateLock);
 
-    // All non-virtual displays are currently considered secure.
-    const bool isSecure = true;
-
-    sp<IGraphicBufferProducer> producer;
-    sp<IGraphicBufferConsumer> consumer;
-    BufferQueue::createBufferQueue(&producer, &consumer, new GraphicBufferAlloc());
-
-    sp<FramebufferSurface> fbs = new FramebufferSurface(*mHwc, type, consumer);
-
-    bool hasWideColorModes = false;
-    std::vector<android_color_mode_t> modes = getHwComposer().getColorModes(
-        type);
-    for (android_color_mode_t colorMode : modes) {
-        switch (colorMode) {
-            case HAL_COLOR_MODE_DISPLAY_P3:
-            case HAL_COLOR_MODE_ADOBE_RGB:
-            case HAL_COLOR_MODE_DCI_P3:
-                hasWideColorModes = true;
-                break;
-            default:
-                break;
-        }
-    }
-    sp<DisplayDevice> hw = new DisplayDevice(this, DisplayDevice::DISPLAY_PRIMARY, type, isSecure,
-                                             token, fbs, producer, mRenderEngine->getEGLConfig(),
-                                             hasWideColorModes && hasWideColorDisplay);
-    mDisplays.add(token, hw);
-    android_color_mode defaultColorMode = HAL_COLOR_MODE_NATIVE;
-    if (hasWideColorModes && hasWideColorDisplay) {
-        defaultColorMode = HAL_COLOR_MODE_SRGB;
-    }
-    setActiveColorModeInternal(hw, defaultColorMode);
-}
+        // All non-virtual displays are currently considered secure.
+        bool isSecure = true;
 
-void SurfaceFlinger::onHotplugReceived(HWComposer* composer, int32_t disp, bool connected) {
-    ALOGV("onHotplugReceived(%d, %s)", disp, connected ? "true" : "false");
+        int32_t type = DisplayDevice::DISPLAY_PRIMARY;
 
-    if (composer->isUsingVrComposer()) {
-        // We handle initializing the primary display device for the VR
-        // window manager hwc explicitly at the time of transition.
-        if (disp != DisplayDevice::DISPLAY_PRIMARY) {
-            ALOGE("External displays are not supported by the vr hardware composer.");
+        // When we're using the vr composer, the assumption is that we've
+        // already created the IBinder object for the primary display.
+        if (!mHwc->isUsingVrComposer()) {
+            createBuiltinDisplayLocked(DisplayDevice::DISPLAY_PRIMARY);
         }
-        return;
-    }
 
-    if (disp == DisplayDevice::DISPLAY_PRIMARY) {
-        Mutex::Autolock lock(mStateLock);
-        createBuiltinDisplayLocked(DisplayDevice::DISPLAY_PRIMARY);
-        createDefaultDisplayDevice();
+        wp<IBinder> token = mBuiltinDisplays[type];
+
+        sp<IGraphicBufferProducer> producer;
+        sp<IGraphicBufferConsumer> consumer;
+        BufferQueue::createBufferQueue(&producer, &consumer,
+                new GraphicBufferAlloc());
+
+        sp<FramebufferSurface> fbs = new FramebufferSurface(*mHwc,
+                DisplayDevice::DISPLAY_PRIMARY, consumer);
+
+        bool hasWideColorModes = false;
+        std::vector<android_color_mode_t> modes = getHwComposer().getColorModes(type);
+        for (android_color_mode_t colorMode : modes) {
+            switch (colorMode) {
+                case HAL_COLOR_MODE_DISPLAY_P3:
+                case HAL_COLOR_MODE_ADOBE_RGB:
+                case HAL_COLOR_MODE_DCI_P3:
+                    hasWideColorModes = true;
+                    break;
+                default:
+                    break;
+            }
+        }
+        sp<DisplayDevice> hw =
+                new DisplayDevice(this, DisplayDevice::DISPLAY_PRIMARY, disp, isSecure, token, fbs,
+                                  producer, mRenderEngine->getEGLConfig(),
+                                  hasWideColorModes && hasWideColorDisplay);
+        mDisplays.add(token, hw);
+        android_color_mode defaultColorMode = HAL_COLOR_MODE_NATIVE;
+        if (hasWideColorModes && hasWideColorDisplay) {
+            defaultColorMode = HAL_COLOR_MODE_SRGB;
+        }
+        setActiveColorModeInternal(hw, defaultColorMode);
     } else {
         auto type = DisplayDevice::DISPLAY_EXTERNAL;
         Mutex::Autolock _l(mStateLock);
@@ -1226,7 +1214,6 @@ void SurfaceFlinger::clearHwcLayers(const LayerVector& layers) {
     }
 }
 
-// Note: it is assumed the caller holds |mStateLock| when this is called
 void SurfaceFlinger::resetHwc() {
     disableHardwareVsync(true);
     clearHwcLayers(mDrawingState.layersSortedByZ);
@@ -1247,46 +1234,36 @@ void SurfaceFlinger::updateVrFlinger() {
     if (vrFlingerRequestsDisplay == mHwc->isUsingVrComposer()) {
         return;
     }
-
-    bool vrHwcNewlyInitialized = false;
-
     if (vrFlingerRequestsDisplay && !mVrHwc) {
         // Construct new HWComposer without holding any locks.
         mVrHwc = new HWComposer(true);
-        vrHwcNewlyInitialized = true;
         ALOGV("Vr HWC created");
     }
+    {
+        Mutex::Autolock _l(mStateLock);
 
-    Mutex::Autolock _l(mStateLock);
+        if (vrFlingerRequestsDisplay) {
+            resetHwc();
 
-    if (vrFlingerRequestsDisplay) {
-        resetHwc();
+            mHwc = mVrHwc;
+            mVrFlinger->GrantDisplayOwnership();
+        } else {
+            mVrFlinger->SeizeDisplayOwnership();
 
-        mHwc = mVrHwc;
-        mVrFlinger->GrantDisplayOwnership();
+            resetHwc();
 
-        if (vrHwcNewlyInitialized) {
-            mVrHwc->setEventHandler(
-                static_cast<HWComposer::EventHandler*>(this));
+            mHwc = mRealHwc;
+            enableHardwareVsync();
         }
-    } else {
-        mVrFlinger->SeizeDisplayOwnership();
-
-        resetHwc();
 
-        mHwc = mRealHwc;
-        enableHardwareVsync();
+        mVisibleRegionsDirty = true;
+        invalidateHwcGeometry();
+        android_atomic_or(1, &mRepaintEverything);
+        setTransactionFlags(eDisplayTransactionNeeded);
+    }
+    if (mVrHwc) {
+        mVrHwc->setEventHandler(static_cast<HWComposer::EventHandler*>(this));
     }
-
-    mVisibleRegionsDirty = true;
-    invalidateHwcGeometry();
-
-    // Explicitly re-initialize the primary display. This is because some other
-    // parts of this class rely on the primary display always being available.
-    createDefaultDisplayDevice();
-
-    android_atomic_or(1, &mRepaintEverything);
-    setTransactionFlags(eDisplayTransactionNeeded);
 }
 
 void SurfaceFlinger::onMessageReceived(int32_t what) {
@@ -1496,8 +1473,7 @@ void SurfaceFlinger::postComposition(nsecs_t refreshStartTime)
         layer->releasePendingBuffer(dequeueReadyTime);
     }
 
-    // |mStateLock| not needed as we are on the main thread
-    const sp<const DisplayDevice> hw(getDefaultDisplayDeviceLocked());
+    const sp<const DisplayDevice> hw(getDefaultDisplayDevice());
 
     std::shared_ptr<FenceTime> glCompositionDoneFenceTime;
     if (mHwc->hasClientComposition(HWC_DISPLAY_PRIMARY)) {
@@ -1845,8 +1821,7 @@ void SurfaceFlinger::postFramebuffer()
     mLastSwapBufferTime = systemTime() - now;
     mDebugInSwapBuffers = 0;
 
-    // |mStateLock| not needed as we are on the main thread
-    uint32_t flipCount = getDefaultDisplayDeviceLocked()->getPageFlipCount();
+    uint32_t flipCount = getDefaultDisplayDevice()->getPageFlipCount();
     if (flipCount % LOG_FRAME_STATS_PERIOD == 0) {
         logFrameStats();
     }
@@ -1931,7 +1906,7 @@ void SurfaceFlinger::handleTransactionLocked(uint32_t transactionFlags)
                         // Call makeCurrent() on the primary display so we can
                         // be sure that nothing associated with this display
                         // is current.
-                        const sp<const DisplayDevice> defaultDisplay(getDefaultDisplayDeviceLocked());
+                        const sp<const DisplayDevice> defaultDisplay(getDefaultDisplayDevice());
                         defaultDisplay->makeCurrent(mEGLDisplay, mEGLContext);
                         sp<DisplayDevice> hw(getDisplayDevice(draw.keyAt(i)));
                         if (hw != NULL)
@@ -2121,7 +2096,7 @@ void SurfaceFlinger::handleTransactionLocked(uint32_t transactionFlags)
                 // could be null when this layer is using a layerStack
                 // that is not visible on any display. Also can occur at
                 // screen off/on times.
-                disp = getDefaultDisplayDeviceLocked();
+                disp = getDefaultDisplayDevice();
             }
             layer->updateTransformHint(disp);
 
@@ -2477,9 +2452,7 @@ bool SurfaceFlinger::doComposeSurfaces(
             ALOGW("DisplayDevice::makeCurrent failed. Aborting surface composition for display %s",
                   displayDevice->getDisplayName().string());
             eglMakeCurrent(mEGLDisplay, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT);
-
-            // |mStateLock| not needed as we are on the main thread
-            if(!getDefaultDisplayDeviceLocked()->makeCurrent(mEGLDisplay, mEGLContext)) {
+            if(!getDefaultDisplayDevice()->makeCurrent(mEGLDisplay, mEGLContext)) {
               ALOGE("DisplayDevice::makeCurrent on default display failed. Aborting.");
             }
             return false;
@@ -3566,7 +3539,7 @@ void SurfaceFlinger::dumpAllLocked(const Vector<String16>& args, size_t& index,
     colorizer.reset(result);
 
     HWComposer& hwc(getHwComposer());
-    sp<const DisplayDevice> hw(getDefaultDisplayDeviceLocked());
+    sp<const DisplayDevice> hw(getDefaultDisplayDevice());
 
     colorizer.bold(result);
     result.appendFormat("EGL implementation : %s\n",
@@ -3796,7 +3769,7 @@ status_t SurfaceFlinger::onTransact(
                 return NO_ERROR;
             case 1013: {
                 Mutex::Autolock _l(mStateLock);
-                sp<const DisplayDevice> hw(getDefaultDisplayDeviceLocked());
+                sp<const DisplayDevice> hw(getDefaultDisplayDevice());
                 reply->writeInt32(hw->getPageFlipCount());
                 return NO_ERROR;
             }

diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 219d662..46121cf 100644 (file)
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -184,9 +184,8 @@ public:
     void repaintEverything();
 
     // returns the default Display
-    sp<const DisplayDevice> getDefaultDisplayDevice() {
-        Mutex::Autolock _l(mStateLock);
-        return getDefaultDisplayDeviceLocked();
+    sp<const DisplayDevice> getDefaultDisplayDevice() const {
+        return getDisplayDevice(mBuiltinDisplays[DisplayDevice::DISPLAY_PRIMARY]);
     }
 
     // utility function to delete a texture on the main thread
@@ -305,7 +304,7 @@ private:
      * HWComposer::EventHandler interface
      */
     virtual void onVSyncReceived(HWComposer* composer, int type, nsecs_t timestamp);
-    virtual void onHotplugReceived(HWComposer* composer, int disp, bool connected);
+    virtual void onHotplugReceived(int disp, bool connected);
     virtual void onInvalidateReceived(HWComposer* composer);
 
     /* ------------------------------------------------------------------------
@@ -440,12 +439,6 @@ private:
         return mDisplays.valueFor(dpy);
     }
 
-    sp<const DisplayDevice> getDefaultDisplayDeviceLocked() const {
-        return getDisplayDevice(mBuiltinDisplays[DisplayDevice::DISPLAY_PRIMARY]);
-    }
-
-    void createDefaultDisplayDevice();
-
     int32_t getDisplayType(const sp<IBinder>& display) {
         if (!display.get()) return NAME_NOT_FOUND;
         for (int i = 0; i < DisplayDevice::NUM_BUILTIN_DISPLAY_TYPES; ++i) {

diff --git a/services/surfaceflinger/SurfaceFlinger_hwc1.cpp b/services/surfaceflinger/SurfaceFlinger_hwc1.cpp
index cd2acef..e640ef7 100644 (file)
--- a/services/surfaceflinger/SurfaceFlinger_hwc1.cpp
+++ b/services/surfaceflinger/SurfaceFlinger_hwc1.cpp
@@ -584,7 +584,7 @@ void SurfaceFlinger::init() {
 
     // make the GLContext current so that we can create textures when creating Layers
     // (which may happens before we render something)
-    getDefaultDisplayDeviceLocked()->makeCurrent(mEGLDisplay, mEGLContext);
+    getDefaultDisplayDevice()->makeCurrent(mEGLDisplay, mEGLContext);
 
     mEventControlThread = new EventControlThread(this);
     mEventControlThread->run("EventControl", PRIORITY_URGENT_DISPLAY);
@@ -1040,7 +1040,7 @@ void SurfaceFlinger::getCompositorTiming(CompositorTiming* compositorTiming) {
     *compositorTiming = mCompositorTiming;
 }
 
-void SurfaceFlinger::onHotplugReceived(HWComposer* /*composer*/, int type, bool connected) {
+void SurfaceFlinger::onHotplugReceived(int type, bool connected) {
     if (mEventThread == NULL) {
         // This is a temporary workaround for b/7145521.  A non-null pointer
         // does not mean EventThread has finished initializing, so this
@@ -3222,7 +3222,7 @@ void SurfaceFlinger::dumpAllLocked(const Vector<String16>& args, size_t& index,
     colorizer.reset(result);
 
     HWComposer& hwc(getHwComposer());
-    sp<const DisplayDevice> hw(getDefaultDisplayDeviceLocked());
+    sp<const DisplayDevice> hw(getDefaultDisplayDevice());
 
     colorizer.bold(result);
     result.appendFormat("EGL implementation : %s\n",