Fix region sampling for secure layers

We were swapping a couple of boolean params when requesting
screenshots for region sampling, breaking region sampling for
secure layers in the process. Fix this by passing flags instead of
booleans in a long list of arguments and add a test to verify the
code path.

FLAG: EXEMPT bugfix
Fixes: 348944802
Test: presubmit
Change-Id: I58f12e5cf81cb59115c1b9c500ac1e18a8ec72e5
diff --git a/services/surfaceflinger/DisplayRenderArea.cpp b/services/surfaceflinger/DisplayRenderArea.cpp
index 55b395b..c63c738 100644
--- a/services/surfaceflinger/DisplayRenderArea.cpp
+++ b/services/surfaceflinger/DisplayRenderArea.cpp
@@ -22,22 +22,20 @@
 std::unique_ptr<RenderArea> DisplayRenderArea::create(wp<const DisplayDevice> displayWeak,
                                                       const Rect& sourceCrop, ui::Size reqSize,
                                                       ui::Dataspace reqDataSpace,
-                                                      bool hintForSeamlessTransition,
-                                                      bool allowSecureLayers) {
+                                                      ftl::Flags<Options> options) {
     if (auto display = displayWeak.promote()) {
         // Using new to access a private constructor.
-        return std::unique_ptr<DisplayRenderArea>(
-                new DisplayRenderArea(std::move(display), sourceCrop, reqSize, reqDataSpace,
-                                      hintForSeamlessTransition, allowSecureLayers));
+        return std::unique_ptr<DisplayRenderArea>(new DisplayRenderArea(std::move(display),
+                                                                        sourceCrop, reqSize,
+                                                                        reqDataSpace, options));
     }
     return nullptr;
 }
 
 DisplayRenderArea::DisplayRenderArea(sp<const DisplayDevice> display, const Rect& sourceCrop,
                                      ui::Size reqSize, ui::Dataspace reqDataSpace,
-                                     bool hintForSeamlessTransition, bool allowSecureLayers)
-      : RenderArea(reqSize, CaptureFill::OPAQUE, reqDataSpace, hintForSeamlessTransition,
-                   allowSecureLayers),
+                                     ftl::Flags<Options> options)
+      : RenderArea(reqSize, CaptureFill::OPAQUE, reqDataSpace, options),
         mDisplay(std::move(display)),
         mSourceCrop(sourceCrop) {}
 
@@ -46,7 +44,7 @@
 }
 
 bool DisplayRenderArea::isSecure() const {
-    return mAllowSecureLayers && mDisplay->isSecure();
+    return mOptions.test(Options::CAPTURE_SECURE_LAYERS) && mDisplay->isSecure();
 }
 
 sp<const DisplayDevice> DisplayRenderArea::getDisplayDevice() const {
diff --git a/services/surfaceflinger/DisplayRenderArea.h b/services/surfaceflinger/DisplayRenderArea.h
index 4555a9e..677d019 100644
--- a/services/surfaceflinger/DisplayRenderArea.h
+++ b/services/surfaceflinger/DisplayRenderArea.h
@@ -29,8 +29,7 @@
 public:
     static std::unique_ptr<RenderArea> create(wp<const DisplayDevice>, const Rect& sourceCrop,
                                               ui::Size reqSize, ui::Dataspace,
-                                              bool hintForSeamlessTransition,
-                                              bool allowSecureLayers = true);
+                                              ftl::Flags<Options> options);
 
     const ui::Transform& getTransform() const override;
     bool isSecure() const override;
@@ -39,7 +38,7 @@
 
 private:
     DisplayRenderArea(sp<const DisplayDevice>, const Rect& sourceCrop, ui::Size reqSize,
-                      ui::Dataspace, bool hintForSeamlessTransition, bool allowSecureLayers = true);
+                      ui::Dataspace, ftl::Flags<Options> options);
 
     const sp<const DisplayDevice> mDisplay;
     const Rect mSourceCrop;
diff --git a/services/surfaceflinger/LayerRenderArea.cpp b/services/surfaceflinger/LayerRenderArea.cpp
index f323ce7..bfe6d2a 100644
--- a/services/surfaceflinger/LayerRenderArea.cpp
+++ b/services/surfaceflinger/LayerRenderArea.cpp
@@ -27,10 +27,9 @@
 
 LayerRenderArea::LayerRenderArea(sp<Layer> layer, frontend::LayerSnapshot layerSnapshot,
                                  const Rect& crop, ui::Size reqSize, ui::Dataspace reqDataSpace,
-                                 bool allowSecureLayers, const ui::Transform& layerTransform,
-                                 const Rect& layerBufferSize, bool hintForSeamlessTransition)
-      : RenderArea(reqSize, CaptureFill::CLEAR, reqDataSpace, hintForSeamlessTransition,
-                   allowSecureLayers),
+                                 const ui::Transform& layerTransform, const Rect& layerBufferSize,
+                                 ftl::Flags<RenderArea::Options> options)
+      : RenderArea(reqSize, CaptureFill::CLEAR, reqDataSpace, options),
         mLayer(std::move(layer)),
         mLayerSnapshot(std::move(layerSnapshot)),
         mLayerBufferSize(layerBufferSize),
