Clean up uses of refreshStartTime for stats.

refreshStartTime is keeps track of frame refreshes for
stats and can be set from SurfaceFlinger. This makes
subsequent changes in refactoring
SurfaceFlinger#composite cleaner and makes tracking the
metric more consistent between changes.

The refreshStartTime field can be removed entirely from
LayerFE's CompositionResult and removed as an argument
in LayerFE#onPreComposition.

Bug: b/294936197, b/329275965
Test: presubmit
Change-Id: I97988315f8982d05aec4b2684d4f5f1826a0fefa

8 files changed, 25 insertions, 24 deletions

diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h
index 18a96f4de7..843b5c5c82 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h
@@ -19,6 +19,7 @@
 #include <chrono>
 #include <optional>
 #include <vector>
+#include "utils/Timers.h"
 
 #include <compositionengine/Display.h>
 #include <compositionengine/LayerFE.h>
@@ -105,6 +106,9 @@ struct CompositionRefreshArgs {
     bool hasTrustedPresentationListener = false;
 
     ICEPowerCallback* powerCallback = nullptr;
+
+    // System time for when frame refresh starts. Used for stats.
+    nsecs_t refreshStartTime = 0;
 };
 
 } // namespace android::compositionengine
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h
index a1d61323a8..a499928dd0 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h
@@ -58,8 +58,7 @@ public:
     // Called before composition starts. Should return true if this layer has
     // pending updates which would require an extra display refresh cycle to
     // process.
