Fix Windows plugin destruction #2

jnschulze · Jan 29, 2022 · 778084a · 778084a
1 parent 1c3dd26
commit 778084a
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 7 deletions.
diff --git a/shell/platform/windows/flutter_windows.cc b/shell/platform/windows/flutter_windows.cc
@@ -213,7 +213,7 @@ FlutterDesktopMessengerRef FlutterDesktopPluginRegistrarGetMessenger(
 void FlutterDesktopPluginRegistrarSetDestructionHandler(
     FlutterDesktopPluginRegistrarRef registrar,
     FlutterDesktopOnPluginRegistrarDestroyed callback) {
-  registrar->engine->AddPluginRegistrarDestructionCallback(callback);
+  registrar->engine->AddPluginRegistrarDestructionCallback(registrar, callback);
 }
 
 bool FlutterDesktopMessengerSendWithReply(FlutterDesktopMessengerRef messenger,

diff --git a/shell/platform/windows/flutter_windows_engine.cc b/shell/platform/windows/flutter_windows_engine.cc
@@ -314,8 +314,9 @@ bool FlutterWindowsEngine::RunWithEntrypoint(const char* entrypoint) {
 
 bool FlutterWindowsEngine::Stop() {
   if (engine_) {
-    for (const auto callback : plugin_registrar_destruction_callbacks_) {
-      callback(plugin_registrar_.get());
+    for (const auto [registrar, callback] :
+         plugin_registrar_destruction_callbacks_) {
+      callback(registrar);
     }
 
     FlutterEngineResult result = embedder_api_.Shutdown(engine_);
@@ -335,8 +336,9 @@ FlutterDesktopPluginRegistrarRef FlutterWindowsEngine::GetRegistrar() {
 }
 
 void FlutterWindowsEngine::AddPluginRegistrarDestructionCallback(
+    FlutterDesktopPluginRegistrarRef registrar,
     FlutterDesktopOnPluginRegistrarDestroyed callback) {
-  plugin_registrar_destruction_callbacks_.insert(callback);
+  plugin_registrar_destruction_callbacks_[registrar] = callback;
 }
 
 void FlutterWindowsEngine::SendWindowMetricsEvent(

diff --git a/shell/platform/windows/flutter_windows_engine.h b/shell/platform/windows/flutter_windows_engine.h
@@ -8,7 +8,6 @@
 #include <map>
 #include <memory>
 #include <optional>
-#include <set>
 #include <vector>
 
 #include "flutter/shell/platform/common/accessibility_bridge.h"
@@ -74,6 +73,7 @@ class FlutterWindowsEngine {
 
   // Registers a |callback| to be called when the plugin registrar is destroyed.
   void AddPluginRegistrarDestructionCallback(
+      FlutterDesktopPluginRegistrarRef registrar,
       FlutterDesktopOnPluginRegistrarDestroyed callback);
 
   // Sets switches member to the given switches.
@@ -219,9 +219,10 @@ class FlutterWindowsEngine {
   // The MethodChannel used for communication with the Flutter engine.
   std::unique_ptr<BasicMessageChannel<rapidjson::Document>> settings_channel_;
 
-  // A set of callbacks to be called when the engine (and thus the plugin
+  // Callbacks to be called when the engine (and thus the plugin
   // registrar) is being destroyed.
-  std::set<FlutterDesktopOnPluginRegistrarDestroyed>
+  std::map<FlutterDesktopPluginRegistrarRef,
+           FlutterDesktopOnPluginRegistrarDestroyed>
       plugin_registrar_destruction_callbacks_;
 
   bool semantics_enabled_ = false;

diff --git a/shell/platform/windows/flutter_windows_engine_unittests.cc b/shell/platform/windows/flutter_windows_engine_unittests.cc
@@ -7,6 +7,7 @@
 #include "flutter/shell/platform/embedder/embedder.h"
 #include "flutter/shell/platform/embedder/test_utils/proc_table_replacement.h"
 #include "flutter/shell/platform/windows/testing/engine_modifier.h"
+#include "flutter/shell/platform/windows/testing/test_keyboard.h"
 #include "gtest/gtest.h"
 
 namespace flutter {
@@ -255,5 +256,36 @@ TEST(FlutterWindowsEngine, DispatchSemanticsAction) {
   EXPECT_TRUE(called);
 }
 
+TEST(FlutterWindowsEngine, AddPluginRegistrarDestructionCallback) {
+  std::unique_ptr<FlutterWindowsEngine> engine = GetTestEngine();
+  EngineModifier modifier(engine.get());
+
+  MockEmbedderApiForKeyboard(
+      modifier, [] { return false; },
+      [](const FlutterKeyEvent* event) { return false; });
+
+  engine->RunWithEntrypoint(nullptr);
+
+  // Ensure that destruction handlers don't overwrite each other.
+  int result1 = 0;
+  int result2 = 0;
+  engine->AddPluginRegistrarDestructionCallback(
+      reinterpret_cast<FlutterDesktopPluginRegistrarRef>(&result1),
+      [](FlutterDesktopPluginRegistrarRef ref) {
+        auto result = reinterpret_cast<int*>(ref);
+        *result = 1;
+      });
+  engine->AddPluginRegistrarDestructionCallback(
+      reinterpret_cast<FlutterDesktopPluginRegistrarRef>(&result2),
+      [](FlutterDesktopPluginRegistrarRef ref) {
+        auto result = reinterpret_cast<int*>(ref);
+        *result = 2;
+      });
+
+  engine->Stop();
+  EXPECT_EQ(result1, 1);
+  EXPECT_EQ(result2, 2);
+}
+
 }  // namespace testing
 }  // namespace flutter