[Windows] Don't always stop engine on view destruction

Author: loic-sharma

ci/licenses_golden/licenses_flutter

@@ -41819,6 +41819,7 @@ ORIGIN: ../../../flutter/shell/platform/windows/flutter_windows_texture_registra
 ORIGIN: ../../../flutter/shell/platform/windows/flutter_windows_texture_registrar.h + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/shell/platform/windows/flutter_windows_view.cc + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/shell/platform/windows/flutter_windows_view.h + ../../../flutter/LICENSE
+ORIGIN: ../../../flutter/shell/platform/windows/flutter_windows_view_controller.cc + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/shell/platform/windows/flutter_windows_view_controller.h + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/shell/platform/windows/keyboard_handler_base.h + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/shell/platform/windows/keyboard_key_channel_handler.cc + ../../../flutter/LICENSE
@@ -44723,6 +44724,7 @@ FILE: ../../../flutter/shell/platform/windows/flutter_windows_texture_registrar.
 FILE: ../../../flutter/shell/platform/windows/flutter_windows_texture_registrar.h
 FILE: ../../../flutter/shell/platform/windows/flutter_windows_view.cc
 FILE: ../../../flutter/shell/platform/windows/flutter_windows_view.h
+FILE: ../../../flutter/shell/platform/windows/flutter_windows_view_controller.cc
 FILE: ../../../flutter/shell/platform/windows/flutter_windows_view_controller.h
 FILE: ../../../flutter/shell/platform/windows/keyboard_handler_base.h
 FILE: ../../../flutter/shell/platform/windows/keyboard_key_channel_handler.cc

shell/platform/windows/BUILD.gn

