Allow creating child surfaces from BlastBufferQueue

App such as Chrome create child surfaces and parent them to
surfaces provided by SurfaceView. When we enable the blast
adapter for SurfaceView, the IGBP returned to the app is
created in the client and SurfaceFlinger does not know about it.
When the app creates a child surface and provides the IGBP as the
parent surface identifier, SF fails to validate the IGBP and the
surface is not created. This can be avoid if the client creates the
child surface from the SV SurfaceControl but we still need to
support existing APIs.

To fix this, when we create a Surface from the adapter, pass in
the handle of the Blast SurfaceControl. When calling
ASurfaceControl_createFromWindow, use this handle to identify
the parent.

Bug: 168917217
Test: adb shell settings put global use_blast_adapter_sv 1 & launch chrome

Change-Id: I404bbd29b63044260f5403aae60f039a36eeea8b

13 files changed, 63 insertions, 43 deletions

diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp
index c8e1f3d1a0..0e47676dd6 100644
--- a/libs/gui/BLASTBufferQueue.cpp
+++ b/libs/gui/BLASTBufferQueue.cpp
@@ -357,11 +357,9 @@ class BBQSurface : public Surface {
 private:
     sp<BLASTBufferQueue> mBbq;
 public:
-  BBQSurface(const sp<IGraphicBufferProducer>& igbp, bool controlledByApp,
-      const sp<BLASTBufferQueue>& bbq) :
-      Surface(igbp, controlledByApp),
-      mBbq(bbq) {
-    }
+    BBQSurface(const sp<IGraphicBufferProducer>& igbp, bool controlledByApp,
+               const sp<IBinder>& scHandle, const sp<BLASTBufferQueue>& bbq)
+          : Surface(igbp, controlledByApp, scHandle), mBbq(bbq) {}
 
     void allocateBuffers() override {
         uint32_t reqWidth = mReqWidth ? mReqWidth : mUserWidth;
@@ -405,8 +403,13 @@ status_t BLASTBufferQueue::setFrameTimelineVsync(int64_t frameTimelineVsyncId) {
         .apply();
 }
 
-sp<Surface> BLASTBufferQueue::getSurface() {
-    return new BBQSurface(mProducer, true, this);
+sp<Surface> BLASTBufferQueue::getSurface(bool includeSurfaceControlHandle) {
+    std::unique_lock _lock{mMutex};
+    sp<IBinder> scHandle = nullptr;
+    if (includeSurfaceControlHandle && mSurfaceControl) {
+        scHandle = mSurfaceControl->getHandle();
+    }
+    return new BBQSurface(mProducer, true, scHandle, this);
 }
 
 } // namespace android
diff --git a/libs/gui/Surface.cpp b/libs/gui/Surface.cpp
index 167bef1449..c1155ab73a 100644
--- a/libs/gui/Surface.cpp
+++ b/libs/gui/Surface.cpp
@@ -63,7 +63,8 @@ bool isInterceptorRegistrationOp(int op) {
 
 } // namespace
 
-Surface::Surface(const sp<IGraphicBufferProducer>& bufferProducer, bool controlledByApp)
+Surface::Surface(const sp<IGraphicBufferProducer>& bufferProducer, bool controlledByApp,
+                 const sp<IBinder>& surfaceControlHandle)
       : mGraphicBufferProducer(bufferProducer),
         mCrop(Rect::EMPTY_RECT),
         mBufferAge(0),
@@ -111,6 +112,7 @@ Surface::Surface(const sp<IGraphicBufferProducer>& bufferProducer, bool controll
     mProducerControlledByApp = controlledByApp;
     mSwapIntervalZero = false;
     mMaxBufferCount = NUM_BUFFER_SLOTS;
+    mSurfaceControlHandle = surfaceControlHandle;
 }
 
 Surface::~Surface() {
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index 60966e1915..4b7d4b1a5d 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -1628,11 +1628,11 @@ void SurfaceComposerClient::dispose() {
 
 sp<SurfaceControl> SurfaceComposerClient::createSurface(const String8& name, uint32_t w, uint32_t h,
                                                         PixelFormat format, uint32_t flags,
-                                                        SurfaceControl* parent,
+                                                        const sp<IBinder>& parentHandle,
                                                         LayerMetadata metadata,
                                                         uint32_t* outTransformHint) {
     sp<SurfaceControl> s;
-    createSurfaceChecked(name, w, h, format, &s, flags, parent, std::move(metadata),
+    createSurfaceChecked(name, w, h, format, &s, flags, parentHandle, std::move(metadata),
                          outTransformHint);
     return s;
 }
@@ -1669,20 +1669,16 @@ sp<SurfaceControl> SurfaceComposerClient::createWithSurfaceParent(const String8&
 status_t SurfaceComposerClient::createSurfaceChecked(const String8& name, uint32_t w, uint32_t h,
                                                      PixelFormat format,
                                                      sp<SurfaceControl>* outSurface, uint32_t flags,
-                                                     SurfaceControl* parent, LayerMetadata metadata,
+                                                     const sp<IBinder>& parentHandle,
+                                                     LayerMetadata metadata,
                                                      uint32_t* outTransformHint) {
     sp<SurfaceControl> sur;
     status_t err = mStatus;
 
     if (mStatus == NO_ERROR) {
         sp<IBinder> handle;
-        sp<IBinder> parentHandle;
         sp<IGraphicBufferProducer> gbp;
 
-        if (parent != nullptr) {
-            parentHandle = parent->getHandle();
-        }
-
         uint32_t transformHint = 0;
         int32_t id = -1;
         err = mClient->createSurface(name, w, h, format, flags, parentHandle, std::move(metadata),
diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h
index 1410b12e12..0d457bfe4a 100644
--- a/libs/gui/include/gui/BLASTBufferQueue.h
+++ b/libs/gui/include/gui/BLASTBufferQueue.h
@@ -72,7 +72,7 @@ public:
     sp<IGraphicBufferProducer> getIGraphicBufferProducer() const {
         return mProducer;
     }
-    sp<Surface> getSurface();
+    sp<Surface> getSurface(bool includeSurfaceControlHandle);
 
     void onBufferFreed(const wp<GraphicBuffer>&/* graphicBuffer*/) override { /* TODO */ }
     void onFrameReplaced(const BufferItem& item) override {onFrameAvailable(item);}
diff --git a/libs/gui/include/gui/Surface.h b/libs/gui/include/gui/Surface.h
index c2b5ec4c4d..4aa076e7b2 100644
--- a/libs/gui/include/gui/Surface.h
+++ b/libs/gui/include/gui/Surface.h
@@ -68,7 +68,6 @@ class Surface
     : public ANativeObjectBase<ANativeWindow, Surface, RefBase>
 {
 public:
-
     /*
      * creates a Surface from the given IGraphicBufferProducer (which concrete
      * implementation is a BufferQueue).
@@ -83,9 +82,15 @@ public:
      *
      * the controlledByApp flag indicates that this Surface (producer) is
      * controlled by the application. This flag is used at connect time.
+     *
+     * Pass in the SurfaceControlHandle to store a weak reference to the layer
+     * that the Surface was created from. This handle can be used to create a
+     * child surface without using the IGBP to identify the layer. This is used
+     * for surfaces created by the BlastBufferQueue whose IGBP is created on the
+     * client and cannot be verified in SF.
      */
-    explicit Surface(const sp<IGraphicBufferProducer>& bufferProducer,
-            bool controlledByApp = false);
+    explicit Surface(const sp<IGraphicBufferProducer>& bufferProducer, bool controlledByApp = false,
+                     const sp<IBinder>& surfaceControlHandle = nullptr);
 
     /* getIGraphicBufferProducer() returns the IGraphicBufferProducer this
      * Surface was created with. Usually it's an error to use the
@@ -93,6 +98,8 @@ public:
      */
     sp<IGraphicBufferProducer> getIGraphicBufferProducer() const;
 
+    sp<IBinder> getSurfaceControlHandle() const { return mSurfaceControlHandle; }
+
     /* convenience function to check that the given surface is non NULL as
      * well as its IGraphicBufferProducer */
     static bool isValid(const sp<Surface>& surface) {
@@ -541,6 +548,11 @@ protected:
     bool mEnableFrameTimestamps = false;
     std::unique_ptr<ProducerFrameEventHistory> mFrameEventHistory;
 
+    // Reference to the SurfaceFlinger layer that was used to create this
+    // surface. This is only populated when the Surface is created from
+    // a BlastBufferQueue.
+    sp<IBinder> mSurfaceControlHandle;
+
     bool mReportRemovedBuffers = false;
     std::vector<sp<GraphicBuffer>> mRemovedBuffers;
     int mMaxBufferCount;
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index 138f82cd21..fb01dc4ba4 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -253,13 +253,13 @@ public:
     static sp<SurfaceComposerClient> getDefault();
 
     //! Create a surface
-    sp<SurfaceControl> createSurface(const String8& name,              // name of the surface
-                                     uint32_t w,                       // width in pixel
-                                     uint32_t h,                       // height in pixel
-                                     PixelFormat format,               // pixel-format desired
-                                     uint32_t flags = 0,               // usage flags
-                                     SurfaceControl* parent = nullptr, // parent
-                                     LayerMetadata metadata = LayerMetadata(), // metadata
+    sp<SurfaceControl> createSurface(const String8& name, // name of the surface
+                                     uint32_t w,          // width in pixel
+                                     uint32_t h,          // height in pixel
+                                     PixelFormat format,  // pixel-format desired
+                                     uint32_t flags = 0,  // usage flags
+                                     const sp<IBinder>& parentHandle = nullptr, // parentHandle
+                                     LayerMetadata metadata = LayerMetadata(),  // metadata
                                      uint32_t* outTransformHint = nullptr);
 
     status_t createSurfaceChecked(const String8& name, // name of the surface
@@ -267,9 +267,9 @@ public:
                                   uint32_t h,          // height in pixel
                                   PixelFormat format,  // pixel-format desired
                                   sp<SurfaceControl>* outSurface,
-                                  uint32_t flags = 0,                       // usage flags
-                                  SurfaceControl* parent = nullptr,         // parent
-                                  LayerMetadata metadata = LayerMetadata(), // metadata
+                                  uint32_t flags = 0,                        // usage flags
+                                  const sp<IBinder>& parentHandle = nullptr, // parentHandle
+                                  LayerMetadata metadata = LayerMetadata(),  // metadata
                                   uint32_t* outTransformHint = nullptr);
 
     //! Create a surface
diff --git a/libs/gui/include/gui/view/Surface.h b/libs/gui/include/gui/view/Surface.h
index cc64fd45dd..f7dcbc698d 100644
--- a/libs/gui/include/gui/view/Surface.h
+++ b/libs/gui/include/gui/view/Surface.h
@@ -21,6 +21,7 @@
 #include <utils/StrongPointer.h>
 #include <utils/String16.h>
 
+#include <binder/IBinder.h>
 #include <binder/Parcelable.h>
 
 namespace android {
@@ -43,6 +44,7 @@ class Surface : public Parcelable {
 
     String16 name;
     sp<IGraphicBufferProducer> graphicBufferProducer;
+    sp<IBinder> surfaceControlHandle;
 
     virtual status_t writeToParcel(Parcel* parcel) const override;
     virtual status_t readFromParcel(const Parcel* parcel) override;
diff --git a/libs/gui/view/Surface.cpp b/libs/gui/view/Surface.cpp
index d64dfd55be..3e49de6dc8 100644
--- a/libs/gui/view/Surface.cpp
+++ b/libs/gui/view/Surface.cpp
@@ -45,7 +45,9 @@ status_t Surface::writeToParcel(Parcel* parcel, bool nameAlreadyWritten) const {
         if (res != OK) return res;
     }
 
-    return IGraphicBufferProducer::exportToParcel(graphicBufferProducer, parcel);
+    res = IGraphicBufferProducer::exportToParcel(graphicBufferProducer, parcel);
+    if (res != OK) return res;
+    return parcel->writeStrongBinder(surfaceControlHandle);
 }
 
 status_t Surface::readFromParcel(const Parcel* parcel) {
@@ -68,6 +70,7 @@ status_t Surface::readFromParcel(const Parcel* parcel, bool nameAlreadyRead) {
     }
 
     graphicBufferProducer = IGraphicBufferProducer::createFromParcel(parcel);
+    surfaceControlHandle = parcel->readStrongBinder();
     return OK;
 }
 
diff --git a/services/surfaceflinger/tests/EffectLayer_test.cpp b/services/surfaceflinger/tests/EffectLayer_test.cpp
index 3dca3916e4..fafb49efba 100644
--- a/services/surfaceflinger/tests/EffectLayer_test.cpp
+++ b/services/surfaceflinger/tests/EffectLayer_test.cpp
@@ -51,7 +51,7 @@ TEST_F(EffectLayerTest, DefaultEffectLayerHasSolidBlackFill) {
     sp<SurfaceControl> effectLayer =
             mClient->createSurface(String8("Effect Layer"), 0 /* width */, 0 /* height */,
                                    PIXEL_FORMAT_RGBA_8888, ISurfaceComposerClient::eFXSurfaceEffect,
-                                   mParentLayer.get());
+                                   mParentLayer->getHandle());
 
     EXPECT_NE(nullptr, effectLayer.get()) << "failed to create SurfaceControl";
     asTransaction([&](Transaction& t) {
@@ -72,7 +72,7 @@ TEST_F(EffectLayerTest, EffectLayerWithNoFill) {
                                    PIXEL_FORMAT_RGBA_8888,
                                    ISurfaceComposerClient::eFXSurfaceEffect |
                                            ISurfaceComposerClient::eNoColorFill,
-                                   mParentLayer.get());
+                                   mParentLayer->getHandle());
 
     EXPECT_NE(nullptr, effectLayer.get()) << "failed to create SurfaceControl";
     asTransaction([&](Transaction& t) {
@@ -93,7 +93,7 @@ TEST_F(EffectLayerTest, EffectLayerCanSetColor) {
                                    PIXEL_FORMAT_RGBA_8888,
                                    ISurfaceComposerClient::eFXSurfaceEffect |
                                            ISurfaceComposerClient::eNoColorFill,
-                                   mParentLayer.get());
+                                   mParentLayer->getHandle());
 
     EXPECT_NE(nullptr, effectLayer.get()) << "failed to create SurfaceControl";
     asTransaction([&](Transaction& t) {
diff --git a/services/surfaceflinger/tests/InvalidHandles_test.cpp b/services/surfaceflinger/tests/InvalidHandles_test.cpp
index cfec0d2b51..152d2d26f4 100644
--- a/services/surfaceflinger/tests/InvalidHandles_test.cpp
+++ b/services/surfaceflinger/tests/InvalidHandles_test.cpp
@@ -56,7 +56,7 @@ TEST_F(InvalidHandleTest, createSurfaceInvalidHandle) {
     auto notSc = makeNotSurfaceControl();
     ASSERT_EQ(nullptr,
               mScc->createSurface(String8("lolcats"), 19, 47, PIXEL_FORMAT_RGBA_8888, 0,
-                                  notSc.get())
+                                  notSc->getHandle())
                       .get());
 }
 
diff --git a/services/surfaceflinger/tests/LayerTransactionTest.h b/services/surfaceflinger/tests/LayerTransactionTest.h
index d4e952aa21..25d3211f6b 100644
--- a/services/surfaceflinger/tests/LayerTransactionTest.h
+++ b/services/surfaceflinger/tests/LayerTransactionTest.h
@@ -75,8 +75,9 @@ protected:
                                              PixelFormat format, uint32_t flags,
                                              SurfaceControl* parent = nullptr,
                                              uint32_t* outTransformHint = nullptr) {
-        auto layer = client->createSurface(String8(name), width, height, format, flags, parent,
-                                           LayerMetadata(), outTransformHint);
+        sp<IBinder> parentHandle = (parent) ? parent->getHandle() : nullptr;
+        auto layer = client->createSurface(String8(name), width, height, format, flags,
+                                           parentHandle, LayerMetadata(), outTransformHint);
         EXPECT_NE(nullptr, layer.get()) << "failed to create SurfaceControl";
         return layer;
     }
diff --git a/services/surfaceflinger/tests/LayerUpdate_test.cpp b/services/surfaceflinger/tests/LayerUpdate_test.cpp
index 0cafd001ff..29473f20a4 100644
--- a/services/surfaceflinger/tests/LayerUpdate_test.cpp
+++ b/services/surfaceflinger/tests/LayerUpdate_test.cpp
@@ -1028,12 +1028,13 @@ TEST_F(BoundlessLayerTest, IntermediateBoundlessLayerCanSetTransform) {
 TEST_F(BoundlessLayerTest, IntermediateBoundlessLayerDoNotCrop) {
     sp<SurfaceControl> boundlessLayer =
             mClient->createSurface(String8("BoundlessLayer"), 0, 0, PIXEL_FORMAT_RGBA_8888,
-                                   0 /* flags */, mFGSurfaceControl.get());
+                                   0 /* flags */, mFGSurfaceControl->getHandle());
     ASSERT_TRUE(boundlessLayer != nullptr);
     ASSERT_TRUE(boundlessLayer->isValid());
     sp<SurfaceControl> colorLayer =
             mClient->createSurface(String8("ColorLayer"), 0, 0, PIXEL_FORMAT_RGBA_8888,
-                                   ISurfaceComposerClient::eFXSurfaceEffect, boundlessLayer.get());
+                                   ISurfaceComposerClient::eFXSurfaceEffect,
+                                   boundlessLayer->getHandle());
     ASSERT_TRUE(colorLayer != nullptr);
     ASSERT_TRUE(colorLayer->isValid());
     asTransaction([&](Transaction& t) {
diff --git a/services/surfaceflinger/tests/fakehwc/SFFakeHwc_test.cpp b/services/surfaceflinger/tests/fakehwc/SFFakeHwc_test.cpp
index 1606f2241e..6c654c07e3 100644
--- a/services/surfaceflinger/tests/fakehwc/SFFakeHwc_test.cpp
+++ b/services/surfaceflinger/tests/fakehwc/SFFakeHwc_test.cpp
@@ -1469,7 +1469,7 @@ protected:
         Base::SetUp();
         mChild = Base::mComposerClient->createSurface(String8("Child surface"), 10, 10,
                                                       PIXEL_FORMAT_RGBA_8888, 0,
-                                                      Base::mFGSurfaceControl.get());
+                                                      Base::mFGSurfaceControl->getHandle());
         fillSurfaceRGBA8(mChild, LIGHT_GRAY);
 
         Base::sFakeComposer->runVSyncAndWait();
@@ -1653,7 +1653,7 @@ protected:
         sp<SurfaceControl> childNewClient =
                 newComposerClient->createSurface(String8("New Child Test Surface"), 10, 10,
                                                  PIXEL_FORMAT_RGBA_8888, 0,
-                                                 Base::mFGSurfaceControl.get());
+                                                 Base::mFGSurfaceControl->getHandle());
         ASSERT_TRUE(childNewClient != nullptr);
         ASSERT_TRUE(childNewClient->isValid());
         fillSurfaceRGBA8(childNewClient, LIGHT_GRAY);
@@ -1732,7 +1732,7 @@ protected:
         mChild = Base::mComposerClient->createSurface(String8("Child surface"), 10, 10,
                                                       PIXEL_FORMAT_RGBA_8888,
                                                       ISurfaceComposerClient::eHidden,
-                                                      Base::mFGSurfaceControl.get());
+                                                      Base::mFGSurfaceControl->getHandle());
 
         // Show the child layer in a deferred transaction
         {
@@ -1819,7 +1819,7 @@ protected:
                 Base::mComposerClient->createSurface(String8("Child surface"), 0, 0,
                                                      PIXEL_FORMAT_RGBA_8888,
                                                      ISurfaceComposerClient::eFXSurfaceEffect,
-                                                     Base::mFGSurfaceControl.get());
+                                                     Base::mFGSurfaceControl->getHandle());
         {
             TransactionScope ts(*Base::sFakeComposer);
             ts.setColor(Base::mChild,