-    virtual bool onPreComposition(nsecs_t refreshStartTime,
-                                  bool updatingOutputGeometryThisFrame) = 0;
+    virtual bool onPreComposition(bool updatingOutputGeometryThisFrame) = 0;
 
     struct ClientCompositionTargetSettings {
         enum class BlurSetting {
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h
index 15e4577ae0..1b8cc2758f 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h
@@ -43,7 +43,7 @@ public:
 
     MOCK_CONST_METHOD0(getCompositionState, const LayerFECompositionState*());
 
-    MOCK_METHOD2(onPreComposition, bool(nsecs_t, bool));
+    MOCK_METHOD1(onPreComposition, bool(bool));
 
     MOCK_CONST_METHOD1(prepareClientComposition,
                        std::optional<compositionengine::LayerFE::LayerSettings>(
diff --git a/services/surfaceflinger/CompositionEngine/src/CompositionEngine.cpp b/services/surfaceflinger/CompositionEngine/src/CompositionEngine.cpp
index d87eae3812..b4702085b6 100644
--- a/services/surfaceflinger/CompositionEngine/src/CompositionEngine.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/CompositionEngine.cpp
@@ -181,10 +181,10 @@ void CompositionEngine::preComposition(CompositionRefreshArgs& args) {
 
     bool needsAnotherUpdate = false;
 
-    mRefreshStartTime = systemTime(SYSTEM_TIME_MONOTONIC);
+    mRefreshStartTime = args.refreshStartTime;
 
     for (auto& layer : args.layers) {
-        if (layer->onPreComposition(mRefreshStartTime, args.updatingOutputGeometryThisFrame)) {
+        if (layer->onPreComposition(args.updatingOutputGeometryThisFrame)) {
             needsAnotherUpdate = true;
         }
     }
diff --git a/services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp b/services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp
index da578e2046..042010edd5 100644
--- a/services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp
@@ -214,6 +214,7 @@ struct CompositionTestPreComposition : public CompositionEngineTest {
 
 TEST_F(CompositionTestPreComposition, preCompositionSetsFrameTimestamp) {
     const nsecs_t before = systemTime(SYSTEM_TIME_MONOTONIC);
+    mRefreshArgs.refreshStartTime = systemTime(SYSTEM_TIME_MONOTONIC);
     mEngine.preComposition(mRefreshArgs);
     const nsecs_t after = systemTime(SYSTEM_TIME_MONOTONIC);
 
@@ -226,12 +227,9 @@ TEST_F(CompositionTestPreComposition, preCompositionInvokesLayerPreCompositionWi
     nsecs_t ts1 = 0;
     nsecs_t ts2 = 0;
     nsecs_t ts3 = 0;
-    EXPECT_CALL(*mLayer1FE, onPreComposition(_, _))
-            .WillOnce(DoAll(SaveArg<0>(&ts1), Return(false)));
-    EXPECT_CALL(*mLayer2FE, onPreComposition(_, _))
-            .WillOnce(DoAll(SaveArg<0>(&ts2), Return(false)));
-    EXPECT_CALL(*mLayer3FE, onPreComposition(_, _))
-            .WillOnce(DoAll(SaveArg<0>(&ts3), Return(false)));
+    EXPECT_CALL(*mLayer1FE, onPreComposition(_)).WillOnce(DoAll(SaveArg<0>(&ts1), Return(false)));
+    EXPECT_CALL(*mLayer2FE, onPreComposition(_)).WillOnce(DoAll(SaveArg<0>(&ts2), Return(false)));
+    EXPECT_CALL(*mLayer3FE, onPreComposition(_)).WillOnce(DoAll(SaveArg<0>(&ts3), Return(false)));
 
     mRefreshArgs.outputs = {mOutput1};
     mRefreshArgs.layers = {mLayer1FE, mLayer2FE, mLayer3FE};
@@ -245,9 +243,9 @@ TEST_F(CompositionTestPreComposition, preCompositionInvokesLayerPreCompositionWi
 }
 
 TEST_F(CompositionTestPreComposition, preCompositionDefaultsToNoUpdateNeeded) {
-    EXPECT_CALL(*mLayer1FE, onPreComposition(_, _)).WillOnce(Return(false));
-    EXPECT_CALL(*mLayer2FE, onPreComposition(_, _)).WillOnce(Return(false));
-    EXPECT_CALL(*mLayer3FE, onPreComposition(_, _)).WillOnce(Return(false));
+    EXPECT_CALL(*mLayer1FE, onPreComposition(_)).WillOnce(Return(false));
+    EXPECT_CALL(*mLayer2FE, onPreComposition(_)).WillOnce(Return(false));
+    EXPECT_CALL(*mLayer3FE, onPreComposition(_)).WillOnce(Return(false));
 
     mEngine.setNeedsAnotherUpdateForTest(true);
 
@@ -262,9 +260,9 @@ TEST_F(CompositionTestPreComposition, preCompositionDefaultsToNoUpdateNeeded) {
 
 TEST_F(CompositionTestPreComposition,
        preCompositionSetsNeedsAnotherUpdateIfAtLeastOneLayerRequestsIt) {
-    EXPECT_CALL(*mLayer1FE, onPreComposition(_, _)).WillOnce(Return(true));
-    EXPECT_CALL(*mLayer2FE, onPreComposition(_, _)).WillOnce(Return(false));
-    EXPECT_CALL(*mLayer3FE, onPreComposition(_, _)).WillOnce(Return(false));
+    EXPECT_CALL(*mLayer1FE, onPreComposition(_)).WillOnce(Return(true));
+    EXPECT_CALL(*mLayer2FE, onPreComposition(_)).WillOnce(Return(false));
+    EXPECT_CALL(*mLayer3FE, onPreComposition(_)).WillOnce(Return(false));
 
     mRefreshArgs.outputs = {mOutput1};
     mRefreshArgs.layers = {mLayer1FE, mLayer2FE, mLayer3FE};
diff --git a/services/surfaceflinger/LayerFE.cpp b/services/surfaceflinger/LayerFE.cpp
index 2dbcb841ac..43a4397899 100644
--- a/services/surfaceflinger/LayerFE.cpp
+++ b/services/surfaceflinger/LayerFE.cpp
@@ -82,8 +82,7 @@ const compositionengine::LayerFECompositionState* LayerFE::getCompositionState()
     return mSnapshot.get();
 }
 
-bool LayerFE::onPreComposition(nsecs_t refreshStartTime, bool) {
-    mCompositionResult.refreshStartTime = refreshStartTime;
+bool LayerFE::onPreComposition(bool) {
     return mSnapshot->hasReadyFrame;
 }
 
diff --git a/services/surfaceflinger/LayerFE.h b/services/surfaceflinger/LayerFE.h
index d584fb7eab..66cb88b20e 100644
--- a/services/surfaceflinger/LayerFE.h
+++ b/services/surfaceflinger/LayerFE.h
@@ -26,9 +26,6 @@
 namespace android {
 
 struct CompositionResult {
-    // TODO(b/238781169) update CE to no longer pass refreshStartTime to LayerFE::onPreComposition
-    // and remove this field.
-    nsecs_t refreshStartTime = 0;
     std::vector<std::pair<ftl::SharedFuture<FenceResult>, ui::LayerStack>> releaseFences;
     sp<Fence> lastClientCompositionFence = nullptr;
 };
@@ -39,7 +36,7 @@ public:
 
     // compositionengine::LayerFE overrides
     const compositionengine::LayerFECompositionState* getCompositionState() const override;
-    bool onPreComposition(nsecs_t refreshStartTime, bool updatingOutputGeometryThisFrame) override;
+    bool onPreComposition(bool updatingOutputGeometryThisFrame) override;
     void onLayerDisplayed(ftl::SharedFuture<FenceResult>, ui::LayerStack) override;
     const char* getDebugName() const override;
     int32_t getSequence() const override;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 78e14aee86..bf210afe6d 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2783,12 +2783,16 @@ CompositeResultsPerDisplay SurfaceFlinger::composite(
         }
     }
 
+    refreshArgs.refreshStartTime = systemTime(SYSTEM_TIME_MONOTONIC);
+    for (auto [layer, layerFE] : layers) {
+        layer->onPreComposition(refreshArgs.refreshStartTime);
+    }
+
     mCompositionEngine->present(refreshArgs);
     moveSnapshotsFromCompositionArgs(refreshArgs, layers);
 
     for (auto [layer, layerFE] : layers) {
         CompositionResult compositionResult{layerFE->stealCompositionResult()};
-        layer->onPreComposition(compositionResult.refreshStartTime);
         for (auto& [releaseFence, layerStack] : compositionResult.releaseFences) {
             Layer* clonedFrom = layer->getClonedFrom().get();
             auto owningLayer = clonedFrom ? clonedFrom : layer;