Place mirrored layers offscreen instead of at root.

Mirrored Layers should never be placed at the root of the layer
hierarchy. Instead, they should be created offscreen and allow the
caller to place the layer where they want in the hierarchy. This
behavior was changed with the introduction of LayerCreatedState.

Send placeAtRoot flag when creating a layer to allow different create
callpoints to specify whether they want the layer created to be placed
at the root.

Test: Window Magnification no longer flickers.
Test: MirrorLayerTest
Fixes: 192536474
Change-Id: I08f47b1b1f19b7c655c3687cb4c1fa64844e2cc5

3 files changed, 66 insertions, 16 deletions

diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 72700162ba..4230ebe458 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3382,7 +3382,7 @@ void SurfaceFlinger::invalidateHwcGeometry() {
 status_t SurfaceFlinger::addClientLayer(const sp<Client>& client, const sp<IBinder>& handle,
                                         const sp<IGraphicBufferProducer>& gbc, const sp<Layer>& lbc,
                                         const sp<IBinder>& parentHandle,
-                                        const sp<Layer>& parentLayer, bool addToCurrentState,
+                                        const sp<Layer>& parentLayer, bool addToRoot,
                                         uint32_t* outTransformHint) {
     if (mNumLayers >= ISurfaceComposer::MAX_LAYERS) {
         ALOGE("AddClientLayer failed, mNumLayers (%zu) >= MAX_LAYERS (%zu)", mNumLayers.load(),
@@ -3394,7 +3394,7 @@ status_t SurfaceFlinger::addClientLayer(const sp<Client>& client, const sp<IBind
     if (gbc != nullptr) {
         initialProducer = IInterface::asBinder(gbc);
     }
-    setLayerCreatedState(handle, lbc, parentHandle, parentLayer, initialProducer);
+    setLayerCreatedState(handle, lbc, parentHandle, parentLayer, initialProducer, addToRoot);
 
     // Create a transaction includes the initial parent and producer.
     Vector<ComposerState> states;
@@ -3917,7 +3917,7 @@ uint32_t SurfaceFlinger::setClientStateLocked(
     sp<Layer> layer = nullptr;
     if (s.surface) {
         if (what & layer_state_t::eLayerCreated) {
-            layer = handleLayerCreatedLocked(s.surface, privileged);
+            layer = handleLayerCreatedLocked(s.surface);
             if (layer) {
                 // put the created layer into mLayersByLocalBinderToken.
                 mLayersByLocalBinderToken.emplace(s.surface->localBinder(), layer);
@@ -4327,9 +4327,9 @@ status_t SurfaceFlinger::createLayer(const String8& name, const sp<Client>& clie
         return result;
     }
 
-    bool addToCurrentState = callingThreadHasUnscopedSurfaceFlingerAccess();
-    result = addClientLayer(client, *handle, *gbp, layer, parentHandle, parentLayer,
-                            addToCurrentState, outTransformHint);
+    bool addToRoot = callingThreadHasUnscopedSurfaceFlingerAccess();
+    result = addClientLayer(client, *handle, *gbp, layer, parentHandle, parentLayer, addToRoot,
+                            outTransformHint);
     if (result != NO_ERROR) {
         return result;
     }
@@ -6872,10 +6872,10 @@ void SurfaceFlinger::TransactionState::traverseStatesWithBuffers(
 
 void SurfaceFlinger::setLayerCreatedState(const sp<IBinder>& handle, const wp<Layer>& layer,
                                           const wp<IBinder>& parent, const wp<Layer> parentLayer,
-                                          const wp<IBinder>& producer) {
+                                          const wp<IBinder>& producer, bool addToRoot) {
     Mutex::Autolock lock(mCreatedLayersLock);
     mCreatedLayers[handle->localBinder()] =
-            std::make_unique<LayerCreatedState>(layer, parent, parentLayer, producer);
+            std::make_unique<LayerCreatedState>(layer, parent, parentLayer, producer, addToRoot);
 }
 
 auto SurfaceFlinger::getLayerCreatedState(const sp<IBinder>& handle) {
@@ -6900,7 +6900,7 @@ auto SurfaceFlinger::getLayerCreatedState(const sp<IBinder>& handle) {
     return state;
 }
 
-sp<Layer> SurfaceFlinger::handleLayerCreatedLocked(const sp<IBinder>& handle, bool privileged) {
+sp<Layer> SurfaceFlinger::handleLayerCreatedLocked(const sp<IBinder>& handle) {
     const auto& state = getLayerCreatedState(handle);
     if (!state) {
         return nullptr;
@@ -6913,7 +6913,7 @@ sp<Layer> SurfaceFlinger::handleLayerCreatedLocked(const sp<IBinder>& handle, bo
     }
 
     sp<Layer> parent;
-    bool allowAddRoot = privileged;
+    bool allowAddRoot = state->addToRoot;
     if (state->initialParent != nullptr) {
         parent = fromHandleLocked(state->initialParent.promote()).promote();
         if (parent == nullptr) {
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index b9b26db260..edaca89c3e 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -905,7 +905,7 @@ private:
     status_t addClientLayer(const sp<Client>& client, const sp<IBinder>& handle,
                             const sp<IGraphicBufferProducer>& gbc, const sp<Layer>& lbc,
                             const sp<IBinder>& parentHandle, const sp<Layer>& parentLayer,
-                            bool addToCurrentState, uint32_t* outTransformHint);
+                            bool addToRoot, uint32_t* outTransformHint);
 
     // Traverse through all the layers and compute and cache its bounds.
     void computeLayerBounds();
@@ -1454,11 +1454,12 @@ private:
     mutable Mutex mCreatedLayersLock;
     struct LayerCreatedState {
         LayerCreatedState(const wp<Layer>& layer, const wp<IBinder>& parent,
-                          const wp<Layer> parentLayer, const wp<IBinder>& producer)
+                          const wp<Layer> parentLayer, const wp<IBinder>& producer, bool addToRoot)
               : layer(layer),
                 initialParent(parent),
                 initialParentLayer(parentLayer),
-                initialProducer(producer) {}
+                initialProducer(producer),
+                addToRoot(addToRoot) {}
         wp<Layer> layer;
         // Indicates the initial parent of the created layer, only used for creating layer in
         // SurfaceFlinger. If nullptr, it may add the created layer into the current root layers.
@@ -1467,6 +1468,10 @@ private:
         // Indicates the initial graphic buffer producer of the created layer, only used for
         // creating layer in SurfaceFlinger.
         wp<IBinder> initialProducer;
+        // Indicates whether the layer getting created should be added at root if there's no parent
+        // and has permission ACCESS_SURFACE_FLINGER. If set to false and no parent, the layer will
+        // be added offscreen.
+        bool addToRoot;
     };
 
     // A temporay pool that store the created layers and will be added to current state in main
@@ -1474,10 +1479,9 @@ private:
     std::unordered_map<BBinder*, std::unique_ptr<LayerCreatedState>> mCreatedLayers;
     void setLayerCreatedState(const sp<IBinder>& handle, const wp<Layer>& layer,
                               const wp<IBinder>& parent, const wp<Layer> parentLayer,
-                              const wp<IBinder>& producer);
+                              const wp<IBinder>& producer, bool addToRoot);
     auto getLayerCreatedState(const sp<IBinder>& handle);
-    sp<Layer> handleLayerCreatedLocked(const sp<IBinder>& handle, bool privileged)
-            REQUIRES(mStateLock);
+    sp<Layer> handleLayerCreatedLocked(const sp<IBinder>& handle) REQUIRES(mStateLock);
 
     std::atomic<ui::Transform::RotationFlags> mDefaultDisplayTransformHint;
 
diff --git a/services/surfaceflinger/tests/MirrorLayer_test.cpp b/services/surfaceflinger/tests/MirrorLayer_test.cpp
index ccf434d63a..d02786504e 100644
--- a/services/surfaceflinger/tests/MirrorLayer_test.cpp
+++ b/services/surfaceflinger/tests/MirrorLayer_test.cpp
@@ -18,7 +18,9 @@
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wconversion"
 
+#include <private/android_filesystem_config.h>
 #include "LayerTransactionTest.h"
+#include "utils/TransactionUtils.h"
 
 namespace android {
 
@@ -227,6 +229,50 @@ TEST_F(MirrorLayerTest, MirrorBufferLayer) {
     }
 }
 
+// Test that the mirror layer is initially offscreen.
+TEST_F(MirrorLayerTest, InitialMirrorState) {
+    const auto display = SurfaceComposerClient::getInternalDisplayToken();
+    ui::DisplayMode mode;
+    SurfaceComposerClient::getActiveDisplayMode(display, &mode);
+    const ui::Size& size = mode.resolution;
+
+    sp<SurfaceControl> mirrorLayer = nullptr;
+    {
+        // Run as system to get the ACCESS_SURFACE_FLINGER permission when mirroring
+        UIDFaker f(AID_SYSTEM);
+        // Mirror mChildLayer
+        mirrorLayer = mClient->mirrorSurface(mChildLayer.get());
+        ASSERT_NE(mirrorLayer, nullptr);
+    }
+
+    // Show the mirror layer, but don't reparent to a layer on screen.
+    Transaction()
+            .setPosition(mirrorLayer, 500, 500)
+            .show(mirrorLayer)
+            .setLayer(mirrorLayer, INT32_MAX - 1)
+            .apply();
+
+    {
+        SCOPED_TRACE("Offscreen Mirror");
+        auto shot = screenshot();
+        shot->expectColor(Rect(0, 0, size.getWidth(), 50), Color::RED);
+        shot->expectColor(Rect(0, 0, 50, size.getHeight()), Color::RED);
+        shot->expectColor(Rect(450, 0, size.getWidth(), size.getHeight()), Color::RED);
+        shot->expectColor(Rect(0, 450, size.getWidth(), size.getHeight()), Color::RED);
+        shot->expectColor(Rect(50, 50, 450, 450), Color::GREEN);
+    }
+
+    // Add mirrorLayer as child of mParentLayer so it's shown on the display
+    Transaction().reparent(mirrorLayer, mParentLayer).apply();
+
+    {
+        SCOPED_TRACE("On Screen Mirror");
+        auto shot = screenshot();
+        // Child mirror
+        shot->expectColor(Rect(550, 550, 950, 950), Color::GREEN);
+    }
+}
+
 } // namespace android
 
 // TODO(b/129481165): remove the #pragma below and fix conversion issues