[fuchsia] Allow multiple vsync requests before the first Present (#31316

)

shell/platform/fuchsia/flutter/flatland_connection.cc

@@ -37,6 +37,10 @@ FlatlandConnection::~FlatlandConnection() = default;
 
 // This method is called from the raster thread.
 void FlatlandConnection::Present() {
+  if (!threadsafe_state_.first_present_called_) {
+    std::scoped_lock<std::mutex> lock(threadsafe_state_.mutex_);
+    threadsafe_state_.first_present_called_ = true;
+  }
   if (present_credits_ > 0) {
     DoPresent();
   } else {
@@ -67,25 +71,26 @@ void FlatlandConnection::DoPresent() {
 
 // This method is called from the UI thread.
 void FlatlandConnection::AwaitVsync(FireCallbackCallback callback) {
-  if (first_call_to_await_vsync_) {
+  std::scoped_lock<std::mutex> lock(threadsafe_state_.mutex_);
+
+  // Immediately fire callbacks until the first Present. We might receive
+  // multiple requests for AwaitVsync() until the first Present, which relies on
+  // receiving size on FlatlandPlatformView::OnGetLayout() at an uncertain time.
+  if (!threadsafe_state_.first_present_called_) {
     fml::TimePoint now = fml::TimePoint::Now();
     callback(now, now + kDefaultFlatlandPresentationInterval);
-    first_call_to_await_vsync_ = false;
     return;
   }
 
-  {
-    std::scoped_lock<std::mutex> lock(threadsafe_state_.mutex_);
-    threadsafe_state_.fire_callback_ = callback;
+  threadsafe_state_.fire_callback_ = callback;
 
-    if (threadsafe_state_.fire_callback_pending_) {
-      fml::TimePoint now = fml::TimePoint::Now();
-      // TODO(fxbug.dev/64201): Calculate correct frame times.
-      threadsafe_state_.fire_callback_(
-          now, now + kDefaultFlatlandPresentationInterval);
-      threadsafe_state_.fire_callback_ = nullptr;
-      threadsafe_state_.fire_callback_pending_ = false;
-    }
+  if (threadsafe_state_.fire_callback_pending_) {
+    fml::TimePoint now = fml::TimePoint::Now();
+    // TODO(fxbug.dev/64201): Calculate correct frame times.
+    threadsafe_state_.fire_callback_(
+        now, now + kDefaultFlatlandPresentationInterval);
+    threadsafe_state_.fire_callback_ = nullptr;
+    threadsafe_state_.fire_callback_pending_ = false;
   }
 }
 

shell/platform/fuchsia/flutter/flatland_connection.h

@@ -80,16 +80,15 @@ class FlatlandConnection final {
   bool present_pending_ = false;
 
   // This struct contains state that is accessed from both from the UI thread
-  // (in AwaitVsync) and the raster thread (in OnNextFrameBegin). You should
-  // always lock mutex_ before touching anything in this struct
+  // (in AwaitVsync) and the raster thread (in OnNextFrameBegin and Present).
+  // You should always lock mutex_ before touching anything in this struct
   struct {
     std::mutex mutex_;
     FireCallbackCallback fire_callback_;
     bool fire_callback_pending_ = false;
+    bool first_present_called_ = false;
   } threadsafe_state_;
 
-  bool first_call_to_await_vsync_ = true;
-
   std::vector<zx::event> acquire_fences_;
   std::vector<zx::event> current_present_release_fences_;
   std::vector<zx::event> previous_present_release_fences_;

shell/platform/fuchsia/flutter/tests/flatland_connection_unittests.cc

@@ -7,6 +7,7 @@
 #include <fuchsia/scenic/scheduling/cpp/fidl.h>
 #include <fuchsia/ui/composition/cpp/fidl.h>
 #include <lib/async-testing/test_loop.h>
+#include <lib/async/cpp/task.h>
 
 #include <string>
 #include <vector>
@@ -194,17 +195,52 @@ TEST_F(FlatlandConnectionTest, BasicPresent) {
   EXPECT_EQ(release_fence_handle, first_release_fence_handle);
 }
 
+TEST_F(FlatlandConnectionTest, AwaitVsyncBeforePresent) {
+  // Set up callbacks which allow sensing of how many presents were handled.
+  size_t presents_called = 0u;
+  fake_flatland().SetPresentHandler(
+      [&presents_called](auto present_args) { presents_called++; });
+
+  // Create the FlatlandConnection but don't pump the loop.  No FIDL calls are
+  // completed yet.
+  flutter_runner::FlatlandConnection flatland_connection(
+      GetCurrentTestName(), TakeFlatlandHandle(), []() { FAIL(); },
+      [](auto...) {}, 1, fml::TimeDelta::Zero());
+  EXPECT_EQ(presents_called, 0u);
+
+  // Pump the loop. Nothing is called.
+  loop().RunUntilIdle();
+  EXPECT_EQ(presents_called, 0u);
+
+  // Simulate an AwaitVsync that comes before the first Present.
+  bool await_vsync_callback_fired = false;
+  AwaitVsyncChecked(flatland_connection, await_vsync_callback_fired,
+                    kDefaultFlatlandPresentationInterval);
+  EXPECT_TRUE(await_vsync_callback_fired);
+
+  // Another AwaitVsync that comes before the first Present.
+  await_vsync_callback_fired = false;
+  AwaitVsyncChecked(flatland_connection, await_vsync_callback_fired,
+                    kDefaultFlatlandPresentationInterval);
+  EXPECT_TRUE(await_vsync_callback_fired);
+
+  // Queue Present.
+  flatland_connection.Present();
+  loop().RunUntilIdle();
+  EXPECT_EQ(presents_called, 1u);
+
+  // Set the callback with AwaitVsync, callback should not be fired
+  await_vsync_callback_fired = false;
+  AwaitVsyncChecked(flatland_connection, await_vsync_callback_fired,
+                    kDefaultFlatlandPresentationInterval);
+  EXPECT_FALSE(await_vsync_callback_fired);
+}
+
 TEST_F(FlatlandConnectionTest, OutOfOrderAwait) {
   // Set up callbacks which allow sensing of how many presents were handled.
   size_t presents_called = 0u;
-  zx_handle_t release_fence_handle;
-  fake_flatland().SetPresentHandler([&presents_called,
-                                     &release_fence_handle](auto present_args) {
-    presents_called++;
-    release_fence_handle = present_args.release_fences().empty()
-                               ? ZX_HANDLE_INVALID
-                               : present_args.release_fences().front().get();
-  });
+  fake_flatland().SetPresentHandler(
+      [&presents_called](auto present_args) { presents_called++; });
 
   // Set up a callback which allows sensing of how many vsync's
   // (`OnFramePresented` events) were handled.
@@ -226,12 +262,17 @@ TEST_F(FlatlandConnectionTest, OutOfOrderAwait) {
   EXPECT_EQ(presents_called, 0u);
   EXPECT_EQ(vsyncs_handled, 0u);
 
-  // Simulate an AwaitVsync that comes after the first call.
+  // Simulate an AwaitVsync that comes before the first Present.
   bool await_vsync_callback_fired = false;
   AwaitVsyncChecked(flatland_connection, await_vsync_callback_fired,
                     kDefaultFlatlandPresentationInterval);
   EXPECT_TRUE(await_vsync_callback_fired);
 
+  // Queue Present.
+  flatland_connection.Present();
+  loop().RunUntilIdle();
+  EXPECT_EQ(presents_called, 1u);
+
   // Set the callback with AwaitVsync, callback should not be fired
   await_vsync_callback_fired = false;
   AwaitVsyncChecked(flatland_connection, await_vsync_callback_fired,
@@ -304,36 +345,33 @@ TEST_F(FlatlandConnectionTest, PresentCreditExhaustion) {
   loop().RunUntilIdle();
   EXPECT_EQ(num_presents_called, 0u);
 
-  // Simulate an AwaitVsync that comes after the first call.
+  // Simulate an AwaitVsync that comes before the first Present.
   flatland_connection.AwaitVsync([](fml::TimePoint, fml::TimePoint) {});
   loop().RunUntilIdle();
   EXPECT_EQ(num_presents_called, 0u);
 
   // This test uses a fire callback that triggers Present() with a single
   // acquire and release fence in order to approximate the behavior of the real
   // flutter fire callback and let us drive presents with ONFBs
-  auto fire_callback = [&flatland_connection](fml::TimePoint frame_start,
-                                              fml::TimePoint frame_end) {
-    zx::event acquire_fence;
-    zx::event::create(0, &acquire_fence);
-    zx::event release_fence;
-    zx::event::create(0, &release_fence);
-    flatland_connection.EnqueueAcquireFence(std::move(acquire_fence));
-    flatland_connection.EnqueueReleaseFence(std::move(release_fence));
-    flatland_connection.Present();
+  auto fire_callback = [dispatcher = loop().dispatcher(), &flatland_connection](
+                           fml::TimePoint frame_start,
+                           fml::TimePoint frame_end) {
+    async::PostTask(dispatcher, [&flatland_connection]() {
+      zx::event acquire_fence;
+      zx::event::create(0, &acquire_fence);
+      zx::event release_fence;
+      zx::event::create(0, &release_fence);
+      flatland_connection.EnqueueAcquireFence(std::move(acquire_fence));
+      flatland_connection.EnqueueReleaseFence(std::move(release_fence));
+      flatland_connection.Present();
+    });
   };
 
-  // Call Await Vsync with a callback that triggers Present, but this should not
-  // present until ONFB is delivered below.
+  // Call Await Vsync with a callback that triggers Present and consumes the one
+  // and only present credit we start with.
   reset_test_counters();
   flatland_connection.AwaitVsync(fire_callback);
   loop().RunUntilIdle();
-  EXPECT_EQ(num_presents_called, 0u);
-
-  // Call ONFB ands supply 0 present credits. This causes `Present` to be
-  // called and consumes the one and only present credit we start with.
-  OnNextFrameBegin(0);
-  loop().RunUntilIdle();
   EXPECT_EQ(num_presents_called, 1u);
   EXPECT_EQ(num_acquire_fences, 1u);
   EXPECT_EQ(num_release_fences, 0u);