diff --git a/shell/platform/windows/flutter_windows_view.cc b/shell/platform/windows/flutter_windows_view.cc index 6a4807fb5006f..28c7b5122d721 100644 --- a/shell/platform/windows/flutter_windows_view.cc +++ b/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 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. diff --git a/shell/platform/windows/flutter_windows_view.h b/shell/platform/windows/flutter_windows_view.h index 2dba87f25f993..5af6dbf2c48c8 100644 --- a/shell/platform/windows/flutter_windows_view.h +++ b/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; diff --git a/shell/platform/windows/flutter_windows_view_unittests.cc b/shell/platform/windows/flutter_windows_view_unittests.cc index 75d29cfeab3e3..945865f1f1cd6 100644 --- a/shell/platform/windows/flutter_windows_view_unittests.cc +++ b/shell/platform/windows/flutter_windows_view_unittests.cc @@ -845,7 +845,7 @@ 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(); @@ -853,7 +853,8 @@ TEST(FlutterWindowsViewTest, WindowResizeTests) { 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 engine = GetTestEngine(); + EngineModifier modifier(engine.get()); + + auto window_binding_handler = + std::make_unique>(); + auto windows_proc_table = std::make_shared(); + std::unique_ptr surface_manager = + std::make_unique(); + + 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 engine = GetTestEngine(); EngineModifier modifier(engine.get()); diff --git a/shell/platform/windows/testing/mock_window_binding_handler_delegate.h b/shell/platform/windows/testing/mock_window_binding_handler_delegate.h index 81c58abceba8a..9cc0ff4d477f1 100644 --- a/shell/platform/windows/testing/mock_window_binding_handler_delegate.h +++ b/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, diff --git a/shell/platform/windows/window_binding_handler_delegate.h b/shell/platform/windows/window_binding_handler_delegate.h index 265ca8093b395..64694c9c1ac1c 100644 --- a/shell/platform/windows/window_binding_handler_delegate.h +++ b/shell/platform/windows/window_binding_handler_delegate.h @@ -20,9 +20,12 @@ class WindowBindingHandlerDelegate { using KeyEventCallback = std::function; // 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.