CaptureLayers: Avoid promoting parent on binder thread

We promote the parent of the root layer when we call getLayerStack.
Since we don't hold an IBinder to the root layers parent, just to
the layer itself, this could create a situation where we are the last
reference to the layer. Layers have to be destroyed on the main
thread and so that would be invalid. Hopefully this is the last
case and now we can start getting rid of refbase for layer.

Bug: 220176775
Bug: 223069308
Bug: 223081111
Test: Existing tests pass
Change-Id: I37a0834ddac6d8e84170674aba0c49268d65fa11
(cherry picked from commit 12a3b9bba87d831f78faabb42523598a8d891ecc)
CRs-Fixed: 3159089

7 files changed, 31 insertions, 24 deletions

diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 512dd6e5d3..a95f9085d7 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -1,4 +1,3 @@
-
 /*
  * Copyright (C) 2007 The Android Open Source Project
  *
diff --git a/services/surfaceflinger/LayerRenderArea.h b/services/surfaceflinger/LayerRenderArea.h
index 6a906944a7..41273e01c1 100644
--- a/services/surfaceflinger/LayerRenderArea.h
+++ b/services/surfaceflinger/LayerRenderArea.h
@@ -46,6 +46,7 @@ public:
     Rect getSourceCrop() const override;
 
     void render(std::function<void()> drawLayers) override;
+    virtual sp<Layer> getParentLayer() const { return mLayer; }
 
 private:
     const sp<Layer> mLayer;
@@ -58,4 +59,4 @@ private:
     const bool mChildrenOnly;
 };
 
-} // namespace android
\ No newline at end of file
+} // namespace android
diff --git a/services/surfaceflinger/RenderArea.h b/services/surfaceflinger/RenderArea.h
index c9f7f46953..387364c03a 100644
--- a/services/surfaceflinger/RenderArea.h
+++ b/services/surfaceflinger/RenderArea.h
@@ -4,6 +4,7 @@
 #include <ui/Transform.h>
 
 #include <functional>
+#include "Layer.h"
 
 namespace android {
 
@@ -85,6 +86,10 @@ public:
     // Returns the source display viewport.
     const Rect& getLayerStackSpaceRect() const { return mLayerStackSpaceRect; }
 
+    // If this is a LayerRenderArea, return the root layer of the
+    // capture operation.
+    virtual sp<Layer> getParentLayer() const { return nullptr; }
+
 protected:
     const bool mAllowSecureLayers;
 
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 0fae1a8479..b527f56463 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -7931,17 +7931,6 @@ status_t SurfaceFlinger::captureLayers(const LayerCaptureArgs& args,
         // and failed if display is not in native mode. This provide a way to force using native
         // colors when capture.
         dataspace = args.dataspace;
-        if (dataspace == ui::Dataspace::UNKNOWN) {
-            auto display = findDisplay(WithLayerStack(parent->getLayerStack()));
-            if (!display) {
-                // If the layer is not on a display, use the dataspace for the default display.
-                display = getDefaultDisplayDeviceLocked();
-            }
-
-            const ui::ColorMode colorMode = display->getCompositionDisplay()->getState().colorMode;
-            dataspace = pickDataspaceFromColorMode(colorMode);
-        }
-
     } // mStateLock
 
     // really small crop or frameScale
@@ -8078,7 +8067,7 @@ status_t SurfaceFlinger::captureScreenCommon(
 
         status_t result = NO_ERROR;
         renderArea->render([&] {
-            result = renderScreenImplLocked(*renderArea, traverseLayers, buffer,
+            result = renderScreenImpl(*renderArea, traverseLayers, buffer,
                                             canCaptureBlackoutContent, regionSampling, grayscale,
                                             captureResults);
         });
@@ -8090,7 +8079,7 @@ status_t SurfaceFlinger::captureScreenCommon(
     return NO_ERROR;
 }
 
-status_t SurfaceFlinger::renderScreenImplLocked(
+status_t SurfaceFlinger::renderScreenImpl(
         const RenderArea& renderArea, TraverseLayersFunction traverseLayers,
         const std::shared_ptr<renderengine::ExternalTexture>& buffer,
         bool canCaptureBlackoutContent, bool regionSampling, bool grayscale,
@@ -8113,7 +8102,20 @@ status_t SurfaceFlinger::renderScreenImplLocked(
     }
 
     captureResults.buffer = buffer->getBuffer();
-    captureResults.capturedDataspace = renderArea.getReqDataSpace();
+    auto dataspace = renderArea.getReqDataSpace();
+    auto parent = renderArea.getParentLayer();
+    if ((dataspace == ui::Dataspace::UNKNOWN) && (parent != nullptr)) {
+        Mutex::Autolock lock(mStateLock);
+        auto display = findDisplay(WithLayerStack(parent->getLayerStack()));
+        if (!display) {
+            // If the layer is not on a display, use the dataspace for the default display.
+            display = getDefaultDisplayDeviceLocked();
+        }
+
+        const ui::ColorMode colorMode = display->getCompositionDisplay()->getState().colorMode;
+        dataspace = pickDataspaceFromColorMode(colorMode);
+    }
+    captureResults.capturedDataspace = dataspace;
 
     const auto reqWidth = renderArea.getReqWidth();
     const auto reqHeight = renderArea.getReqHeight();
@@ -8131,7 +8133,7 @@ status_t SurfaceFlinger::renderScreenImplLocked(
     clientCompositionDisplay.clip = sourceCrop;
     clientCompositionDisplay.orientation = rotation;
 
-    clientCompositionDisplay.outputDataspace = renderArea.getReqDataSpace();
+    clientCompositionDisplay.outputDataspace = dataspace;
     clientCompositionDisplay.maxLuminance = DisplayDevice::sDefaultMaxLumiance;
 
     const float colorSaturation = grayscale ? 0 : 1;
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index dad158d89b..2096e63575 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -1023,10 +1023,10 @@ private:
                                  const std::shared_ptr<renderengine::ExternalTexture>&,
                                  bool regionSampling, bool grayscale,
                                  const sp<IScreenCaptureListener>&);
-    status_t renderScreenImplLocked(const RenderArea&, TraverseLayersFunction,
-                                    const std::shared_ptr<renderengine::ExternalTexture>&,
-                                    bool canCaptureBlackoutContent, bool regionSampling,
-                                    bool grayscale, ScreenCaptureResults&);
+    status_t renderScreenImpl(const RenderArea&, TraverseLayersFunction,
+                              const std::shared_ptr<renderengine::ExternalTexture>&,
+                              bool canCaptureBlackoutContent, bool regionSampling,
+                              bool grayscale, ScreenCaptureResults&) EXCLUDES(mStateLock);
 
 
     bool canAllocateHwcDisplayIdForVDS(uint64_t usage);
diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
index 52a36a2719..fdf445fd93 100644
--- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
@@ -248,7 +248,7 @@ void CompositionTest::captureScreenComposition() {
                                            *mRenderEngine, true);
 
     status_t result =
-            mFlinger.renderScreenImplLocked(*renderArea, traverseLayers, mCaptureScreenBuffer,
+            mFlinger.renderScreenImpl(*renderArea, traverseLayers, mCaptureScreenBuffer,
                                             forSystem, regionSampling);
     EXPECT_EQ(NO_ERROR, result);
 
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index 8656c16198..3e676560c5 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -347,12 +347,12 @@ public:
         return mFlinger->onMessageReceived(what, /*vsyncId=*/0, systemTime());
     }
 
-    auto renderScreenImplLocked(const RenderArea& renderArea,
+    auto renderScreenImpl(const RenderArea& renderArea,
                                 SurfaceFlinger::TraverseLayersFunction traverseLayers,
                                 const std::shared_ptr<renderengine::ExternalTexture>& buffer,
                                 bool forSystem, bool regionSampling) {
         ScreenCaptureResults captureResults;
-        return mFlinger->renderScreenImplLocked(renderArea, traverseLayers, buffer, forSystem,
+        return mFlinger->renderScreenImpl(renderArea, traverseLayers, buffer, forSystem,
                                                 regionSampling, false /* grayscale */,
                                                 captureResults);
     }