VsyncWaiter schedules unneeded extra AwaitVSync callbacks for one frame (flutter#36406)

Author: fzyzcjy

ci/licenses_golden/licenses_flutter

@@ -2073,6 +2073,7 @@ FILE: ../../../flutter/shell/common/vsync_waiter.cc
 FILE: ../../../flutter/shell/common/vsync_waiter.h
 FILE: ../../../flutter/shell/common/vsync_waiter_fallback.cc
 FILE: ../../../flutter/shell/common/vsync_waiter_fallback.h
+FILE: ../../../flutter/shell/common/vsync_waiter_unittests.cc
 FILE: ../../../flutter/shell/common/vsync_waiters_test.cc
 FILE: ../../../flutter/shell/common/vsync_waiters_test.h
 FILE: ../../../flutter/shell/gpu/gpu_surface_gl_delegate.cc

shell/common/BUILD.gn

@@ -293,6 +293,7 @@ if (enable_unittests) {
       "shell_unittests.cc",
       "switches_unittests.cc",
       "variable_refresh_rate_display_unittests.cc",
+      "vsync_waiter_unittests.cc",
     ]
 
     deps = [

shell/common/vsync_waiter.cc

@@ -62,6 +62,7 @@ void VsyncWaiter::ScheduleSecondaryCallback(uintptr_t id,
 
   {
     std::scoped_lock lock(callback_mutex_);
+    bool secondary_callbacks_originally_empty = secondary_callbacks_.empty();
     auto [_, inserted] = secondary_callbacks_.emplace(id, callback);
     if (!inserted) {
       // Multiple schedules must result in a single callback per frame interval.
@@ -74,6 +75,11 @@ void VsyncWaiter::ScheduleSecondaryCallback(uintptr_t id,
       // `AsyncWaitForVsync`.
       return;
     }
+    if (!secondary_callbacks_originally_empty) {
+      // Return directly as `AwaitVSync` is already called by
+      // `ScheduleSecondaryCallback`.
+      return;
+    }
   }
   AwaitVSyncForSecondaryCallback();
 }

shell/common/vsync_waiter_unittests.cc

@@ -0,0 +1,47 @@
+#define FML_USED_ON_EMBEDDER
+
+#include <initializer_list>
+
+#include "flutter/common/settings.h"
+#include "flutter/common/task_runners.h"
+#include "flutter/shell/common/switches.h"
+
+#include "gtest/gtest.h"
+#include "thread_host.h"
+#include "vsync_waiter.h"
+
+namespace flutter {
+namespace testing {
+
+class TestVsyncWaiter : public VsyncWaiter {
+ public:
+  explicit TestVsyncWaiter(const TaskRunners& task_runners)
+      : VsyncWaiter(task_runners) {}
+
+  int await_vsync_call_count_ = 0;
+
+ protected:
+  void AwaitVSync() override { await_vsync_call_count_++; }
+};
+
+TEST(VsyncWaiterTest, NoUnneededAwaitVsync) {
+  using flutter::ThreadHost;
+  std::string prefix = "vsync_waiter_test";
+
+  fml::MessageLoop::EnsureInitializedForCurrentThread();
+  auto task_runner = fml::MessageLoop::GetCurrent().GetTaskRunner();
+
+  const flutter::TaskRunners task_runners(prefix, task_runner, task_runner,
+                                          task_runner, task_runner);
+
+  TestVsyncWaiter vsync_waiter(task_runners);
+
+  vsync_waiter.ScheduleSecondaryCallback(1, [] {});
+  EXPECT_EQ(vsync_waiter.await_vsync_call_count_, 1);
+
+  vsync_waiter.ScheduleSecondaryCallback(2, [] {});
+  EXPECT_EQ(vsync_waiter.await_vsync_call_count_, 1);
+}
+
+}  // namespace testing
+}  // namespace flutter