SF: replace dont_skip_on_early flag

.. with a new flag due to a Gantry bug: b/323967908

Bug: 273702768
Change-Id: I126bc6c4927c6ae8d1cd54316babe1590d3dde28
Test: presubmit

Author: adyabr

services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp

@@ -107,7 +107,7 @@ ScheduleResult VSyncDispatchTimerQueueEntry::schedule(VSyncDispatch::ScheduleTim
             mArmedInfo && (nextVsyncTime > (mArmedInfo->mActualVsyncTime + mMinVsyncDistance));
     bool const wouldSkipAWakeup =
             mArmedInfo && ((nextWakeupTime > (mArmedInfo->mActualWakeupTime + mMinVsyncDistance)));
-    if (FlagManager::getInstance().dont_skip_on_early()) {
+    if (FlagManager::getInstance().dont_skip_on_early_ro()) {
         if (wouldSkipAVsyncTarget || wouldSkipAWakeup) {
             nextVsyncTime = mArmedInfo->mActualVsyncTime;
         } else {

services/surfaceflinger/common/FlagManager.cpp

@@ -108,7 +108,6 @@ void FlagManager::dump(std::string& result) const {
     DUMP_SERVER_FLAG(use_skia_tracing);
 
     /// Trunk stable server flags ///
-    DUMP_SERVER_FLAG(dont_skip_on_early);
     DUMP_SERVER_FLAG(refresh_rate_overlay_on_external_display);
 
     /// Trunk stable readonly flags ///
@@ -131,6 +130,7 @@ void FlagManager::dump(std::string& result) const {
     DUMP_READ_ONLY_FLAG(vulkan_renderengine);
     DUMP_READ_ONLY_FLAG(renderable_buffer_usage);
     DUMP_READ_ONLY_FLAG(restore_blur_step);
+    DUMP_READ_ONLY_FLAG(dont_skip_on_early_ro);
 #undef DUMP_READ_ONLY_FLAG
 #undef DUMP_SERVER_FLAG
 #undef DUMP_FLAG_INTERVAL
@@ -209,15 +209,9 @@ FLAG_MANAGER_READ_ONLY_FLAG(screenshot_fence_preservation, "debug.sf.screenshot_
 FLAG_MANAGER_READ_ONLY_FLAG(vulkan_renderengine, "debug.renderengine.vulkan")
 FLAG_MANAGER_READ_ONLY_FLAG(renderable_buffer_usage, "")
 FLAG_MANAGER_READ_ONLY_FLAG(restore_blur_step, "debug.renderengine.restore_blur_step")
+FLAG_MANAGER_READ_ONLY_FLAG(dont_skip_on_early_ro, "")
 
 /// Trunk stable server flags ///
 FLAG_MANAGER_SERVER_FLAG(refresh_rate_overlay_on_external_display, "")
 
-/// Exceptions ///
-bool FlagManager::dont_skip_on_early() const {
-    // Even though this is a server writable flag, we do call it before boot completed, but that's
-    // fine since the decision is done per frame. We can't do caching though.
-    return flags::dont_skip_on_early();
-}
-
 } // namespace android

services/surfaceflinger/common/include/common/FlagManager.h

@@ -48,7 +48,6 @@ class FlagManager {
     bool use_skia_tracing() const;
 
     /// Trunk stable server flags ///
-    bool dont_skip_on_early() const;
     bool refresh_rate_overlay_on_external_display() const;
 
     /// Trunk stable readonly flags ///
@@ -71,6 +70,7 @@ class FlagManager {
     bool vulkan_renderengine() const;
     bool renderable_buffer_usage() const;
     bool restore_blur_step() const;
+    bool dont_skip_on_early_ro() const;
 
 protected:
     // overridden for unit tests

services/surfaceflinger/surfaceflinger_flags.aconfig

@@ -33,13 +33,6 @@ flag {
   is_fixed_read_only: true
 }
 
-flag {
-  name: "dont_skip_on_early"
-  namespace: "core_graphics"
-  description: "This flag is guarding the behaviour where SurfaceFlinger is trying to opportunistically present a frame when the configuration change from late to early"
-  bug: "273702768"
-}
-
 flag {
   name: "multithreaded_present"
   namespace: "core_graphics"
@@ -190,3 +183,14 @@ flag {
     purpose: PURPOSE_BUGFIX
   }
 }
+
+flag {
+  name: "dont_skip_on_early_ro"
+  namespace: "core_graphics"
+  description: "This flag is guarding the behaviour where SurfaceFlinger is trying to opportunistically present a frame when the configuration change from late to early"
+  bug: "273702768"
+  is_fixed_read_only: true
+  metadata {
+    purpose: PURPOSE_BUGFIX
+  }
+}

services/surfaceflinger/tests/unittests/FlagManagerTest.cpp

@@ -169,18 +169,4 @@ TEST_F(FlagManagerTest, readonlyReturnsValue) {
     }
 }
 
-TEST_F(FlagManagerTest, dontSkipOnEarlyIsNotCached) {
-    EXPECT_CALL(mFlagManager, getBoolProperty).WillRepeatedly(Return(std::nullopt));
-
-    const auto initialValue = flags::dont_skip_on_early();
-
-    flags::dont_skip_on_early(true);
-    EXPECT_EQ(true, mFlagManager.dont_skip_on_early());
-
-    flags::dont_skip_on_early(false);
-    EXPECT_EQ(false, mFlagManager.dont_skip_on_early());
-
-    flags::dont_skip_on_early(initialValue);
-}
-
 } // namespace android

services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp

@@ -734,7 +734,7 @@ TEST_F(VSyncDispatchTimerQueueTest, canMoveCallbackBackwardsInTime) {
 
 // b/1450138150
 TEST_F(VSyncDispatchTimerQueueTest, doesNotMoveCallbackBackwardsAndSkipAScheduledTargetVSync) {
-    SET_FLAG_FOR_TEST(flags::dont_skip_on_early, false);
+    SET_FLAG_FOR_TEST(flags::dont_skip_on_early_ro, false);
 
     EXPECT_CALL(mMockClock, alarmAt(_, 500));
     CountingCallback cb(mDispatch);
@@ -754,7 +754,7 @@ TEST_F(VSyncDispatchTimerQueueTest, doesNotMoveCallbackBackwardsAndSkipASchedule
 
 // b/1450138150
 TEST_F(VSyncDispatchTimerQueueTest, movesCallbackBackwardsAndSkipAScheduledTargetVSync) {
-    SET_FLAG_FOR_TEST(flags::dont_skip_on_early, true);
+    SET_FLAG_FOR_TEST(flags::dont_skip_on_early_ro, true);
 
     Sequence seq;
     EXPECT_CALL(mMockClock, alarmAt(_, 500)).InSequence(seq);
@@ -821,7 +821,7 @@ TEST_F(VSyncDispatchTimerQueueTest, canScheduleLargeNegativeOffset) {
 }
 
 TEST_F(VSyncDispatchTimerQueueTest, scheduleUpdatesDoesNotAffectSchedulingState) {
-    SET_FLAG_FOR_TEST(flags::dont_skip_on_early, false);
+    SET_FLAG_FOR_TEST(flags::dont_skip_on_early_ro, false);
 
     EXPECT_CALL(mMockClock, alarmAt(_, 600));
 
@@ -839,7 +839,7 @@ TEST_F(VSyncDispatchTimerQueueTest, scheduleUpdatesDoesNotAffectSchedulingState)
 }
 
 TEST_F(VSyncDispatchTimerQueueTest, scheduleUpdatesDoesAffectSchedulingState) {
-    SET_FLAG_FOR_TEST(flags::dont_skip_on_early, true);
+    SET_FLAG_FOR_TEST(flags::dont_skip_on_early_ro, true);
 
     Sequence seq;
     EXPECT_CALL(mMockClock, alarmAt(_, 600)).InSequence(seq);
@@ -1051,7 +1051,7 @@ TEST_F(VSyncDispatchTimerQueueTest, basicAlarmSettingFutureWithReadyDuration) {
 }
 
 TEST_F(VSyncDispatchTimerQueueTest, updatesVsyncTimeForCloseWakeupTime) {
-    SET_FLAG_FOR_TEST(flags::dont_skip_on_early, false);
+    SET_FLAG_FOR_TEST(flags::dont_skip_on_early_ro, false);
 
     Sequence seq;
     EXPECT_CALL(mMockClock, alarmAt(_, 600)).InSequence(seq);
@@ -1074,7 +1074,7 @@ TEST_F(VSyncDispatchTimerQueueTest, updatesVsyncTimeForCloseWakeupTime) {
 }
 
 TEST_F(VSyncDispatchTimerQueueTest, doesNotUpdatesVsyncTimeForCloseWakeupTime) {
-    SET_FLAG_FOR_TEST(flags::dont_skip_on_early, true);
+    SET_FLAG_FOR_TEST(flags::dont_skip_on_early_ro, true);
 
     Sequence seq;
     EXPECT_CALL(mMockClock, alarmAt(_, 600)).InSequence(seq);
@@ -1098,7 +1098,7 @@ TEST_F(VSyncDispatchTimerQueueTest, doesNotUpdatesVsyncTimeForCloseWakeupTime) {
 }
 
 TEST_F(VSyncDispatchTimerQueueTest, skipAVsyc) {
-    SET_FLAG_FOR_TEST(flags::dont_skip_on_early, false);
+    SET_FLAG_FOR_TEST(flags::dont_skip_on_early_ro, false);
 
     EXPECT_CALL(mMockClock, alarmAt(_, 500));
     CountingCallback cb(mDispatch);
@@ -1117,7 +1117,7 @@ TEST_F(VSyncDispatchTimerQueueTest, skipAVsyc) {
 }
 
 TEST_F(VSyncDispatchTimerQueueTest, dontskipAVsyc) {
-    SET_FLAG_FOR_TEST(flags::dont_skip_on_early, true);
+    SET_FLAG_FOR_TEST(flags::dont_skip_on_early_ro, true);
 
     Sequence seq;
     EXPECT_CALL(mMockClock, alarmAt(_, 500)).InSequence(seq);