Merge "SurfaceControl: C++ Binding Lifetime refactoring"
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index 7017b7c..ff8b719 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -526,21 +526,6 @@
     mDesiredPresentTime = -1;
 }
 
-void SurfaceComposerClient::doDropReferenceTransaction(const sp<IBinder>& handle) {
-    sp<ISurfaceComposer> sf(ComposerService::getComposerService());
-    Vector<ComposerState> composerStates;
-    Vector<DisplayState> displayStates;
-
-    ComposerState s;
-    s.state.surface = handle;
-    s.state.what |= layer_state_t::eReparent;
-    s.state.parentHandleForChild = nullptr;
-
-    composerStates.add(s);
-    sp<IBinder> applyToken = IInterface::asBinder(TransactionCompletedListener::getIInstance());
-    sf->setTransactionState(composerStates, displayStates, 0, applyToken, {}, -1, {}, false, {});
-}
-
 void SurfaceComposerClient::doUncacheBufferTransaction(uint64_t cacheId) {
     sp<ISurfaceComposer> sf(ComposerService::getComposerService());
 
@@ -1558,7 +1543,7 @@
         }
         ALOGE_IF(err, "SurfaceComposerClient::createWithSurfaceParent error %s", strerror(-err));
         if (err == NO_ERROR) {
-            return new SurfaceControl(this, handle, gbp, true /* owned */, transformHint);
+            return new SurfaceControl(this, handle, gbp, transformHint);
         }
     }
     return nullptr;
@@ -1589,7 +1574,7 @@
         }
         ALOGE_IF(err, "SurfaceComposerClient::createSurface error %s", strerror(-err));
         if (err == NO_ERROR) {
-            *outSurface = new SurfaceControl(this, handle, gbp, true /* owned */, transformHint);
+            *outSurface = new SurfaceControl(this, handle, gbp, transformHint);
         }
     }
     return err;
diff --git a/libs/gui/SurfaceControl.cpp b/libs/gui/SurfaceControl.cpp
index 6292388..a332a1f 100644
--- a/libs/gui/SurfaceControl.cpp
+++ b/libs/gui/SurfaceControl.cpp
@@ -46,34 +46,22 @@
 // ============================================================================
 
 SurfaceControl::SurfaceControl(const sp<SurfaceComposerClient>& client, const sp<IBinder>& handle,
-                               const sp<IGraphicBufferProducer>& gbp, bool owned,
+                               const sp<IGraphicBufferProducer>& gbp,
                                uint32_t transform)
       : mClient(client),
         mHandle(handle),
         mGraphicBufferProducer(gbp),
-        mOwned(owned),
         mTransformHint(transform) {}
 
 SurfaceControl::SurfaceControl(const sp<SurfaceControl>& other) {
     mClient = other->mClient;
     mHandle = other->mHandle;
     mGraphicBufferProducer = other->mGraphicBufferProducer;
-    mOwned = false;
     mTransformHint = other->mTransformHint;
 }
 
 SurfaceControl::~SurfaceControl()
 {
-    // Avoid reparenting the server-side surface to null if we are not the owner of it,
-    // meaning that we retrieved it from another process.
-    if (mHandle != nullptr && mOwned) {
-        SurfaceComposerClient::doDropReferenceTransaction(mHandle);
-    }
-    release();
-}
-
-void SurfaceControl::release()
-{
     // Trigger an IPC now, to make sure things
     // happen without delay, since these resources are quite heavy.
     mClient.clear();
@@ -157,7 +145,6 @@
 
 sp<IBinder> SurfaceControl::getHandle() const
 {
-    Mutex::Autolock lock(mLock);
     return mHandle;
 }
 
@@ -206,7 +193,7 @@
     return new SurfaceControl(new SurfaceComposerClient(
                                       interface_cast<ISurfaceComposerClient>(client)),
                               handle.get(), interface_cast<IGraphicBufferProducer>(gbp),
-                              false /* owned */, transformHint);
+                               transformHint);
 }
 
 // ----------------------------------------------------------------------------
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index d0bb6a3..27877bb 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -184,12 +184,6 @@
     static bool getProtectedContentSupport();
 
     /**
-     * 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);
-
-    /**
      * Uncaches a buffer in ISurfaceComposer. It must be uncached via a transaction so that it is
      * in order with other transactions that use buffers.
      */
diff --git a/libs/gui/include/gui/SurfaceControl.h b/libs/gui/include/gui/SurfaceControl.h
index 7bc7c68..ac2bbcc 100644
--- a/libs/gui/include/gui/SurfaceControl.h
+++ b/libs/gui/include/gui/SurfaceControl.h
@@ -58,10 +58,6 @@
     static bool isSameSurface(
             const sp<SurfaceControl>& lhs, const sp<SurfaceControl>& rhs);
 
-    // Release the handles assosciated with the SurfaceControl, without reparenting
-    // them off-screen. At the moment if this isn't executed before ~SurfaceControl
-    // is called then the destructor will reparent the layer off-screen for you.
-    void        release();
     // Reparent off-screen and release. This is invoked by the destructor.
     void destroy();
 
@@ -89,7 +85,7 @@
     explicit SurfaceControl(const sp<SurfaceControl>& other);
 
     SurfaceControl(const sp<SurfaceComposerClient>& client, const sp<IBinder>& handle,
-                   const sp<IGraphicBufferProducer>& gbp, bool owned, uint32_t transformHint = 0);
+                   const sp<IGraphicBufferProducer>& gbp, uint32_t transformHint = 0);
 
 private:
     // can't be copied
@@ -109,7 +105,6 @@
     sp<IGraphicBufferProducer>  mGraphicBufferProducer;
     mutable Mutex               mLock;
     mutable sp<Surface>         mSurfaceData;
-    bool                        mOwned;
     uint32_t mTransformHint;
 };
 
diff --git a/services/surfaceflinger/tests/LayerTypeAndRenderTypeTransaction_test.cpp b/services/surfaceflinger/tests/LayerTypeAndRenderTypeTransaction_test.cpp
index dbace11..2fd2579 100644
--- a/services/surfaceflinger/tests/LayerTypeAndRenderTypeTransaction_test.cpp
+++ b/services/surfaceflinger/tests/LayerTypeAndRenderTypeTransaction_test.cpp
@@ -92,7 +92,8 @@
             .setRelativeLayer(layerG, layerR->getHandle(), 1)
             .apply();
 
-    layerG.clear();
+    Transaction().reparent(layerG, nullptr).apply();
+
     // layerG should have been removed
     getScreenCapture()->expectColor(Rect(0, 0, 32, 32), Color::RED);
 }
diff --git a/services/surfaceflinger/tests/LayerUpdate_test.cpp b/services/surfaceflinger/tests/LayerUpdate_test.cpp
index cf3f8e8..cdd9d92 100644
--- a/services/surfaceflinger/tests/LayerUpdate_test.cpp
+++ b/services/surfaceflinger/tests/LayerUpdate_test.cpp
@@ -542,6 +542,7 @@
         mCapture->checkPixel(64, 64, 111, 111, 111);
     }
 
+    Transaction().reparent(mChild, nullptr).apply();
     mChild.clear();
 
     {
@@ -1702,6 +1703,7 @@
     ASSERT_NO_FATAL_FAILURE(fillBufferQueueLayerColor(redLayer, Color::RED, 60, 60));
 
     auto redLayerHandle = redLayer->getHandle();
+    Transaction().reparent(redLayer, nullptr).apply();
     redLayer.clear();
     SurfaceComposerClient::Transaction().apply(true);