@@ -42,7 +41,7 @@
 }
 
 bool LayerRenderArea::isSecure() const {
-    return mAllowSecureLayers;
+    return mOptions.test(Options::CAPTURE_SECURE_LAYERS);
 }
 
 sp<const DisplayDevice> LayerRenderArea::getDisplayDevice() const {
diff --git a/services/surfaceflinger/LayerRenderArea.h b/services/surfaceflinger/LayerRenderArea.h
index a12bfca..f72c7c7 100644
--- a/services/surfaceflinger/LayerRenderArea.h
+++ b/services/surfaceflinger/LayerRenderArea.h
@@ -33,9 +33,9 @@
 class LayerRenderArea : public RenderArea {
 public:
     LayerRenderArea(sp<Layer> layer, frontend::LayerSnapshot layerSnapshot, const Rect& crop,
-                    ui::Size reqSize, ui::Dataspace reqDataSpace, bool allowSecureLayers,
+                    ui::Size reqSize, ui::Dataspace reqDataSpace,
                     const ui::Transform& layerTransform, const Rect& layerBufferSize,
-                    bool hintForSeamlessTransition);
+                    ftl::Flags<RenderArea::Options> options);
 
     const ui::Transform& getTransform() const override;
     bool isSecure() const override;
diff --git a/services/surfaceflinger/RegionSamplingThread.cpp b/services/surfaceflinger/RegionSamplingThread.cpp
index 5add290..e579eb0 100644
--- a/services/surfaceflinger/RegionSamplingThread.cpp
+++ b/services/surfaceflinger/RegionSamplingThread.cpp
@@ -277,7 +277,6 @@
     }
 
     const Rect sampledBounds = sampleRegion.bounds();
-    constexpr bool kHintForSeamlessTransition = false;
 
     std::unordered_set<sp<IRegionSamplingListener>, SpHash<IRegionSamplingListener>> listeners;
 
@@ -350,9 +349,8 @@
 
     SurfaceFlinger::RenderAreaBuilderVariant
             renderAreaBuilder(std::in_place_type<DisplayRenderAreaBuilder>, sampledBounds,
-                              sampledBounds.getSize(), ui::Dataspace::V0_SRGB,
-                              kHintForSeamlessTransition, true /* captureSecureLayers */,
-                              displayWeak);
+                              sampledBounds.getSize(), ui::Dataspace::V0_SRGB, displayWeak,
+                              RenderArea::Options::CAPTURE_SECURE_LAYERS);
 
     FenceResult fenceResult;
     if (FlagManager::getInstance().single_hop_screenshot() &&
diff --git a/services/surfaceflinger/RenderArea.h b/services/surfaceflinger/RenderArea.h
index e8d20af..034e467 100644
--- a/services/surfaceflinger/RenderArea.h
+++ b/services/surfaceflinger/RenderArea.h
@@ -21,16 +21,23 @@
 class RenderArea {
 public:
     enum class CaptureFill {CLEAR, OPAQUE};
+    enum class Options {
+        // If not set, the secure layer would be blacked out or skipped
+        // when rendered to an insecure render area
+        CAPTURE_SECURE_LAYERS = 1 << 0,
 
+        // If set, the render result may be used for system animations
+        // that must preserve the exact colors of the display
+        HINT_FOR_SEAMLESS_TRANSITION = 1 << 1,
+    };
     static float getCaptureFillValue(CaptureFill captureFill);
 
     RenderArea(ui::Size reqSize, CaptureFill captureFill, ui::Dataspace reqDataSpace,
-               bool hintForSeamlessTransition, bool allowSecureLayers = false)
-          : mAllowSecureLayers(allowSecureLayers),
+               ftl::Flags<Options> options)
+          : mOptions(options),
             mReqSize(reqSize),
             mReqDataSpace(reqDataSpace),
-            mCaptureFill(captureFill),
-            mHintForSeamlessTransition(hintForSeamlessTransition) {}
+            mCaptureFill(captureFill) {}
 
     static std::function<std::vector<std::pair<Layer*, sp<LayerFE>>>()> fromTraverseLayersLambda(
             std::function<void(const LayerVector::Visitor&)> traverseLayers) {
@@ -90,16 +97,17 @@
 
     // Returns whether the render result may be used for system animations that
     // must preserve the exact colors of the display.
-    bool getHintForSeamlessTransition() const { return mHintForSeamlessTransition; }
+    bool getHintForSeamlessTransition() const {
+        return mOptions.test(Options::HINT_FOR_SEAMLESS_TRANSITION);
+    }
 
 protected:
-    const bool mAllowSecureLayers;
+    ftl::Flags<Options> mOptions;
 
 private:
     const ui::Size mReqSize;
     const ui::Dataspace mReqDataSpace;
     const CaptureFill mCaptureFill;
-    const bool mHintForSeamlessTransition;
 };
 
 } // namespace android
