Add captureLayersSync function

This captureLayers function in SurfaceFlinger will wait on the
requested binder thread for the screenshot composition to complete
before returning the buffer to the client. This is different than the
existing captureLayers method since the other request is oneway and
invokes the screen capture callback on a different binder thread.

This is needed because there are places in system server that request
screenshots while holding a lock and then wait synchronously on the
results. While waiting on the buffer and holding the lock, additional
two way binder calls can be made into system server that are waiting to
acquire the same lock. If there are enough requests, we may run out of
binder threads and then the screen capture result can't be posted back
to system server because it needs a free binder thread. This will result
in a deadlock because the lock that the screenshot request is holding
can never be unlocked without a free binder thread. Binder threads will
never be freed up because they are waiting to acquire the lock.

The async screencapture code is still useful for cases where there's no
global lock being held while waiting on results or the results is posted
to another thread.

Test: Screenshots
Bug: 321263247
Change-Id: I259173a59f488995e13af8f7dd2ca98c3bbf8639

7 files changed, 45 insertions, 3 deletions

diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index 8d18551b69..83c2b7f703 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -56,6 +56,7 @@
 
 #include <android-base/thread_annotations.h>
 #include <gui/LayerStatePermissions.h>
+#include <gui/ScreenCaptureResults.h>
 #include <private/gui/ComposerService.h>
 #include <private/gui/ComposerServiceAIDL.h>
 
@@ -3138,11 +3139,19 @@ status_t ScreenshotClient::captureDisplay(DisplayId displayId, const gui::Captur
 }
 
 status_t ScreenshotClient::captureLayers(const LayerCaptureArgs& captureArgs,
-                                         const sp<IScreenCaptureListener>& captureListener) {
+                                         const sp<IScreenCaptureListener>& captureListener,
+                                         bool sync) {
     sp<gui::ISurfaceComposer> s(ComposerServiceAIDL::getComposerService());
     if (s == nullptr) return NO_INIT;
 
-    binder::Status status = s->captureLayers(captureArgs, captureListener);
+    binder::Status status;
+    if (sync) {
+        gui::ScreenCaptureResults captureResults;
+        status = s->captureLayersSync(captureArgs, &captureResults);
+        captureListener->onScreenCaptureCompleted(captureResults);
+    } else {
+        status = s->captureLayers(captureArgs, captureListener);
+    }
     return statusTFromBinderStatus(status);
 }
 
diff --git a/libs/gui/aidl/android/gui/ISurfaceComposer.aidl b/libs/gui/aidl/android/gui/ISurfaceComposer.aidl
index e3122bc300..51e01930d3 100644
--- a/libs/gui/aidl/android/gui/ISurfaceComposer.aidl
+++ b/libs/gui/aidl/android/gui/ISurfaceComposer.aidl
@@ -46,6 +46,7 @@ import android.gui.LayerCaptureArgs;
 import android.gui.LayerDebugInfo;
 import android.gui.OverlayProperties;
 import android.gui.PullAtomData;
+import android.gui.ScreenCaptureResults;
 import android.gui.ARect;
 import android.gui.SchedulingPolicy;
 import android.gui.StalledTransactionInfo;
@@ -245,6 +246,16 @@ interface ISurfaceComposer {
     /**
      * Capture a subtree of the layer hierarchy, potentially ignoring the root node.
      * This requires READ_FRAME_BUFFER permission. This function will fail if there
+     * is a secure window on screen. This is a blocking call and will return the
+     * ScreenCaptureResults, including the captured buffer. Because this is blocking, the
+     * caller doesn't care about the fence and the binder thread in SurfaceFlinger will wait
+     * on the fence to fire before returning the results.
+     */
+    ScreenCaptureResults captureLayersSync(in LayerCaptureArgs args);
+
+    /**
+     * Capture a subtree of the layer hierarchy, potentially ignoring the root node.
+     * This requires READ_FRAME_BUFFER permission. This function will fail if there
      * is a secure window on screen
      */
     oneway void captureLayers(in LayerCaptureArgs args, IScreenCaptureListener listener);
diff --git a/libs/gui/fuzzer/libgui_fuzzer_utils.h b/libs/gui/fuzzer/libgui_fuzzer_utils.h
index c952ba2e10..2bdbd43233 100644
--- a/libs/gui/fuzzer/libgui_fuzzer_utils.h
+++ b/libs/gui/fuzzer/libgui_fuzzer_utils.h
@@ -104,6 +104,8 @@ public:
                 (int64_t, const gui::CaptureArgs&, const sp<IScreenCaptureListener>&), (override));
     MOCK_METHOD(binder::Status, captureLayers,
                 (const LayerCaptureArgs&, const sp<IScreenCaptureListener>&), (override));
