Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 12a3b9bb authored by Robert Carr's avatar Robert Carr
Browse files

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
parent 70fa373d

services/surfaceflinger/Layer.h

+0 −1
Original line number Diff line number Diff line 


/*

 * Copyright (C) 2007 The Android Open Source Project

 *

services/surfaceflinger/LayerRenderArea.h

+2 −1
Original line number Diff line number Diff line
@@ -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;

services/surfaceflinger/RenderArea.h

+5 −0
Original line number Diff line number Diff line
@@ -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;

services/surfaceflinger/SurfaceFlinger.cpp

+19 −17
Original line number Diff line number Diff line
@@ -6499,19 +6499,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([layerStack = parent->getLayerStack()](const auto& display) {

                return display.getLayerStack() == layerStack;

            });

            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
@@ -6640,7 +6627,7 @@ std::shared_future<renderengine::RenderEngineResult> SurfaceFlinger::captureScre 


        renderArea->render([&] {

            renderEngineResultFuture =

                    renderScreenImplLocked(*renderArea, traverseLayers, buffer,

                    renderScreenImpl(*renderArea, traverseLayers, buffer,

                                           canCaptureBlackoutContent, regionSampling, grayscale,

                                           captureResults);

        });
@@ -6673,7 +6660,7 @@ std::shared_future<renderengine::RenderEngineResult> SurfaceFlinger::captureScre 
    }

}



std::shared_future<renderengine::RenderEngineResult> SurfaceFlinger::renderScreenImplLocked(

std::shared_future<renderengine::RenderEngineResult> SurfaceFlinger::renderScreenImpl(

        const RenderArea& renderArea, TraverseLayersFunction traverseLayers,

        const std::shared_ptr<renderengine::ExternalTexture>& buffer,

        bool canCaptureBlackoutContent, bool regionSampling, bool grayscale,
@@ -6697,7 +6684,22 @@ std::shared_future<renderengine::RenderEngineResult> SurfaceFlinger::renderScree 
    }



    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([layerStack = parent->getLayerStack()](const auto& display) {

            return display.getLayerStack() == layerStack;

        });

        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();
@@ -6715,7 +6717,7 @@ std::shared_future<renderengine::RenderEngineResult> SurfaceFlinger::renderScree 
    clientCompositionDisplay.clip = sourceCrop;

    clientCompositionDisplay.orientation = rotation;



    clientCompositionDisplay.outputDataspace = renderArea.getReqDataSpace();

    clientCompositionDisplay.outputDataspace = dataspace;

    clientCompositionDisplay.maxLuminance = DisplayDevice::sDefaultMaxLumiance;



    const float colorSaturation = grayscale ? 0 : 1;

services/surfaceflinger/SurfaceFlinger.h

+2 −2
Original line number Diff line number Diff line
@@ -863,10 +863,10 @@ private: 
            RenderAreaFuture, TraverseLayersFunction,

            const std::shared_ptr<renderengine::ExternalTexture>&, bool regionSampling,

            bool grayscale, const sp<IScreenCaptureListener>&);

    std::shared_future<renderengine::RenderEngineResult> renderScreenImplLocked(

    std::shared_future<renderengine::RenderEngineResult> renderScreenImpl(

            const RenderArea&, TraverseLayersFunction,

            const std::shared_ptr<renderengine::ExternalTexture>&, bool canCaptureBlackoutContent,

            bool regionSampling, bool grayscale, ScreenCaptureResults&);

            bool regionSampling, bool grayscale, ScreenCaptureResults&) EXCLUDES(mStateLock);



    // If the uid provided is not UNSET_UID, the traverse will skip any layers that don't have a

    // matching ownerUid