diff --git a/services/surfaceflinger/RenderAreaBuilder.h b/services/surfaceflinger/RenderAreaBuilder.h
index a25c6e0..599fa7e 100644
--- a/services/surfaceflinger/RenderAreaBuilder.h
+++ b/services/surfaceflinger/RenderAreaBuilder.h
@@ -36,50 +36,34 @@
     // Composition data space of the render area
     ui::Dataspace reqDataSpace;
 
-    // If true, the secure layer would be blacked out or skipped
-    // when rendered to an insecure render area
-    bool allowSecureLayers;
-
-    // If true, the render result may be used for system animations
-    // that must preserve the exact colors of the display
-    bool hintForSeamlessTransition;
-
+    ftl::Flags<RenderArea::Options> options;
     virtual std::unique_ptr<RenderArea> build() const = 0;
 
     RenderAreaBuilder(Rect crop, ui::Size reqSize, ui::Dataspace reqDataSpace,
-                      bool allowSecureLayers, bool hintForSeamlessTransition)
-          : crop(crop),
-            reqSize(reqSize),
-            reqDataSpace(reqDataSpace),
-            allowSecureLayers(allowSecureLayers),
-            hintForSeamlessTransition(hintForSeamlessTransition) {}
+                      ftl::Flags<RenderArea::Options> options)
+          : crop(crop), reqSize(reqSize), reqDataSpace(reqDataSpace), options(options) {}
 
     virtual ~RenderAreaBuilder() = default;
 };
 
 struct DisplayRenderAreaBuilder : RenderAreaBuilder {
     DisplayRenderAreaBuilder(Rect crop, ui::Size reqSize, ui::Dataspace reqDataSpace,
-                             bool allowSecureLayers, bool hintForSeamlessTransition,
-                             wp<const DisplayDevice> displayWeak)
-          : RenderAreaBuilder(crop, reqSize, reqDataSpace, allowSecureLayers,
-                              hintForSeamlessTransition),
-            displayWeak(displayWeak) {}
+                             wp<const DisplayDevice> displayWeak,
+                             ftl::Flags<RenderArea::Options> options)
+          : RenderAreaBuilder(crop, reqSize, reqDataSpace, options), displayWeak(displayWeak) {}
 
     // Display that render area will be on
     wp<const DisplayDevice> displayWeak;
 
     std::unique_ptr<RenderArea> build() const override {
-        return DisplayRenderArea::create(displayWeak, crop, reqSize, reqDataSpace,
-                                         hintForSeamlessTransition, allowSecureLayers);
+        return DisplayRenderArea::create(displayWeak, crop, reqSize, reqDataSpace, options);
     }
 };
 
 struct LayerRenderAreaBuilder : RenderAreaBuilder {
-    LayerRenderAreaBuilder(Rect crop, ui::Size reqSize, ui::Dataspace reqDataSpace,
-                           bool allowSecureLayers, bool hintForSeamlessTransition, sp<Layer> layer,
-                           bool childrenOnly)
-          : RenderAreaBuilder(crop, reqSize, reqDataSpace, allowSecureLayers,
-                              hintForSeamlessTransition),
+    LayerRenderAreaBuilder(Rect crop, ui::Size reqSize, ui::Dataspace reqDataSpace, sp<Layer> layer,
+                           bool childrenOnly, ftl::Flags<RenderArea::Options> options)
+          : RenderAreaBuilder(crop, reqSize, reqDataSpace, options),
             layer(layer),
             childrenOnly(childrenOnly) {}
 
@@ -110,8 +94,8 @@
 
     std::unique_ptr<RenderArea> build() const override {
         return std::make_unique<LayerRenderArea>(layer, std::move(layerSnapshot), crop, reqSize,
-                                                 reqDataSpace, allowSecureLayers, layerTransform,
-                                                 layerBufferSize, hintForSeamlessTransition);
+                                                 reqDataSpace, layerTransform, layerBufferSize,
+                                                 options);
     }
 };
 
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 5b40acf..13a1464 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -7958,10 +7958,13 @@
     GetLayerSnapshotsFunction getLayerSnapshotsFn =
             getLayerSnapshotsForScreenshots(layerStack, args.uid, std::move(excludeLayerIds));
 
