Fix race condition and data race in FrameInfoContainsValidWidthAndHeight. (#20706)

The data race: gl_surface_fbo_frame_infos_ in the test context to was appended
to on the raster task runner but read on the platform task runner. This is now
sidestepped by using a callback and pushing that responsibility to the test.
Setting the callback is guarded behind a mutex.

The race condition: The assertions were previously run when the UI thread was
done generating the frames. However, the assertions were run on the results
collected on the raster thread in response the frame requests from UI thread.
Just run the assertions on the raster thread directly.

Fixes https://github.com/flutter/flutter/issues/64344

shell/platform/embedder/tests/embedder_test_context.cc

@@ -30,7 +30,9 @@ EmbedderTestContext::EmbedderTestContext(std::string assets_path)
       });
 }
 
-EmbedderTestContext::~EmbedderTestContext() = default;
+EmbedderTestContext::~EmbedderTestContext() {
+  SetGLGetFBOCallback(nullptr);
+}
 
 void EmbedderTestContext::SetupAOTMappingsIfNecessary() {
   if (!DartVM::IsRunningPrecompiledCode()) {
@@ -197,14 +199,25 @@ bool EmbedderTestContext::GLPresent() {
   return true;
 }
 
+void EmbedderTestContext::SetGLGetFBOCallback(GLGetFBOCallback callback) {
+  std::scoped_lock lock(gl_get_fbo_callback_mutex_);
+  gl_get_fbo_callback_ = callback;
+}
+
 uint32_t EmbedderTestContext::GLGetFramebuffer(FlutterFrameInfo frame_info) {
   FML_CHECK(gl_surface_) << "GL surface must be initialized.";
-  gl_surface_fbo_frame_infos_.push_back(frame_info);
-  return gl_surface_->GetFramebuffer();
-}
 
-std::vector<FlutterFrameInfo> EmbedderTestContext::GetGLFBOFrameInfos() {
-  return gl_surface_fbo_frame_infos_;
+  GLGetFBOCallback callback;
+  {
+    std::scoped_lock lock(gl_get_fbo_callback_mutex_);
+    callback = gl_get_fbo_callback_;
+  }
+
+  if (callback) {
+    callback(frame_info);
+  }
+
+  return gl_surface_->GetFramebuffer();
 }
 
 bool EmbedderTestContext::GLMakeResourceCurrent() {

shell/platform/embedder/tests/embedder_test_context.h

@@ -8,6 +8,7 @@
 #include <future>
 #include <map>
 #include <memory>
+#include <mutex>
 #include <string>
 #include <vector>
 
@@ -79,8 +80,20 @@ class EmbedderTestContext {
 
   size_t GetSoftwareSurfacePresentCount() const;
 
-  // Returns the frame information for all the frames that were rendered.
-  std::vector<FlutterFrameInfo> GetGLFBOFrameInfos();
+  using GLGetFBOCallback = std::function<void(FlutterFrameInfo frame_info)>;
+
+  //----------------------------------------------------------------------------
+  /// @brief      Sets a callback that will be invoked (on the raster task
+  ///             runner) when the engine asks the embedder for a new FBO ID at
+  ///             the updated size.
+  ///
+  /// @attention  The callback will be invoked on the raster task runner. The
+  ///             callback can be set on the tests host thread.
+  ///
+  /// @param[in]  callback  The callback to set. The previous callback will be
+  ///                       un-registered.
+  ///
+  void SetGLGetFBOCallback(GLGetFBOCallback callback);
 
  private:
   // This allows the builder to access the hooks.
@@ -104,9 +117,10 @@ class EmbedderTestContext {
   std::unique_ptr<EmbedderTestCompositor> compositor_;
   NextSceneCallback next_scene_callback_;
   SkMatrix root_surface_transformation_;
-  std::vector<FlutterFrameInfo> gl_surface_fbo_frame_infos_;
   size_t gl_surface_present_count_ = 0;
   size_t software_surface_present_count_ = 0;
+  std::mutex gl_get_fbo_callback_mutex_;
+  GLGetFBOCallback gl_get_fbo_callback_;
 
   static VoidCallback GetIsolateCreateCallbackHook();
 

shell/platform/embedder/tests/embedder_unittests.cc

@@ -4376,24 +4376,23 @@ TEST_F(EmbedderTest, FrameInfoContainsValidWidthAndHeight) {
             kSuccess);
   ASSERT_TRUE(engine.is_valid());
 
-  constexpr size_t frames_expected = 10;
-  fml::CountDownLatch frame_latch(frames_expected);
-  size_t frames_seen = 0;
+  fml::CountDownLatch frame_latch(10);
+
   context.AddNativeCallback("SignalNativeTest",
                             CREATE_NATIVE_ENTRY([&](Dart_NativeArguments args) {
-                              frames_seen++;
-                              frame_latch.CountDown();
+                              /* Nothing to do. */
                             }));
-  frame_latch.Wait();
 
-  ASSERT_EQ(frames_expected, frames_seen);
-  ASSERT_EQ(context.GetGLFBOFrameInfos().size(), frames_seen);
+  context.SetGLGetFBOCallback(
+      [&event, &frame_latch](FlutterFrameInfo frame_info) {
+        // width and height are rotated by 90 deg
+        ASSERT_EQ(frame_info.size.width, event.height);
+        ASSERT_EQ(frame_info.size.height, event.width);
 
-  for (FlutterFrameInfo frame_info : context.GetGLFBOFrameInfos()) {
-    // width and height are rotated by 90 deg
-    ASSERT_EQ(frame_info.size.width, event.height);
-    ASSERT_EQ(frame_info.size.height, event.width);
-  }
+        frame_latch.CountDown();
+      });
+
+  frame_latch.Wait();
 }
 
 TEST_F(EmbedderTest, MustNotRunWithBothFBOCallbacksSet) {