[Windows] Fix resize crash (#49935)

#49872 introduced a crash in debug mode if the platform thread starts a window resize in between `OnFrameGenerated` and `OnFramePresented`.

Fixes flutter/flutter#141855.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

Author: loic-sharma

shell/platform/windows/flutter_windows_view.cc

@@ -157,38 +157,41 @@ void FlutterWindowsView::ForceRedraw() {
   }
 }
 
-void FlutterWindowsView::OnWindowSizeChanged(size_t width, size_t height) {
+bool FlutterWindowsView::OnWindowSizeChanged(size_t width, size_t height) {
   // Called on the platform thread.
   std::unique_lock<std::mutex> lock(resize_mutex_);
 
   if (!engine_->surface_manager()) {
     SendWindowMetrics(width, height, binding_handler_->GetDpiScale());
-    return;
+    return true;
   }
 
+  // We're using OpenGL rendering. Resizing the surface must happen on the
+  // raster thread.
   EGLint surface_width, surface_height;
   engine_->surface_manager()->GetSurfaceDimensions(&surface_width,
                                                    &surface_height);
 
   bool surface_will_update =
       SurfaceWillUpdate(surface_width, surface_height, width, height);
-  if (surface_will_update) {
-    resize_status_ = ResizeState::kResizeStarted;
-    resize_target_width_ = width;
-    resize_target_height_ = height;
+  if (!surface_will_update) {
+    SendWindowMetrics(width, height, binding_handler_->GetDpiScale());
+    return true;
   }
 
+  resize_status_ = ResizeState::kResizeStarted;
+  resize_target_width_ = width;
+  resize_target_height_ = height;
+
   SendWindowMetrics(width, height, binding_handler_->GetDpiScale());
 
-  if (surface_will_update) {
-    // Block the platform thread until:
-    //   1. GetFrameBufferId is called with the right frame size.
-    //   2. Any pending SwapBuffers calls have been invoked.
-    resize_cv_.wait_for(lock, kWindowResizeTimeout,
-                        [&resize_status = resize_status_] {
-                          return resize_status == ResizeState::kDone;
-                        });
-  }
+  // Block the platform thread until a frame is presented with the target
+  // size. See |OnFrameGenerated|, |OnEmptyFrameGenerated|, and
+  // |OnFramePresented|.
+  return resize_cv_.wait_for(lock, kWindowResizeTimeout,
+                             [&resize_status = resize_status_] {
+                               return resize_status == ResizeState::kDone;
+                             });
 }
 
 void FlutterWindowsView::OnWindowRepaint() {
@@ -597,10 +600,15 @@ void FlutterWindowsView::OnFramePresented() {
   switch (resize_status_) {
     case ResizeState::kResizeStarted:
       // The caller must first call |OnFrameGenerated| or
-      // |OnEmptyFrameGenerated| before calling this method. This status
-      // indicates the caller did not call these methods or ignored their
-      // result.
-      FML_UNREACHABLE();
+      // |OnEmptyFrameGenerated| before calling this method. This
+      // indicates one of the following:
+      //
+      // 1. The caller did not call these methods.
+      // 2. The caller ignored these methods' result.
+      // 3. The platform thread started a resize after the caller called these
+      //    methods. We might have presented a frame of the wrong size to the
+      //    view.
+      return;
     case ResizeState::kFrameGenerated: {
       // A frame was generated for a pending resize.
       // Unblock the platform thread.

shell/platform/windows/flutter_windows_view.h

@@ -105,7 +105,7 @@ class FlutterWindowsView : public WindowBindingHandlerDelegate {
   void SetFlutterCursor(HCURSOR cursor);
 
   // |WindowBindingHandlerDelegate|
-  void OnWindowSizeChanged(size_t width, size_t height) override;
+  bool OnWindowSizeChanged(size_t width, size_t height) override;
 
   // |WindowBindingHandlerDelegate|
   void OnWindowRepaint() override;

shell/platform/windows/flutter_windows_view_unittests.cc

@@ -845,15 +845,16 @@ TEST(FlutterWindowsViewTest, WindowResizeTests) {
   std::thread([&resized_latch, &view]() {
     // Start the window resize. This sends the new window metrics
     // and then blocks until another thread completes the window resize.
-    view.OnWindowSizeChanged(500, 500);
+    EXPECT_TRUE(view.OnWindowSizeChanged(500, 500));
     resized_latch.Signal();
   }).detach();
 
   // Wait until the platform thread has started the window resize.
   metrics_sent_latch.Wait();
 
   // Complete the window resize by reporting a frame with the new window size.
-  view.OnFrameGenerated(500, 500);
+  ASSERT_TRUE(view.OnFrameGenerated(500, 500));
+  view.OnFramePresented();
   resized_latch.Wait();
 }
 
@@ -894,7 +895,7 @@ TEST(FlutterWindowsViewTest, TestEmptyFrameResizes) {
   std::thread([&resized_latch, &view]() {
     // Start the window resize. This sends the new window metrics
     // and then blocks until another thread completes the window resize.
-    view.OnWindowSizeChanged(500, 500);
+    EXPECT_TRUE(view.OnWindowSizeChanged(500, 500));
     resized_latch.Signal();
   }).detach();
 
@@ -903,9 +904,51 @@ TEST(FlutterWindowsViewTest, TestEmptyFrameResizes) {
 
   // Complete the window resize by reporting an empty frame.
   view.OnEmptyFrameGenerated();
+  view.OnFramePresented();
   resized_latch.Wait();
 }
 
+// A window resize can be interleaved between a frame generation and
+// presentation. This should not crash the app. Regression test for:
+// https://github.com/flutter/flutter/issues/141855
+TEST(FlutterWindowsViewTest, WindowResizeRace) {
+  std::unique_ptr<FlutterWindowsEngine> engine = GetTestEngine();
+  EngineModifier modifier(engine.get());
+
+  auto window_binding_handler =
+      std::make_unique<NiceMock<MockWindowBindingHandler>>();
+  auto windows_proc_table = std::make_shared<MockWindowsProcTable>();
+  std::unique_ptr<MockAngleSurfaceManager> surface_manager =
+      std::make_unique<MockAngleSurfaceManager>();
+
+  EXPECT_CALL(*surface_manager.get(), DestroySurface).Times(1);
+
+  FlutterWindowsView view(std::move(window_binding_handler),
+                          std::move(windows_proc_table));
+  modifier.SetSurfaceManager(std::move(surface_manager));
+  view.SetEngine(engine.get());
+
+  // Begin a frame.
+  ASSERT_TRUE(view.OnFrameGenerated(100, 100));
+
+  // Inject a window resize between the frame generation and
+  // frame presentation. The new size invalidates the current frame.
+  fml::AutoResetWaitableEvent resized_latch;
+  std::thread([&resized_latch, &view]() {
+    // The resize is never completed. The view times out and returns false.
+    EXPECT_FALSE(view.OnWindowSizeChanged(500, 500));
+    resized_latch.Signal();
+  }).detach();
+
+  // Wait until the platform thread has started the window resize.
+  resized_latch.Wait();
+
+  // Complete the invalidated frame while a resize is pending. Although this
+  // might mean that we presented a frame with the wrong size, this should not
+  // crash the app.
+  view.OnFramePresented();
+}
+
 TEST(FlutterWindowsViewTest, WindowRepaintTests) {
   std::unique_ptr<FlutterWindowsEngine> engine = GetTestEngine();
   EngineModifier modifier(engine.get());

shell/platform/windows/testing/mock_window_binding_handler_delegate.h

@@ -16,7 +16,7 @@ class MockWindowBindingHandlerDelegate : public WindowBindingHandlerDelegate {
  public:
   MockWindowBindingHandlerDelegate() {}
 
-  MOCK_METHOD(void, OnWindowSizeChanged, (size_t, size_t), (override));
+  MOCK_METHOD(bool, OnWindowSizeChanged, (size_t, size_t), (override));
   MOCK_METHOD(void, OnWindowRepaint, (), (override));
   MOCK_METHOD(void,
               OnPointerMove,

shell/platform/windows/window_binding_handler_delegate.h

@@ -20,9 +20,12 @@ class WindowBindingHandlerDelegate {
   using KeyEventCallback = std::function<void(bool)>;
 
   // Notifies delegate that backing window size has changed.
-  // Typically called by currently configured WindowBindingHandler, this is
-  // called on the platform thread.
-  virtual void OnWindowSizeChanged(size_t width, size_t height) = 0;
+  //
+  // Called by |FlutterWindow| on the platform thread.
+  //
+  // Returns true if the delegate completed the window resize synchronously.
+  // The return value is exposed for unit testing.
+  virtual bool OnWindowSizeChanged(size_t width, size_t height) = 0;
 
   // Notifies delegate that backing window needs to be repainted.
   // Typically called by currently configured WindowBindingHandler.