+    ftl::Flags<RenderArea::Options> options;
+    if (args.captureSecureLayers) options |= RenderArea::Options::CAPTURE_SECURE_LAYERS;
+    if (args.hintForSeamlessTransition)
+        options |= RenderArea::Options::HINT_FOR_SEAMLESS_TRANSITION;
     captureScreenCommon(RenderAreaBuilderVariant(std::in_place_type<DisplayRenderAreaBuilder>,
                                                  args.sourceCrop, reqSize, args.dataspace,
-                                                 args.hintForSeamlessTransition,
-                                                 args.captureSecureLayers, displayWeak),
+                                                 displayWeak, options),
                         getLayerSnapshotsFn, reqSize, args.pixelFormat, args.allowProtected,
                         args.grayscale, captureListener);
 }
@@ -8012,10 +8015,12 @@
     constexpr bool kAllowProtected = false;
     constexpr bool kGrayscale = false;
 
+    ftl::Flags<RenderArea::Options> options;
+    if (args.hintForSeamlessTransition)
+        options |= RenderArea::Options::HINT_FOR_SEAMLESS_TRANSITION;
     captureScreenCommon(RenderAreaBuilderVariant(std::in_place_type<DisplayRenderAreaBuilder>,
-                                                 Rect(), size, args.dataspace,
-                                                 args.hintForSeamlessTransition,
-                                                 false /* captureSecureLayers */, displayWeak),
+                                                 Rect(), size, args.dataspace, displayWeak,
+                                                 options),
                         getLayerSnapshotsFn, size, args.pixelFormat, kAllowProtected, kGrayscale,
                         captureListener);
 }
@@ -8114,10 +8119,13 @@
         return;
     }
 
+    ftl::Flags<RenderArea::Options> options;
+    if (args.captureSecureLayers) options |= RenderArea::Options::CAPTURE_SECURE_LAYERS;
+    if (args.hintForSeamlessTransition)
+        options |= RenderArea::Options::HINT_FOR_SEAMLESS_TRANSITION;
     captureScreenCommon(RenderAreaBuilderVariant(std::in_place_type<LayerRenderAreaBuilder>, crop,
-                                                 reqSize, dataspace, args.captureSecureLayers,
-                                                 args.hintForSeamlessTransition, parent,
-                                                 args.childrenOnly),
+                                                 reqSize, dataspace, parent, args.childrenOnly,
+                                                 options),
                         getLayerSnapshotsFn, reqSize, args.pixelFormat, args.allowProtected,
                         args.grayscale, captureListener);
 }
diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
index 08973de..cdd77fe 100644
--- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
@@ -70,6 +70,7 @@
 
 using FakeHwcDisplayInjector = TestableSurfaceFlinger::FakeHwcDisplayInjector;
 using FakeDisplayDeviceInjector = TestableSurfaceFlinger::FakeDisplayDeviceInjector;
+using namespace ftl::flag_operators;
 
 constexpr hal::HWDisplayId HWC_DISPLAY = FakeHwcDisplayInjector::DEFAULT_HWC_DISPLAY_ID;
 constexpr hal::HWLayerId HWC_LAYER = 5000;
@@ -197,8 +198,11 @@
     const Rect sourceCrop(0, 0, DEFAULT_DISPLAY_WIDTH, DEFAULT_DISPLAY_HEIGHT);
     constexpr bool regionSampling = false;
 
-    auto renderArea = DisplayRenderArea::create(mDisplay, sourceCrop, sourceCrop.getSize(),
-                                                ui::Dataspace::V0_SRGB, true, true);
+    auto renderArea =
+            DisplayRenderArea::create(mDisplay, sourceCrop, sourceCrop.getSize(),
+                                      ui::Dataspace::V0_SRGB,
+                                      RenderArea::Options::CAPTURE_SECURE_LAYERS |
+                                              RenderArea::Options::HINT_FOR_SEAMLESS_TRANSITION);
 
     auto traverseLayers = [this](const LayerVector::Visitor& visitor) {
         return mFlinger.traverseLayersInLayerStack(mDisplay->getLayerStack(),