Merge changes from topic "multiple-picture-listeners" into main

* changes:
  Add additional ActivePictureTracker test coverage and rename tests
  Support multiple active picture listeners
  Rename of ActivePictureUpdater to ActivePictureTracker

Author: Brian3031

METADATA

@@ -1,3 +1,3 @@
-third_party {
+ third_party {
   license_type: NOTICE
 }

libs/gui/SurfaceComposerClient.cpp

@@ -3293,10 +3293,17 @@ status_t SurfaceComposerClient::removeHdrLayerInfoListener(
     return statusTFromBinderStatus(status);
 }
 
-status_t SurfaceComposerClient::setActivePictureListener(
+status_t SurfaceComposerClient::addActivePictureListener(
         const sp<gui::IActivePictureListener>& listener) {
     binder::Status status =
-            ComposerServiceAIDL::getComposerService()->setActivePictureListener(listener);
+            ComposerServiceAIDL::getComposerService()->addActivePictureListener(listener);
+    return statusTFromBinderStatus(status);
+}
+
+status_t SurfaceComposerClient::removeActivePictureListener(
+        const sp<gui::IActivePictureListener>& listener) {
+    binder::Status status =
+            ComposerServiceAIDL::getComposerService()->removeActivePictureListener(listener);
     return statusTFromBinderStatus(status);
 }
 

libs/gui/aidl/android/gui/ISurfaceComposer.aidl

@@ -607,8 +607,14 @@ interface ISurfaceComposer {
     oneway void removeJankListener(int layerId, IJankListener listener, long afterVsync);
 
     /**
-     * Sets the listener used to monitor visible content that is being processed with picture
+     * Adds a listener used to monitor visible content that is being processed with picture
      * profiles.
      */
-    oneway void setActivePictureListener(IActivePictureListener listener);
+    oneway void addActivePictureListener(IActivePictureListener listener);
+
+    /**
+     * Removes a listener used to monitor visible content that is being processed with picture
+     * profiles.
+     */
+    oneway void removeActivePictureListener(IActivePictureListener listener);
 }

libs/gui/include/gui/SurfaceComposerClient.h

@@ -298,7 +298,9 @@ class SurfaceComposerClient : public RefBase
     static status_t removeHdrLayerInfoListener(const sp<IBinder>& displayToken,
                                                const sp<gui::IHdrLayerInfoListener>& listener);
 
-    static status_t setActivePictureListener(const sp<gui::IActivePictureListener>& listener);
+    static status_t addActivePictureListener(const sp<gui::IActivePictureListener>& listener);
+
+    static status_t removeActivePictureListener(const sp<gui::IActivePictureListener>& listener);
 
     /*
      * Sends a power boost to the composer. This function is asynchronous.

libs/gui/tests/Surface_test.cpp

@@ -1016,7 +1016,11 @@ class FakeSurfaceComposerAIDL : public gui::ISurfaceComposer {
         return binder::Status::ok();
     }
 
-    binder::Status setActivePictureListener(const sp<gui::IActivePictureListener>&) {
+    binder::Status addActivePictureListener(const sp<gui::IActivePictureListener>&) {
+        return binder::Status::ok();
+    }
+
+    binder::Status removeActivePictureListener(const sp<gui::IActivePictureListener>&) {
         return binder::Status::ok();
     }
 

…/surfaceflinger/ActivePictureUpdater.cpp …/surfaceflinger/ActivePictureTracker.cppservices/surfaceflinger/ActivePictureUpdater.cpp renamed to services/surfaceflinger/ActivePictureTracker.cpp services/surfaceflinger/ActivePictureUpdater.cpp renamed to services/surfaceflinger/ActivePictureTracker.cpp

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-#include "ActivePictureUpdater.h"
+#include "ActivePictureTracker.h"
 
 #include <algorithm>
 
@@ -23,7 +23,10 @@
 
 namespace android {
 
-void ActivePictureUpdater::onLayerComposed(const Layer& layer, const LayerFE& layerFE,
+using gui::ActivePicture;
+using gui::IActivePictureListener;
+
+void ActivePictureTracker::onLayerComposed(const Layer& layer, const LayerFE& layerFE,
                                            const CompositionResult& result) {
     if (result.wasPictureProfileCommitted) {
         gui::ActivePicture picture;
@@ -39,10 +42,47 @@ void ActivePictureUpdater::onLayerComposed(const Layer& layer, const LayerFE& la
     }
 }
 
-bool ActivePictureUpdater::updateAndHasChanged() {
+void ActivePictureTracker::updateAndNotifyListeners(const Listeners& listenersToAdd,
+                                                    const Listeners& listenersToRemove) {
+    Listeners newListeners = updateListeners(listenersToAdd, listenersToRemove);
+    if (updateAndHasChanged()) {
+        for (auto listener : mListeners) {
+            listener->onActivePicturesChanged(getActivePictures());
+        }
+    } else {
+        for (auto listener : newListeners) {
+            listener->onActivePicturesChanged(getActivePictures());
+        }
+    }
+}
+
+ActivePictureTracker::Listeners ActivePictureTracker::updateListeners(
+        const Listeners& listenersToAdd, const Listeners& listenersToRemove) {
+    Listeners newListeners;
+    for (auto listener : listenersToRemove) {
+        std::erase_if(mListeners, [listener](const sp<IActivePictureListener>& otherListener) {
+            return IInterface::asBinder(listener) == IInterface::asBinder(otherListener);
+        });
+    }
+    for (auto listener : listenersToAdd) {
+        if (std::find_if(mListeners.begin(), mListeners.end(),
+                         [listener](const sp<IActivePictureListener>& otherListener) {
+                             return IInterface::asBinder(listener) ==
+                                     IInterface::asBinder(otherListener);
+                         }) == mListeners.end()) {
+            newListeners.push_back(listener);
+        }
+    }
+    for (auto listener : newListeners) {
+        mListeners.push_back(listener);
+    }
+    return newListeners;
+}
+
+bool ActivePictureTracker::updateAndHasChanged() {
     bool hasChanged = true;
     if (mNewActivePictures.size() == mOldActivePictures.size()) {
-        auto compare = [](const gui::ActivePicture& lhs, const gui::ActivePicture& rhs) -> int {
+        auto compare = [](const ActivePicture& lhs, const ActivePicture& rhs) -> int {
             if (lhs.layerId == rhs.layerId) {
                 return lhs.pictureProfileId < rhs.pictureProfileId;
             }
@@ -59,7 +99,7 @@ bool ActivePictureUpdater::updateAndHasChanged() {
     return hasChanged;
 }
 
-const std::vector<gui::ActivePicture>& ActivePictureUpdater::getActivePictures() const {
+const std::vector<ActivePicture>& ActivePictureTracker::getActivePictures() const {
     return mOldActivePictures;
 }
 

…es/surfaceflinger/ActivePictureUpdater.h …es/surfaceflinger/ActivePictureTracker.hservices/surfaceflinger/ActivePictureUpdater.h renamed to services/surfaceflinger/ActivePictureTracker.h services/surfaceflinger/ActivePictureUpdater.h renamed to services/surfaceflinger/ActivePictureTracker.h

@@ -19,6 +19,7 @@
 #include <vector>
 
 #include <android/gui/ActivePicture.h>
+#include <android/gui/IActivePictureListener.h>
 
 namespace android {
 
@@ -27,21 +28,28 @@ class LayerFE;
 struct CompositionResult;
 
 // Keeps track of active pictures - layers that are undergoing picture processing.
-class ActivePictureUpdater {
+class ActivePictureTracker {
 public:
+    typedef std::vector<sp<gui::IActivePictureListener>> Listeners;
+
     // Called for each visible layer when SurfaceFlinger finishes composing.
     void onLayerComposed(const Layer& layer, const LayerFE& layerFE,
                          const CompositionResult& result);
 
     // Update internals and return whether the set of active pictures have changed.
-    bool updateAndHasChanged();
+    void updateAndNotifyListeners(const Listeners& activePictureListenersToAdd,
+                                  const Listeners& activePictureListenersToRemove);
 
     // The current set of active pictures.
     const std::vector<gui::ActivePicture>& getActivePictures() const;
 
 private:
+    Listeners updateListeners(const Listeners& listenersToAdd, const Listeners& listenersToRemove);
+    bool updateAndHasChanged();
+
     std::vector<gui::ActivePicture> mOldActivePictures;
     std::vector<gui::ActivePicture> mNewActivePictures;
+    Listeners mListeners;
 };
 
 } // namespace android

services/surfaceflinger/Android.bp

@@ -198,7 +198,7 @@ filegroup {
     name: "libsurfaceflinger_sources",
     srcs: [
         ":libsurfaceflinger_backend_sources",
-        "ActivePictureUpdater.cpp",
+        "ActivePictureTracker.cpp",
         "BackgroundExecutor.cpp",
         "Client.cpp",
         "ClientCache.cpp",

services/surfaceflinger/SurfaceFlinger.cpp

@@ -126,7 +126,6 @@
 #include <gui/SchedulingPolicy.h>
 #include <gui/SyncScreenCaptureListener.h>
 #include <ui/DisplayIdentification.h>
-#include "ActivePictureUpdater.h"
 #include "BackgroundExecutor.h"
 #include "Client.h"
 #include "ClientCache.h"
@@ -2952,7 +2951,7 @@ CompositeResultsPerDisplay SurfaceFlinger::composite(
             layer->setWasClientComposed(compositionResult.lastClientCompositionFence);
         }
         if (com_android_graphics_libgui_flags_apply_picture_profiles()) {
-            mActivePictureUpdater.onLayerComposed(*layer, *layerFE, compositionResult);
+            mActivePictureTracker.onLayerComposed(*layer, *layerFE, compositionResult);
         }
     }
 
@@ -3264,8 +3263,8 @@ void SurfaceFlinger::onCompositionPresented(PhysicalDisplayId pacesetterId,
     std::vector<std::pair<std::shared_ptr<compositionengine::Display>, sp<HdrLayerInfoReporter>>>
             hdrInfoListeners;
     bool haveNewHdrInfoListeners = false;
-    sp<gui::IActivePictureListener> activePictureListener;
-    bool haveNewActivePictureListener = false;
+    ActivePictureTracker::Listeners activePictureListenersToAdd;
+    ActivePictureTracker::Listeners activePictureListenersToRemove;
     {
         Mutex::Autolock lock(mStateLock);
         if (mFpsReporter) {
@@ -3287,9 +3286,8 @@ void SurfaceFlinger::onCompositionPresented(PhysicalDisplayId pacesetterId,
         haveNewHdrInfoListeners = mAddingHDRLayerInfoListener; // grab this with state lock
         mAddingHDRLayerInfoListener = false;
 
-        activePictureListener = mActivePictureListener;
-        haveNewActivePictureListener = mHaveNewActivePictureListener;
-        mHaveNewActivePictureListener = false;
+        std::swap(activePictureListenersToAdd, mActivePictureListenersToAdd);
+        std::swap(activePictureListenersToRemove, mActivePictureListenersToRemove);
     }
 
     if (haveNewHdrInfoListeners || mHdrLayerInfoChanged) {
@@ -3353,14 +3351,10 @@ void SurfaceFlinger::onCompositionPresented(PhysicalDisplayId pacesetterId,
     mHdrLayerInfoChanged = false;
 
     if (com_android_graphics_libgui_flags_apply_picture_profiles()) {
-        // Track, update and notify changes to active pictures - layers that are undergoing picture
-        // processing
-        if (mActivePictureUpdater.updateAndHasChanged() || haveNewActivePictureListener) {
-            if (activePictureListener) {
-                activePictureListener->onActivePicturesChanged(
-                        mActivePictureUpdater.getActivePictures());
-            }
-        }
+        // Track, update and notify changes to active pictures - layers that are undergoing
+        // picture processing
+        mActivePictureTracker.updateAndNotifyListeners(activePictureListenersToAdd,
+                                                       activePictureListenersToRemove);
     }
 
     mTransactionCallbackInvoker.sendCallbacks(false /* onCommitOnly */);
@@ -8226,12 +8220,20 @@ void SurfaceFlinger::updateHdcpLevels(hal::HWDisplayId hwcDisplayId, int32_t con
     }));
 }
 
