[Windows] Shutdown the engine before destroying the surface (flutter/engine#41012)

[Windows] Shutdown the engine before destroying the surface

engine/src/flutter/shell/platform/windows/angle_surface_manager.h

@@ -28,7 +28,7 @@ namespace flutter {
 class AngleSurfaceManager {
  public:
   static std::unique_ptr<AngleSurfaceManager> Create();
-  ~AngleSurfaceManager();
+  virtual ~AngleSurfaceManager();
 
   // Creates an EGLSurface wrapper and backing DirectX 11 SwapChain
   // associated with window, in the appropriate format for display.
@@ -51,7 +51,7 @@ class AngleSurfaceManager {
   void GetSurfaceDimensions(EGLint* width, EGLint* height);
 
   // Releases the pass-in EGLSurface wrapping and backing resources if not null.
-  void DestroySurface();
+  virtual void DestroySurface();
 
   // Binds egl_context_ to the current rendering thread and to the draw and read
   // surfaces returning a boolean result reflecting success.
@@ -79,11 +79,12 @@ class AngleSurfaceManager {
   // Gets the |ID3D11Device| chosen by ANGLE.
   bool GetDevice(ID3D11Device** device);
 
- private:
+ protected:
   // Creates a new surface manager retaining reference to the passed-in target
   // for the lifetime of the manager.
   AngleSurfaceManager();
 
+ private:
   bool Initialize();
   void CleanUp();
 

engine/src/flutter/shell/platform/windows/flutter_windows_engine.h

@@ -109,7 +109,7 @@ class FlutterWindowsEngine {
   // Stops the engine. This invalidates the pointer returned by engine().
   //
   // Returns false if stopping the engine fails, or if it was not running.
-  bool Stop();
+  virtual bool Stop();
 
   // Sets the view that is displaying this engine's content.
   void SetView(FlutterWindowsView* view);

engine/src/flutter/shell/platform/windows/flutter_windows_view.cc

@@ -49,6 +49,12 @@ FlutterWindowsView::FlutterWindowsView(
 }
 
 FlutterWindowsView::~FlutterWindowsView() {
+  // The engine renders into the view's surface. The engine must be
+  // shutdown before the view's resources can be destroyed.
+  if (engine_) {
+    engine_->Stop();
+  }
+
   DestroyRenderSurface();
 }
 

engine/src/flutter/shell/platform/windows/flutter_windows_view_unittests.cc

@@ -27,6 +27,9 @@
 namespace flutter {
 namespace testing {
 
+using ::testing::InSequence;
+using ::testing::NiceMock;
+
 constexpr uint64_t kScanCodeKeyA = 0x1e;
 constexpr uint64_t kVirtualKeyA = 0x41;
 
@@ -55,16 +58,22 @@ std::unique_ptr<std::vector<uint8_t>> keyHandlingResponse(bool handled) {
   return flutter::JsonMessageCodec::GetInstance().EncodeMessage(document);
 }
 
-// Returns an engine instance configured with dummy project path values, and
-// overridden methods for sending platform messages, so that the engine can
-// respond as if the framework were connected.
-std::unique_ptr<FlutterWindowsEngine> GetTestEngine() {
+// Returns a Flutter project with the required path values to create
+// a test engine.
+FlutterProjectBundle GetTestProject() {
   FlutterDesktopEngineProperties properties = {};
   properties.assets_path = L"C:\\foo\\flutter_assets";
   properties.icu_data_path = L"C:\\foo\\icudtl.dat";
   properties.aot_library_path = L"C:\\foo\\aot.so";
-  FlutterProjectBundle project(properties);
-  auto engine = std::make_unique<FlutterWindowsEngine>(project);
+
+  return FlutterProjectBundle{properties};
+}
+
+// Returns an engine instance configured with test project path values, and
+// overridden methods for sending platform messages, so that the engine can
+// respond as if the framework were connected.
+std::unique_ptr<FlutterWindowsEngine> GetTestEngine() {
+  auto engine = std::make_unique<FlutterWindowsEngine>(GetTestProject());
 
   EngineModifier modifier(engine.get());
 
@@ -94,15 +103,57 @@ std::unique_ptr<FlutterWindowsEngine> GetTestEngine() {
   return engine;
 }
 
+class MockFlutterWindowsEngine : public FlutterWindowsEngine {
+ public:
+  MockFlutterWindowsEngine() : FlutterWindowsEngine(GetTestProject()) {}
+
+  MOCK_METHOD0(Stop, bool());
+
+ private:
+  FML_DISALLOW_COPY_AND_ASSIGN(MockFlutterWindowsEngine);
+};
+
+class MockAngleSurfaceManager : public AngleSurfaceManager {
+ public:
+  MockAngleSurfaceManager() {}
+
+  MOCK_METHOD0(DestroySurface, void());
+
+ private:
+  FML_DISALLOW_COPY_AND_ASSIGN(MockAngleSurfaceManager);
+};
+
 }  // namespace
 
+// The view's surface must be destroyed after the engine is shutdown.
+// See: https://github.com/flutter/flutter/issues/124463
+TEST(FlutterWindowsViewTest, Shutdown) {
+  std::unique_ptr<MockFlutterWindowsEngine> engine =
+      std::make_unique<MockFlutterWindowsEngine>();
+  auto window_binding_handler =
+      std::make_unique<NiceMock<MockWindowBindingHandler>>();
+  std::unique_ptr<MockAngleSurfaceManager> surface_manager =
+      std::make_unique<MockAngleSurfaceManager>();
+
+  EngineModifier modifier(engine.get());
+  FlutterWindowsView view(std::move(window_binding_handler));
+
+  // The engine must be stopped before the surface can be destroyed.
+  InSequence s;
+  EXPECT_CALL(*engine.get(), Stop).Times(1);
+  EXPECT_CALL(*surface_manager.get(), DestroySurface).Times(1);
+
+  modifier.SetSurfaceManager(surface_manager.release());
+  view.SetEngine(std::move(engine));
+}
+
 TEST(FlutterWindowsViewTest, KeySequence) {
   std::unique_ptr<FlutterWindowsEngine> engine = GetTestEngine();
 
   test_response = false;
 
   auto window_binding_handler =
-      std::make_unique<::testing::NiceMock<MockWindowBindingHandler>>();
+      std::make_unique<NiceMock<MockWindowBindingHandler>>();
   FlutterWindowsView view(std::move(window_binding_handler));
   view.SetEngine(std::move(engine));
 
@@ -130,7 +181,7 @@ TEST(FlutterWindowsViewTest, EnableSemantics) {
       });
 
   auto window_binding_handler =
-      std::make_unique<::testing::NiceMock<MockWindowBindingHandler>>();
+      std::make_unique<NiceMock<MockWindowBindingHandler>>();
   FlutterWindowsView view(std::move(window_binding_handler));
   view.SetEngine(std::move(engine));
 
@@ -138,7 +189,7 @@ TEST(FlutterWindowsViewTest, EnableSemantics) {
   EXPECT_TRUE(semantics_enabled);
 }
 
-TEST(FlutterWindowsView, AddSemanticsNodeUpdate) {
+TEST(FlutterWindowsViewTest, AddSemanticsNodeUpdate) {
   std::unique_ptr<FlutterWindowsEngine> engine = GetTestEngine();
   EngineModifier modifier(engine.get());
   modifier.embedder_api().UpdateSemanticsEnabled =
@@ -147,7 +198,7 @@ TEST(FlutterWindowsView, AddSemanticsNodeUpdate) {
       };
 
   auto window_binding_handler =
-      std::make_unique<::testing::NiceMock<MockWindowBindingHandler>>();
+      std::make_unique<NiceMock<MockWindowBindingHandler>>();
   FlutterWindowsView view(std::move(window_binding_handler));
   view.SetEngine(std::move(engine));
 
@@ -237,7 +288,7 @@ TEST(FlutterWindowsView, AddSemanticsNodeUpdate) {
 //             node3
 //
 // node0 and node2 are grouping nodes. node1 and node2 are static text nodes.
-TEST(FlutterWindowsView, AddSemanticsNodeUpdateWithChildren) {
+TEST(FlutterWindowsViewTest, AddSemanticsNodeUpdateWithChildren) {
   std::unique_ptr<FlutterWindowsEngine> engine = GetTestEngine();
   EngineModifier modifier(engine.get());
   modifier.embedder_api().UpdateSemanticsEnabled =
@@ -246,7 +297,7 @@ TEST(FlutterWindowsView, AddSemanticsNodeUpdateWithChildren) {
       };
 
   auto window_binding_handler =
-      std::make_unique<::testing::NiceMock<MockWindowBindingHandler>>();
+      std::make_unique<NiceMock<MockWindowBindingHandler>>();
   FlutterWindowsView view(std::move(window_binding_handler));
   view.SetEngine(std::move(engine));
 
@@ -435,7 +486,7 @@ TEST(FlutterWindowsView, AddSemanticsNodeUpdateWithChildren) {
 //        node2
 //
 // node1 is a grouping node, node0 is a static text node.
-TEST(FlutterWindowsView, NonZeroSemanticsRoot) {
+TEST(FlutterWindowsViewTest, NonZeroSemanticsRoot) {
   std::unique_ptr<FlutterWindowsEngine> engine = GetTestEngine();
   EngineModifier modifier(engine.get());
   modifier.embedder_api().UpdateSemanticsEnabled =
@@ -444,7 +495,7 @@ TEST(FlutterWindowsView, NonZeroSemanticsRoot) {
       };
 
   auto window_binding_handler =
-      std::make_unique<::testing::NiceMock<MockWindowBindingHandler>>();
+      std::make_unique<NiceMock<MockWindowBindingHandler>>();
   FlutterWindowsView view(std::move(window_binding_handler));
   view.SetEngine(std::move(engine));
 
@@ -576,7 +627,7 @@ TEST(FlutterWindowsViewTest, AccessibilityHitTesting) {
       };
 
   auto window_binding_handler =
-      std::make_unique<::testing::NiceMock<MockWindowBindingHandler>>();
+      std::make_unique<NiceMock<MockWindowBindingHandler>>();
   FlutterWindowsView view(std::move(window_binding_handler));
   view.SetEngine(std::move(engine));
 
@@ -662,7 +713,7 @@ TEST(FlutterWindowsViewTest, WindowResizeTests) {
   EngineModifier modifier(engine.get());
 
   auto window_binding_handler =
-      std::make_unique<::testing::NiceMock<MockWindowBindingHandler>>();
+      std::make_unique<NiceMock<MockWindowBindingHandler>>();
 
   FlutterWindowsView view(std::move(window_binding_handler));
   view.SetEngine(std::move(engine));
@@ -721,7 +772,7 @@ TEST(FlutterWindowsViewTest, CheckboxNativeState) {
       };
 
   auto window_binding_handler =
-      std::make_unique<::testing::NiceMock<MockWindowBindingHandler>>();
+      std::make_unique<NiceMock<MockWindowBindingHandler>>();
   FlutterWindowsView view(std::move(window_binding_handler));
   view.SetEngine(std::move(engine));
 
@@ -867,7 +918,7 @@ TEST(FlutterWindowsViewTest, SwitchNativeState) {
       };
 
   auto window_binding_handler =
-      std::make_unique<::testing::NiceMock<MockWindowBindingHandler>>();
+      std::make_unique<NiceMock<MockWindowBindingHandler>>();
   FlutterWindowsView view(std::move(window_binding_handler));
   view.SetEngine(std::move(engine));
 
@@ -984,7 +1035,7 @@ TEST(FlutterWindowsViewTest, TooltipNodeData) {
       };
 
   auto window_binding_handler =
-      std::make_unique<::testing::NiceMock<MockWindowBindingHandler>>();
+      std::make_unique<NiceMock<MockWindowBindingHandler>>();
   FlutterWindowsView view(std::move(window_binding_handler));
   view.SetEngine(std::move(engine));