+    MOCK_METHOD(binder::Status, captureLayersSync,
+                (const LayerCaptureArgs&, gui::ScreenCaptureResults*), (override));
     MOCK_METHOD(binder::Status, clearAnimationFrameStats, (), (override));
     MOCK_METHOD(binder::Status, getAnimationFrameStats, (gui::FrameStats*), (override));
     MOCK_METHOD(binder::Status, overrideHdrTypes, (const sp<IBinder>&, const std::vector<int32_t>&),
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index 14e3dd583e..88a2c342e7 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -849,7 +849,8 @@ public:
     static status_t captureDisplay(const DisplayCaptureArgs&, const sp<IScreenCaptureListener>&);
     static status_t captureDisplay(DisplayId, const gui::CaptureArgs&,
                                    const sp<IScreenCaptureListener>&);
-    static status_t captureLayers(const LayerCaptureArgs&, const sp<IScreenCaptureListener>&);
+    static status_t captureLayers(const LayerCaptureArgs&, const sp<IScreenCaptureListener>&,
+                                  bool sync);
 
     [[deprecated]] static status_t captureDisplay(DisplayId id,
                                                   const sp<IScreenCaptureListener>& listener) {
diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp
index c6ea317949..577d2394c6 100644
--- a/libs/gui/tests/Surface_test.cpp
+++ b/libs/gui/tests/Surface_test.cpp
@@ -791,6 +791,10 @@ public:
         return binder::Status::ok();
     }
 
+    binder::Status captureLayersSync(const LayerCaptureArgs&, ScreenCaptureResults*) override {
+        return binder::Status::ok();
+    }
+
     binder::Status captureLayers(const LayerCaptureArgs&,
                                  const sp<IScreenCaptureListener>&) override {
         return binder::Status::ok();
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index fe5b159051..178828a5f6 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -116,6 +116,7 @@
 #include <common/FlagManager.h>
 #include <gui/LayerStatePermissions.h>
 #include <gui/SchedulingPolicy.h>
+#include <gui/SyncScreenCaptureListener.h>
 #include <ui/DisplayIdentification.h>
 #include "BackgroundExecutor.h"
 #include "Client.h"
@@ -7842,6 +7843,12 @@ void SurfaceFlinger::captureDisplay(DisplayId displayId, const CaptureArgs& args
                         kAllowProtected, kGrayscale, captureListener);
 }
 
+ScreenCaptureResults SurfaceFlinger::captureLayersSync(const LayerCaptureArgs& args) {
+    sp<SyncScreenCaptureListener> captureListener = sp<SyncScreenCaptureListener>::make();
+    captureLayers(args, captureListener);
+    return captureListener->waitForResults();
+}
+
 void SurfaceFlinger::captureLayers(const LayerCaptureArgs& args,
                                    const sp<IScreenCaptureListener>& captureListener) {
     ATRACE_CALL();
@@ -9686,6 +9693,12 @@ binder::Status SurfaceComposerAIDL::captureDisplayById(
     return binderStatusFromStatusT(NO_ERROR);
 }
 
+binder::Status SurfaceComposerAIDL::captureLayersSync(const LayerCaptureArgs& args,
+                                                      ScreenCaptureResults* outResults) {
+    *outResults = mFlinger->captureLayersSync(args);
+    return binderStatusFromStatusT(NO_ERROR);
+}
+
 binder::Status SurfaceComposerAIDL::captureLayers(
         const LayerCaptureArgs& args, const sp<IScreenCaptureListener>& captureListener) {
     mFlinger->captureLayers(args, captureListener);
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 6ae05d604a..24a1a108ce 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -560,6 +560,7 @@ private:
 
     void captureDisplay(const DisplayCaptureArgs&, const sp<IScreenCaptureListener>&);
     void captureDisplay(DisplayId, const CaptureArgs&, const sp<IScreenCaptureListener>&);
+    ScreenCaptureResults captureLayersSync(const LayerCaptureArgs&);
     void captureLayers(const LayerCaptureArgs&, const sp<IScreenCaptureListener>&);
 
     status_t getDisplayStats(const sp<IBinder>& displayToken, DisplayStatInfo* stats);
@@ -1546,6 +1547,7 @@ public:
                                       const sp<IScreenCaptureListener>&) override;
     binder::Status captureLayers(const LayerCaptureArgs&,
                                  const sp<IScreenCaptureListener>&) override;
+    binder::Status captureLayersSync(const LayerCaptureArgs&, ScreenCaptureResults* results);
 
     // TODO(b/239076119): Remove deprecated AIDL.
     [[deprecated]] binder::Status clearAnimationFrameStats() override {