SurfaceFlinger: Remove removeLayer

We remove explicit layer destruction and replace it
with reparent->null, completing the transition to
a reference counted model.

Test: Manual
Bug: 62536731
Bug: 111373437
Bug: 111297488

Change-Id: I8ac7c5c5125e1c8daf84b42db00e1dd93a544bb5

15 files changed, 82 insertions, 219 deletions

diff --git a/cmds/surfacereplayer/replayer/Replayer.cpp b/cmds/surfacereplayer/replayer/Replayer.cpp
index 384f21ff1..34886a99e 100644
--- a/cmds/surfacereplayer/replayer/Replayer.cpp
+++ b/cmds/surfacereplayer/replayer/Replayer.cpp
@@ -286,10 +286,6 @@ status_t Replayer::dispatchEvent(int index) {
             std::thread(&Replayer::createSurfaceControl, this, increment.surface_creation(), event)
                     .detach();
         } break;
-        case increment.kSurfaceDeletion: {
-            std::thread(&Replayer::deleteSurfaceControl, this, increment.surface_deletion(), event)
-                    .detach();
-        } break;
         case increment.kBufferUpdate: {
             std::lock_guard<std::mutex> lock1(mLayerLock);
             std::lock_guard<std::mutex> lock2(mBufferQueueSchedulerLock);
@@ -628,47 +624,10 @@ status_t Replayer::createSurfaceControl(
     return NO_ERROR;
 }
 
-status_t Replayer::deleteSurfaceControl(
-        const SurfaceDeletion& delete_, const std::shared_ptr<Event>& event) {
-    ALOGV("Deleting %d Surface Control", delete_.id());
-    event->readyToExecute();
-
-    std::lock_guard<std::mutex> lock1(mPendingLayersLock);
-
-    mLayersPendingRemoval.push_back(delete_.id());
-
-    const auto& iterator = mBufferQueueSchedulers.find(delete_.id());
-    if (iterator != mBufferQueueSchedulers.end()) {
-        (*iterator).second->stopScheduling();
-    }
-
-    std::lock_guard<std::mutex> lock2(mLayerLock);
-    if (mLayers[delete_.id()] != nullptr) {
-        mComposerClient->destroySurface(mLayers[delete_.id()]->getHandle());
-    }
-
-    return NO_ERROR;
-}
-
-void Replayer::doDeleteSurfaceControls() {
-    std::lock_guard<std::mutex> lock1(mPendingLayersLock);
-    std::lock_guard<std::mutex> lock2(mLayerLock);
-    if (!mLayersPendingRemoval.empty()) {
-        for (int id : mLayersPendingRemoval) {
-            mLayers.erase(id);
-            mColors.erase(id);
-            mBufferQueueSchedulers.erase(id);
-        }
-        mLayersPendingRemoval.clear();
-    }
-}
-
 status_t Replayer::injectVSyncEvent(
         const VSyncEvent& vSyncEvent, const std::shared_ptr<Event>& event) {
     ALOGV("Injecting VSync Event");
 
-    doDeleteSurfaceControls();
-
     event->readyToExecute();
 
     SurfaceComposerClient::injectVSync(vSyncEvent.when());
diff --git a/cmds/surfacereplayer/replayer/Replayer.h b/cmds/surfacereplayer/replayer/Replayer.h
index 120dd9bab..ad807ee95 100644
--- a/cmds/surfacereplayer/replayer/Replayer.h
+++ b/cmds/surfacereplayer/replayer/Replayer.h
@@ -70,8 +70,6 @@ class Replayer {
     status_t doTransaction(const Transaction& transaction, const std::shared_ptr<Event>& event);
     status_t createSurfaceControl(const SurfaceCreation& create,
             const std::shared_ptr<Event>& event);
-    status_t deleteSurfaceControl(const SurfaceDeletion& delete_,
-            const std::shared_ptr<Event>& event);
     status_t injectVSyncEvent(const VSyncEvent& vsyncEvent, const std::shared_ptr<Event>& event);
     void createDisplay(const DisplayCreation& create, const std::shared_ptr<Event>& event);
     void deleteDisplay(const DisplayDeletion& delete_, const std::shared_ptr<Event>& event);
@@ -120,7 +118,6 @@ class Replayer {
     void setDisplayProjection(SurfaceComposerClient::Transaction& t,
             display_id id, const ProjectionChange& pc);
 
-    void doDeleteSurfaceControls();
     void waitUntilTimestamp(int64_t timestamp);
     void waitUntilDeferredTransactionLayerExists(
             const DeferredTransactionChange& dtc, std::unique_lock<std::mutex>& lock);
diff --git a/libs/gui/ISurfaceComposerClient.cpp b/libs/gui/ISurfaceComposerClient.cpp
index a6890eeb1..369f523ea 100644
--- a/libs/gui/ISurfaceComposerClient.cpp
+++ b/libs/gui/ISurfaceComposerClient.cpp
@@ -31,7 +31,6 @@ namespace { // Anonymous
 
 enum class Tag : uint32_t {
     CREATE_SURFACE = IBinder::FIRST_CALL_TRANSACTION,
-    DESTROY_SURFACE,
     CLEAR_LAYER_FRAME_STATS,
     GET_LAYER_FRAME_STATS,
     LAST = GET_LAYER_FRAME_STATS,
@@ -57,11 +56,6 @@ public:
                                                                             handle, gbp);
     }
 
-    status_t destroySurface(const sp<IBinder>& handle) override {
-        return callRemote<decltype(&ISurfaceComposerClient::destroySurface)>(Tag::DESTROY_SURFACE,
-                                                                             handle);
-    }
-
     status_t clearLayerFrameStats(const sp<IBinder>& handle) const override {
         return callRemote<decltype(
                 &ISurfaceComposerClient::clearLayerFrameStats)>(Tag::CLEAR_LAYER_FRAME_STATS,
@@ -92,8 +86,6 @@ status_t BnSurfaceComposerClient::onTransact(uint32_t code, const Parcel& data,
     switch (tag) {
         case Tag::CREATE_SURFACE:
             return callLocal(data, reply, &ISurfaceComposerClient::createSurface);
-        case Tag::DESTROY_SURFACE:
-            return callLocal(data, reply, &ISurfaceComposerClient::destroySurface);
         case Tag::CLEAR_LAYER_FRAME_STATS:
             return callLocal(data, reply, &ISurfaceComposerClient::clearLayerFrameStats);
         case Tag::GET_LAYER_FRAME_STATS:
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index 824e43fb0..6c39d6f1b 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -205,6 +205,22 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::merge(Tr
     return *this;
 }
 
+void SurfaceComposerClient::doDropReferenceTransaction(const sp<IBinder>& handle,
+        const sp<ISurfaceComposerClient>& client) {
+    sp<ISurfaceComposer> sf(ComposerService::getComposerService());
+    Vector<ComposerState> composerStates;
+    Vector<DisplayState> displayStates;
+
+    ComposerState s;
+    s.client = client;
+    s.state.surface = handle;
+    s.state.what |= layer_state_t::eReparent;
+    s.state.parentHandleForChild = nullptr;
+
+    composerStates.add(s);
+    sf->setTransactionState(composerStates, displayStates, 0, nullptr, {});
+}
+
 status_t SurfaceComposerClient::Transaction::apply(bool synchronous) {
     if (mStatus != NO_ERROR) {
         return mStatus;
@@ -819,17 +835,6 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::transfer
 
 #endif
 
-SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::destroySurface(
-        const sp<SurfaceControl>& sc) {
-    layer_state_t* s = getLayerState(sc);
-    if (!s) {
-        mStatus = BAD_INDEX;
-        return *this;
-    }
-    s->what |= layer_state_t::eDestroySurface;
-    return *this;
-}
-
 SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setColorTransform(
     const sp<SurfaceControl>& sc, const mat3& matrix, const vec3& translation) {
     layer_state_t* s = getLayerState(sc);
@@ -1004,13 +1009,6 @@ status_t SurfaceComposerClient::createSurfaceChecked(
     return err;
 }
 
-status_t SurfaceComposerClient::destroySurface(const sp<IBinder>& sid) {
-    if (mStatus != NO_ERROR)
-        return mStatus;
-    status_t err = mClient->destroySurface(sid);
-    return err;
-}
-
 status_t SurfaceComposerClient::clearLayerFrameStats(const sp<IBinder>& token) const {
     if (mStatus != NO_ERROR) {
         return mStatus;
diff --git a/libs/gui/SurfaceControl.cpp b/libs/gui/SurfaceControl.cpp
index b6ef286fd..8e500a4f9 100644
--- a/libs/gui/SurfaceControl.cpp
+++ b/libs/gui/SurfaceControl.cpp
@@ -63,7 +63,13 @@ SurfaceControl::SurfaceControl(const sp<SurfaceControl>& other) {
 
 SurfaceControl::~SurfaceControl()
 {
-    destroy();
+    if (mClient != nullptr && mHandle != nullptr && mOwned) {
+        SurfaceComposerClient::doDropReferenceTransaction(mHandle, mClient->getClient());
+    }
+    mClient.clear();
+    mHandle.clear();
+    mGraphicBufferProducer.clear();
+    IPCThreadState::self()->flushCommands();
 }
 
 void SurfaceControl::destroy()
@@ -71,7 +77,7 @@ void SurfaceControl::destroy()
     // Avoid destroying the server-side surface if we are not the owner of it, meaning that we
     // retrieved it from another process.
     if (isValid() && mOwned) {
-        mClient->destroySurface(mHandle);
+        SurfaceComposerClient::Transaction().reparent(this, nullptr).apply();
     }
     // clear all references and trigger an IPC now, to make sure things
     // happen without delay, since these resources are quite heavy.
diff --git a/libs/gui/include/gui/ISurfaceComposerClient.h b/libs/gui/include/gui/ISurfaceComposerClient.h
index 82b01b8cf..56ca197ff 100644
--- a/libs/gui/include/gui/ISurfaceComposerClient.h
+++ b/libs/gui/include/gui/ISurfaceComposerClient.h
@@ -58,11 +58,6 @@ public:
     /*
      * Requires ACCESS_SURFACE_FLINGER permission
      */
-    virtual status_t destroySurface(const sp<IBinder>& handle) = 0;
-
-    /*
-     * Requires ACCESS_SURFACE_FLINGER permission
-     */
     virtual status_t clearLayerFrameStats(const sp<IBinder>& handle) const = 0;
 
     /*
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index 4a8e01bec..e0cbb705c 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -145,6 +145,13 @@ public:
                                              ui::Dataspace* wideColorGamutDataspace,
                                              ui::PixelFormat* wideColorGamutPixelFormat);
 
+    /**
+     * Called from SurfaceControl d'tor to 'destroy' the surface (or rather, reparent it
+     * to null), but without needing an sp<SurfaceControl> to avoid infinite ressurection.
+     */
+    static void doDropReferenceTransaction(const sp<IBinder>& handle,
+            const sp<ISurfaceComposerClient>& client);
+
     // ------------------------------------------------------------------------
     // surface creation / destruction
 
@@ -338,8 +345,6 @@ public:
         Transaction& transferTouchFocus(const sp<IBinder>& fromToken, const sp<IBinder>& toToken);
 #endif
 
-        Transaction& destroySurface(const sp<SurfaceControl>& sc);
-
         // Set a color transform matrix on the given layer on the built-in display.
         Transaction& setColorTransform(const sp<SurfaceControl>& sc, const mat3& matrix,
                                        const vec3& translation);
@@ -368,8 +373,6 @@ public:
         void setEarlyWakeup();
     };
 
-    status_t    destroySurface(const sp<IBinder>& id);
-
     status_t clearLayerFrameStats(const sp<IBinder>& token) const;
     status_t getLayerFrameStats(const sp<IBinder>& token, FrameStats* outStats) const;
     static status_t clearAnimationFrameStats();
diff --git a/services/surfaceflinger/Client.cpp b/services/surfaceflinger/Client.cpp
index ee4ec506f..4f6fb1cca 100644
--- a/services/surfaceflinger/Client.cpp
+++ b/services/surfaceflinger/Client.cpp
@@ -39,26 +39,6 @@ Client::Client(const sp<SurfaceFlinger>& flinger)
 {
 }
 
-Client::~Client()
-{
-    // We need to post a message to remove our remaining layers rather than
-    // do so directly by acquiring the SurfaceFlinger lock. If we were to
-    // attempt to directly call the lock it becomes effectively impossible
-    // to use sp<Client> while holding the SF lock as descoping it could
-    // then trigger a dead-lock.
-
-    const size_t count = mLayers.size();
-    for (size_t i=0 ; i<count ; i++) {
-        sp<Layer> l = mLayers.valueAt(i).promote();
-        if (l == nullptr) {
-            continue;
-        }
-        mFlinger->postMessageAsync(new LambdaMessage([flinger = mFlinger, l]() {
-            flinger->removeLayer(l);
-        }));
-    }
-}
-
 status_t Client::initCheck() const {
     return NO_ERROR;
 }
@@ -115,10 +95,6 @@ status_t Client::createSurface(
                                  ownerUid, handle, gbp, &parent);
 }
 
-status_t Client::destroySurface(const sp<IBinder>& handle) {
-    return mFlinger->onLayerRemoved(this, handle);
-}
-
 status_t Client::clearLayerFrameStats(const sp<IBinder>& handle) const {
     sp<Layer> layer = getLayerUser(handle);
     if (layer == nullptr) {
diff --git a/services/surfaceflinger/Client.h b/services/surfaceflinger/Client.h
index 4a74739a1..d0051dee1 100644
--- a/services/surfaceflinger/Client.h
+++ b/services/surfaceflinger/Client.h
@@ -39,13 +39,12 @@ class Client : public BnSurfaceComposerClient
 {
 public:
     explicit Client(const sp<SurfaceFlinger>& flinger);
-    ~Client();
+    ~Client() = default;
 
     status_t initCheck() const;
 
     // protected by SurfaceFlinger::mStateLock
     void attachLayer(const sp<IBinder>& handle, const sp<Layer>& layer);
-
     void detachLayer(const Layer* layer);
 
     sp<Layer> getLayerUser(const sp<IBinder>& handle) const;
@@ -59,8 +58,6 @@ private:
             sp<IBinder>* handle,
             sp<IGraphicBufferProducer>* gbp);
 
-    virtual status_t destroySurface(const sp<IBinder>& handle);
-
     virtual status_t clearLayerFrameStats(const sp<IBinder>& handle) const;
 
     virtual status_t getLayerFrameStats(const sp<IBinder>& handle, FrameStats* outStats) const;
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index b1827c191..3f2d10a45 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -179,6 +179,8 @@ void Layer::onRemovedFromCurrentState() {
     for (const auto& child : mCurrentChildren) {
         child->onRemovedFromCurrentState();
     }
+
+    mFlinger->markLayerPendingRemovalLocked(this);
 }
 
 void Layer::addToCurrentState() {
@@ -1571,6 +1573,7 @@ void Layer::addChild(const sp<Layer>& layer) {
 
 ssize_t Layer::removeChild(const sp<Layer>& layer) {
     layer->setParent(nullptr);
+
     return mCurrentChildren.remove(layer);
 }
 
@@ -1605,14 +1608,14 @@ void Layer::setChildrenDrawingParent(const sp<Layer>& newParent) {
 }
 
 bool Layer::reparent(const sp<IBinder>& newParentHandle) {
-    if (newParentHandle == nullptr) {
-        return false;
-    }
-
-    auto handle = static_cast<Handle*>(newParentHandle.get());
-    sp<Layer> newParent = handle->owner.promote();
-    if (newParent == nullptr) {
-        ALOGE("Unable to promote Layer handle");
+    bool callSetTransactionFlags = false;
+
+    // While layers are detached, we allow most operations
+    // and simply halt performing the actual transaction. However
+    // for reparent != null we would enter the mRemovedFromCurrentState
+    // state, regardless of whether doTransaction was called, and
+    // so we need to prevent the update here.
+    if (mLayerDetached && newParentHandle == nullptr) {
         return false;
     }
 
@@ -1620,17 +1623,31 @@ bool Layer::reparent(const sp<IBinder>& newParentHandle) {
     if (parent != nullptr) {
         parent->removeChild(this);
     }
-    newParent->addChild(this);
 
-    if (!newParent->isRemovedFromCurrentState()) {
-        addToCurrentState();
-    }
+    if (newParentHandle != nullptr) {
+        auto handle = static_cast<Handle*>(newParentHandle.get());
+        sp<Layer> newParent = handle->owner.promote();
+        if (newParent == nullptr) {
+            ALOGE("Unable to promote Layer handle");
+            return false;
+        }
 
-    if (mLayerDetached) {
-        mLayerDetached = false;
-        setTransactionFlags(eTransactionNeeded);
+        newParent->addChild(this);
+        if (!newParent->isRemovedFromCurrentState()) {
+            addToCurrentState();
+        } else {
+            onRemovedFromCurrentState();
+        }
+
+        if (mLayerDetached) {
+            mLayerDetached = false;
+            callSetTransactionFlags = true;
+        }
+    } else {
+        onRemovedFromCurrentState();
     }
-    if (attachChildren()) {
+
+    if (callSetTransactionFlags || attachChildren()) {
         setTransactionFlags(eTransactionNeeded);
     }
     return true;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index fd25abf1a..3648be49e 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2799,10 +2799,8 @@ void SurfaceFlinger::commitTransaction()
             // showing at its last configured state until we eventually
             // abandon the buffer queue.
             if (l->isRemovedFromCurrentState()) {
-                l->traverseInZOrder(LayerVector::StateSet::Drawing, [&](Layer* child) {
-                    child->destroyHwcLayersForAllDisplays();
-                    latchAndReleaseBuffer(child);
-                });
+                l->destroyHwcLayersForAllDisplays();
+                latchAndReleaseBuffer(l);
             }
         }
         mLayersPendingRemoval.clear();
@@ -3276,30 +3274,6 @@ status_t SurfaceFlinger::addClientLayer(const sp<Client>& client,
     return NO_ERROR;
 }
 
-status_t SurfaceFlinger::removeLayer(const sp<Layer>& layer) {
-    Mutex::Autolock _l(mStateLock);
-    return removeLayerLocked(mStateLock, layer);
-}
-
-status_t SurfaceFlinger::removeLayerLocked(const Mutex& lock, const sp<Layer>& layer) {
-    if (layer->isLayerDetached()) {
-        return NO_ERROR;
-    }
-
-    const auto& p = layer->getParent();
-    ssize_t index;
-    if (p != nullptr) {
-        index = p->removeChild(layer);
-    } else {
-        index = mCurrentState.layersSortedByZ.remove(layer);
-    }
-
-    layer->onRemovedFromCurrentState();
-
-    markLayerPendingRemovalLocked(lock, layer);
-    return NO_ERROR;
-}
-
 uint32_t SurfaceFlinger::peekTransactionFlags() {
     return mTransactionFlags;
 }
@@ -3434,14 +3408,6 @@ void SurfaceFlinger::applyTransactionState(const Vector<ComposerState>& states,
     }
     transactionFlags |= clientStateFlags;
 
-    // Iterate through all layers again to determine if any need to be destroyed. Marking layers
-    // as destroyed should only occur after setting all other states. This is to allow for a
-    // child re-parent to happen before marking its original parent as destroyed (which would
-    // then mark the child as destroyed).
-    for (const ComposerState& state : states) {
-        setDestroyStateLocked(state);
-    }
-
     transactionFlags |= addInputWindowCommands(inputWindowCommands);
 
     // If a synchronous transaction is explicitly requested without any changes, force a transaction
@@ -3767,20 +3733,6 @@ uint32_t SurfaceFlinger::setClientStateLocked(const ComposerState& composerState
     return flags;
 }
 
-void SurfaceFlinger::setDestroyStateLocked(const ComposerState& composerState) {
-    const layer_state_t& state = composerState.state;
-    sp<Client> client(static_cast<Client*>(composerState.client.get()));
-
-    sp<Layer> layer(client->getLayerUser(state.surface));
-    if (layer == nullptr) {
-        return;
-    }
-
-    if (state.what & layer_state_t::eDestroySurface) {
-        removeLayerLocked(mStateLock, layer);
-    }
-}
-
 uint32_t SurfaceFlinger::addInputWindowCommands(const InputWindowCommands& inputWindowCommands) {
     uint32_t flags = 0;
     if (!inputWindowCommands.transferTouchFocusCommands.empty()) {
@@ -3960,21 +3912,7 @@ status_t SurfaceFlinger::createContainerLayer(const sp<Client>& client,
 }
 
 
-status_t SurfaceFlinger::onLayerRemoved(const sp<Client>& client, const sp<IBinder>& handle)
-{
-    // called by a client when it wants to remove a Layer
-    status_t err = NO_ERROR;
-    sp<Layer> l(client->getLayerUser(handle));
-    if (l != nullptr) {
-        mInterceptor->saveSurfaceDeletion(l);
-        err = removeLayer(l);
-        ALOGE_IF(err<0 && err != NAME_NOT_FOUND,
-                "error removing layer=%p (%s)", l.get(), strerror(-err));
-    }
-    return err;
-}
-
-void SurfaceFlinger::markLayerPendingRemovalLocked(const Mutex&, const sp<Layer>& layer) {
+void SurfaceFlinger::markLayerPendingRemovalLocked(const sp<Layer>& layer) {
     mLayersPendingRemoval.add(layer);
     mLayersRemoved = true;
     setTransactionFlags(eTransactionNeeded);
@@ -3983,7 +3921,14 @@ void SurfaceFlinger::markLayerPendingRemovalLocked(const Mutex&, const sp<Layer>
 void SurfaceFlinger::onHandleDestroyed(sp<Layer>& layer)
 {
     Mutex::Autolock lock(mStateLock);
-    markLayerPendingRemovalLocked(mStateLock, layer);
+    // If a layer has a parent, we allow it to out-live it's handle
+    // with the idea that the parent holds a reference and will eventually
+    // be cleaned up. However no one cleans up the top-level so we do so
+    // here.
+    if (layer->getParent() == nullptr) {
+        mCurrentState.layersSortedByZ.remove(layer);
+    }
+    markLayerPendingRemovalLocked(layer);
     layer.clear();
 }
 
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index bfc87a02e..b8bfcf1f9 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -569,7 +569,6 @@ private:
     bool composerStateContainsUnsignaledFences(const Vector<ComposerState>& states);
     uint32_t setClientStateLocked(const ComposerState& composerState);
     uint32_t setDisplayStateLocked(const DisplayState& s);
-    void setDestroyStateLocked(const ComposerState& composerState);
     uint32_t addInputWindowCommands(const InputWindowCommands& inputWindowCommands);
 
     /* ------------------------------------------------------------------------
@@ -599,20 +598,11 @@ private:
 
     String8 getUniqueLayerName(const String8& name);
 
-    // called in response to the window-manager calling
-    // ISurfaceComposerClient::destroySurface()
-    status_t onLayerRemoved(const sp<Client>& client, const sp<IBinder>& handle);
-
-    void markLayerPendingRemovalLocked(const Mutex& /* mStateLock */, const sp<Layer>& layer);
-
     // called when all clients have released all their references to
     // this layer meaning it is entirely safe to destroy all
     // resources associated to this layer.
     void onHandleDestroyed(sp<Layer>& layer);
-
-    // remove a layer from SurfaceFlinger immediately
-    status_t removeLayer(const sp<Layer>& layer);
-    status_t removeLayerLocked(const Mutex&, const sp<Layer>& layer);
+    void markLayerPendingRemovalLocked(const sp<Layer>& layer);
 
     // add a layer to SurfaceFlinger
     status_t addClientLayer(const sp<Client>& client,
diff --git a/services/surfaceflinger/tests/Stress_test.cpp b/services/surfaceflinger/tests/Stress_test.cpp
index 3e1be8e28..89c26f4a9 100644
--- a/services/surfaceflinger/tests/Stress_test.cpp
+++ b/services/surfaceflinger/tests/Stress_test.cpp
@@ -34,7 +34,7 @@ TEST(SurfaceFlingerStress, create_and_destroy) {
             auto surf = client->createSurface(String8("t"), 100, 100,
                     PIXEL_FORMAT_RGBA_8888, 0);
             ASSERT_TRUE(surf != nullptr);
-            client->destroySurface(surf->getHandle());
+            surf->clear();
         }
     };
 
diff --git a/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp b/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp
index e50675786..8ec3e154e 100644
--- a/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp
+++ b/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp
@@ -806,18 +806,6 @@ TEST_F(SurfaceInterceptorTest, InterceptSurfaceCreationWorks) {
             Increment::IncrementCase::kSurfaceCreation);
 }
 
-TEST_F(SurfaceInterceptorTest, InterceptSurfaceDeletionWorks) {
-    enableInterceptor();
-    sp<SurfaceControl> layerToDelete = mComposerClient->createSurface(String8(LAYER_NAME),
-            SIZE_UPDATE, SIZE_UPDATE, PIXEL_FORMAT_RGBA_8888, 0);
-    mComposerClient->destroySurface(layerToDelete->getHandle());
-    disableInterceptor();
-
-    Trace capturedTrace;
-    ASSERT_EQ(NO_ERROR, readProtoFile(&capturedTrace));
-    ASSERT_TRUE(singleIncrementFound(capturedTrace, Increment::IncrementCase::kSurfaceDeletion));
-}
-
 TEST_F(SurfaceInterceptorTest, InterceptDisplayCreationWorks) {
     captureTest(&SurfaceInterceptorTest::displayCreation,
             Increment::IncrementCase::kDisplayCreation);
diff --git a/services/surfaceflinger/tests/Transaction_test.cpp b/services/surfaceflinger/tests/Transaction_test.cpp
index d118ad640..991ea36a3 100644
--- a/services/surfaceflinger/tests/Transaction_test.cpp
+++ b/services/surfaceflinger/tests/Transaction_test.cpp
@@ -1023,7 +1023,7 @@ TEST_P(LayerTypeTransactionTest, SetRelativeZBug64572777) {
             .setRelativeLayer(layerG, layerR->getHandle(), 1)
             .apply();
 
-    mClient->destroySurface(layerG->getHandle());
+    layerG->clear();
     // layerG should have been removed
     screenshot()->expectColor(Rect(0, 0, 32, 32), Color::RED);
 }
@@ -4030,9 +4030,9 @@ TEST_F(ChildLayerTest, ReparentToNoParent) {
     asTransaction([&](Transaction& t) { t.reparent(mChild, nullptr); });
     {
         mCapture = screenshot();
-        // Nothing should have changed.
+        // The surface should now be offscreen.
         mCapture->expectFGColor(64, 64);
-        mCapture->expectChildColor(74, 74);
+        mCapture->expectFGColor(74, 74);
         mCapture->expectFGColor(84, 84);
     }
 }
@@ -4610,7 +4610,7 @@ TEST_F(ScreenCaptureTest, CaptureInvalidLayer) {
     ASSERT_NO_FATAL_FAILURE(fillBufferQueueLayerColor(redLayer, Color::RED, 60, 60));
 
     auto redLayerHandle = redLayer->getHandle();
-    mClient->destroySurface(redLayerHandle);
+    redLayer->clear();
     SurfaceComposerClient::Transaction().apply(true);
 
     sp<GraphicBuffer> outBuffer;