-void SurfaceFlinger::setActivePictureListener(const sp<gui::IActivePictureListener>& listener) {
-    if (com_android_graphics_libgui_flags_apply_picture_profiles()) {
-        Mutex::Autolock lock(mStateLock);
-        mActivePictureListener = listener;
-        mHaveNewActivePictureListener = listener != nullptr;
-    }
+void SurfaceFlinger::addActivePictureListener(const sp<gui::IActivePictureListener>& listener) {
+    Mutex::Autolock lock(mStateLock);
+    std::erase_if(mActivePictureListenersToRemove, [listener](const auto& otherListener) {
+        return IInterface::asBinder(listener) == IInterface::asBinder(otherListener);
+    });
+    mActivePictureListenersToAdd.push_back(listener);
+}
+
+void SurfaceFlinger::removeActivePictureListener(const sp<gui::IActivePictureListener>& listener) {
+    Mutex::Autolock lock(mStateLock);
+    std::erase_if(mActivePictureListenersToAdd, [listener](const auto& otherListener) {
+        return IInterface::asBinder(listener) == IInterface::asBinder(otherListener);
+    });
+    mActivePictureListenersToRemove.push_back(listener);
 }
 
 std::shared_ptr<renderengine::ExternalTexture> SurfaceFlinger::getExternalTextureFromBufferData(
@@ -9184,11 +9186,20 @@ binder::Status SurfaceComposerAIDL::removeHdrLayerInfoListener(
     return binderStatusFromStatusT(status);
 }
 
-binder::Status SurfaceComposerAIDL::setActivePictureListener(
+binder::Status SurfaceComposerAIDL::addActivePictureListener(
+        const sp<gui::IActivePictureListener>& listener) {
+    status_t status = checkObservePictureProfilesPermission();
+    if (status == OK) {
+        mFlinger->addActivePictureListener(listener);
+    }
+    return binderStatusFromStatusT(status);
+}
+
+binder::Status SurfaceComposerAIDL::removeActivePictureListener(
         const sp<gui::IActivePictureListener>& listener) {
     status_t status = checkObservePictureProfilesPermission();
     if (status == OK) {
-        mFlinger->setActivePictureListener(listener);
+        mFlinger->removeActivePictureListener(listener);
     }
     return binderStatusFromStatusT(status);
 }

services/surfaceflinger/SurfaceFlinger.h

@@ -69,7 +69,7 @@
 #include <ui/FenceResult.h>
 
 #include <common/FlagManager.h>
-#include "ActivePictureUpdater.h"
+#include "ActivePictureTracker.h"
 #include "BackgroundExecutor.h"
 #include "Display/DisplayModeController.h"
 #include "Display/PhysicalDisplay.h"
@@ -667,7 +667,9 @@ class SurfaceFlinger : public BnSurfaceComposer,
 
     void updateHdcpLevels(hal::HWDisplayId hwcDisplayId, int32_t connectedLevel, int32_t maxLevel);
 
-    void setActivePictureListener(const sp<gui::IActivePictureListener>& listener);
+    void addActivePictureListener(const sp<gui::IActivePictureListener>& listener);
+
+    void removeActivePictureListener(const sp<gui::IActivePictureListener>& listener);
 
     // IBinder::DeathRecipient overrides:
     void binderDied(const wp<IBinder>& who) override;
@@ -1407,9 +1409,9 @@ class SurfaceFlinger : public BnSurfaceComposer,
     std::unordered_map<DisplayId, sp<HdrLayerInfoReporter>> mHdrLayerInfoListeners
             GUARDED_BY(mStateLock);
 
-    sp<gui::IActivePictureListener> mActivePictureListener GUARDED_BY(mStateLock);
-    bool mHaveNewActivePictureListener GUARDED_BY(mStateLock);
-    ActivePictureUpdater mActivePictureUpdater GUARDED_BY(kMainThreadContext);
+    ActivePictureTracker mActivePictureTracker GUARDED_BY(kMainThreadContext);
+    ActivePictureTracker::Listeners mActivePictureListenersToAdd GUARDED_BY(mStateLock);
+    ActivePictureTracker::Listeners mActivePictureListenersToRemove GUARDED_BY(mStateLock);
 
     std::atomic<ui::Transform::RotationFlags> mActiveDisplayTransformHint;
 
@@ -1646,8 +1648,8 @@ class SurfaceComposerAIDL : public gui::BnSurfaceComposer {
     binder::Status flushJankData(int32_t layerId) override;
     binder::Status removeJankListener(int32_t layerId, const sp<gui::IJankListener>& listener,
                                       int64_t afterVsync) override;
-    binder::Status setActivePictureListener(const sp<gui::IActivePictureListener>& listener);
-    binder::Status clearActivePictureListener();
+    binder::Status addActivePictureListener(const sp<gui::IActivePictureListener>& listener);
+    binder::Status removeActivePictureListener(const sp<gui::IActivePictureListener>& listener);
 
 private:
     static const constexpr bool kUsePermissionCache = true;