@@ -87,6 +87,7 @@ source_set("flutter_windows_source") {
     "flutter_windows_texture_registrar.h",
     "flutter_windows_view.cc",
     "flutter_windows_view.h",
+    "flutter_windows_view_controller.cc",
     "flutter_windows_view_controller.h",
     "keyboard_handler_base.h",
     "keyboard_key_channel_handler.cc",

shell/platform/windows/flutter_windows.cc

@@ -130,6 +130,8 @@ FlutterDesktopViewControllerRef FlutterDesktopEngineCreateViewController(
 
 void FlutterDesktopViewControllerDestroy(FlutterDesktopViewControllerRef ref) {
   auto controller = ViewControllerFromHandle(ref);
+  controller->Destroy();
+
   delete controller;
 }
 

shell/platform/windows/flutter_windows_engine.cc

@@ -502,6 +502,24 @@ std::unique_ptr<FlutterWindowsView> FlutterWindowsEngine::CreateView(
   return std::move(view);
 }
 
+void FlutterWindowsEngine::RemoveView(FlutterViewId view_id) {
+  FML_DCHECK(running());
+  FML_DCHECK(views_.find(view_id) != views_.end());
+
+  if (view_id == kImplicitViewId) {
+    // The engine and framework assume the implicit view always exists.
+    // Attempts to render to the implicit view will be ignored.
+    views_.erase(view_id);
+    return;
+  }
+
+  // TODO(loicsharma): Remove the view from the engine using the
+  // `FlutterEngineRemoveView` embedder API. Windows does not
+  // support views other than the implicit view yet.
+  // https://github.com/flutter/flutter/issues/144810
+  FML_UNREACHABLE();
+}
+
 void FlutterWindowsEngine::OnVsync(intptr_t baton) {
   std::chrono::nanoseconds current_time =
       std::chrono::nanoseconds(embedder_api_.GetCurrentTime());

shell/platform/windows/flutter_windows_engine.h

@@ -124,6 +124,9 @@ class FlutterWindowsEngine {
   std::unique_ptr<FlutterWindowsView> CreateView(
       std::unique_ptr<WindowBindingHandler> window);
 
+  // Remove a view. The engine will no longer render into it.
+  virtual void RemoveView(FlutterViewId view_id);
+
   // Get a view that displays this engine's content.
   //
   // Returns null if the view does not exist.

shell/platform/windows/flutter_windows_unittests.cc

@@ -139,6 +139,9 @@ TEST_F(WindowsTest, EngineCanTransitionToHeadless) {
 
   // The engine is back in headless mode now.
   ASSERT_NE(engine, nullptr);
+
+  auto engine_ptr = reinterpret_cast<FlutterWindowsEngine*>(engine.get());
+  ASSERT_TRUE(engine_ptr->running());
 }
 
 // Verify that accessibility features are initialized when a view is created.

shell/platform/windows/flutter_windows_view.cc

@@ -105,10 +105,6 @@ FlutterWindowsView::~FlutterWindowsView() {
   // Notify the engine the view's child window will no longer be visible.
   engine_->OnWindowStateEvent(GetWindowHandle(), WindowStateEvent::kHide);
 
-  // The engine renders into the view's surface. The engine must be
-  // shutdown before the view's resources can be destroyed.
-  engine_->Stop();
-
   DestroyRenderSurface();
 }
 

shell/platform/windows/flutter_windows_view_controller.cc

@@ -0,0 +1,30 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "flutter/shell/platform/windows/flutter_windows_view_controller.h"
+
+namespace flutter {
+
+FlutterWindowsViewController::~FlutterWindowsViewController() {
+  Destroy();
+}
+
+void FlutterWindowsViewController::Destroy() {
+  if (!view_) {
+    return;
+  }
+
+  // Prevent the engine from rendering into this view.
+  if (view_->GetEngine()->running()) {
+    auto view_id = view_->view_id();
+
+    view_->GetEngine()->RemoveView(view_id);
+  }
+
+  // Destroy the view, followed by the engine if it is owned by this controller.
+  view_.reset();
+  engine_.reset();
+}
+
+}  // namespace flutter

shell/platform/windows/flutter_windows_view_controller.h

@@ -7,8 +7,9 @@
 
 #include <memory>
 
-#include "flutter_windows_engine.h"
-#include "flutter_windows_view.h"
+#include "flutter/fml/macros.h"
+#include "flutter/shell/platform/windows/flutter_windows_engine.h"
+#include "flutter/shell/platform/windows/flutter_windows_view.h"
 
 namespace flutter {
 
@@ -19,6 +20,13 @@ class FlutterWindowsViewController {
                                std::unique_ptr<FlutterWindowsView> view)
       : engine_(std::move(engine)), view_(std::move(view)) {}
 
+  ~FlutterWindowsViewController();
+
+  // Destroy this view controller and its view.
+  //
+  // If this view controller owns the engine, the engine is also destroyed.
+  void Destroy();
+
   FlutterWindowsEngine* engine() { return view_->GetEngine(); }
   FlutterWindowsView* view() { return view_.get(); }
 
@@ -36,6 +44,8 @@ class FlutterWindowsViewController {
   std::unique_ptr<FlutterWindowsEngine> engine_;
 
   std::unique_ptr<FlutterWindowsView> view_;
+
+  FML_DISALLOW_COPY_AND_ASSIGN(FlutterWindowsViewController);
 };
 
 }  // namespace flutter

shell/platform/windows/flutter_windows_view_unittests.cc

@@ -18,6 +18,7 @@
 #include "flutter/shell/platform/windows/flutter_window.h"
 #include "flutter/shell/platform/windows/flutter_windows_engine.h"
 #include "flutter/shell/platform/windows/flutter_windows_texture_registrar.h"
+#include "flutter/shell/platform/windows/flutter_windows_view_controller.h"
 #include "flutter/shell/platform/windows/testing/egl/mock_context.h"
 #include "flutter/shell/platform/windows/testing/egl/mock_manager.h"
 #include "flutter/shell/platform/windows/testing/egl/mock_window_surface.h"
@@ -121,6 +122,7 @@ class MockFlutterWindowsEngine : public FlutterWindowsEngine {
 
   MOCK_METHOD(bool, running, (), (const));
   MOCK_METHOD(bool, Stop, (), ());
+  MOCK_METHOD(void, RemoveView, (FlutterViewId view_id), ());
   MOCK_METHOD(bool, PostRasterThreadTask, (fml::closure), (const));
 
  private:
@@ -241,6 +243,8 @@ TEST(FlutterWindowsViewTest, Shutdown) {
       std::make_unique<NiceMock<MockWindowBindingHandler>>();
   auto egl_manager = std::make_unique<egl::MockManager>();
   auto surface = std::make_unique<egl::MockWindowSurface>();
+
+  auto engine_ptr = engine.get();
   auto surface_ptr = surface.get();
 
   EngineModifier modifier{engine.get()};
@@ -250,12 +254,16 @@ TEST(FlutterWindowsViewTest, Shutdown) {
     std::unique_ptr<FlutterWindowsView> view =
         engine->CreateView(std::move(window_binding_handler));
 
+    auto view_id = view->view_id();
     ViewModifier view_modifier{view.get()};
     view_modifier.SetSurface(std::move(surface));
 
-    // The engine must be stopped before the surface can be destroyed.
+    FlutterWindowsViewController controller{std::move(engine), std::move(view)};
+
+    // The view must be removed before the surface can be destroyed.
     InSequence s;
-    EXPECT_CALL(*engine.get(), Stop).Times(1);
+    EXPECT_CALL(*engine_ptr, running).WillOnce(Return(true));
+    EXPECT_CALL(*engine_ptr, RemoveView(view_id)).Times(1);
     EXPECT_CALL(*surface_ptr, Destroy).Times(1);
   }
 }
@@ -1392,7 +1400,6 @@ TEST(FlutterWindowsViewTest, DisablesVSyncAtStartup) {
   EXPECT_CALL(*surface_ptr, SetVSyncEnabled(false)).WillOnce(Return(true));
   EXPECT_CALL(render_context, ClearCurrent).WillOnce(Return(true));
 
-  EXPECT_CALL(*engine.get(), Stop).Times(1);
   EXPECT_CALL(*surface_ptr, Destroy).Times(1);
 
   EngineModifier modifier{engine.get()};
@@ -1430,7 +1437,6 @@ TEST(FlutterWindowsViewTest, EnablesVSyncAtStartup) {
   EXPECT_CALL(*surface_ptr, SetVSyncEnabled(true)).WillOnce(Return(true));
   EXPECT_CALL(render_context, ClearCurrent).WillOnce(Return(true));
 
-  EXPECT_CALL(*engine.get(), Stop).Times(1);
   EXPECT_CALL(*surface_ptr, Destroy).Times(1);
 
   EngineModifier modifier{engine.get()};
@@ -1471,7 +1477,6 @@ TEST(FlutterWindowsViewTest, DisablesVSyncAfterStartup) {
   EXPECT_CALL(*surface_ptr, MakeCurrent).WillOnce(Return(true));
   EXPECT_CALL(*surface_ptr, SetVSyncEnabled(false)).WillOnce(Return(true));
   EXPECT_CALL(render_context, ClearCurrent).WillOnce(Return(true));
-  EXPECT_CALL(*engine.get(), Stop).Times(1);
   EXPECT_CALL(*surface_ptr, Destroy).Times(1);
 
   EngineModifier modifier{engine.get()};
@@ -1514,7 +1519,6 @@ TEST(FlutterWindowsViewTest, EnablesVSyncAfterStartup) {
   EXPECT_CALL(*surface_ptr, MakeCurrent).WillOnce(Return(true));
   EXPECT_CALL(*surface_ptr, SetVSyncEnabled(true)).WillOnce(Return(true));
   EXPECT_CALL(render_context, ClearCurrent).WillOnce(Return(true));
-  EXPECT_CALL(*engine.get(), Stop).Times(1);
   EXPECT_CALL(*surface_ptr, Destroy).Times(1);
 
   EngineModifier modifier{engine.get()};
@@ -1563,7 +1567,6 @@ TEST(FlutterWindowsViewTest, UpdatesVSyncOnDwmUpdates) {
   EXPECT_CALL(*surface_ptr, SetVSyncEnabled(false)).WillOnce(Return(true));
   EXPECT_CALL(render_context, ClearCurrent).WillOnce(Return(true));
 
-  EXPECT_CALL(*engine.get(), Stop).Times(1);
   EXPECT_CALL(*surface_ptr, Destroy).Times(1);
 
   EngineModifier engine_modifier{engine.get()};