Revert "Improve C++ plugin lifetime handling (#17489)" (#17563)

Seems to have triggered flaky failures on the Windows bot since landing.
Example failure:

    [ RUN      ] PluginRegistrarTest.ManagerRemovesOnDestruction
    c:\b\s\w\ir\cache\builder\src\flutter\shell\platform\common\cpp\client_wrapper\plugin_registrar_unittests.cc(149): error: Expected: (manager->GetRegistrar<PluginRegistrar>(dummy_registrar_handle)) != (first_wrapper), actual: 000002400A90E3D0 vs 000002400A90E3D0

This reverts commit faf44fe.

Author: cbracken

shell/platform/common/cpp/client_wrapper/BUILD.gn

@@ -65,6 +65,4 @@ executable("client_wrapper_unittests") {
     # https://github.com/flutter/flutter/issues/41414.
     "//third_party/dart/runtime:libdart_jit",
   ]
-
-  defines = [ "FLUTTER_DESKTOP_LIBRARY" ]
 }

shell/platform/common/cpp/client_wrapper/include/flutter/plugin_registrar.h

@@ -5,7 +5,6 @@
 #ifndef FLUTTER_SHELL_PLATFORM_COMMON_CPP_CLIENT_WRAPPER_INCLUDE_FLUTTER_PLUGIN_REGISTRAR_H_
 #define FLUTTER_SHELL_PLATFORM_COMMON_CPP_CLIENT_WRAPPER_INCLUDE_FLUTTER_PLUGIN_REGISTRAR_H_
 
-#include <map>
 #include <memory>
 #include <set>
 #include <string>
@@ -70,48 +69,6 @@ class Plugin {
   virtual ~Plugin() = default;
 };
 
-// A singleton to own PluginRegistrars. This is intended for use in plugins,
-// where there is no higher-level object to own a PluginRegistrar that can
-// own plugin instances and ensure that they live as long as the engine they
-// are registered with.
-class PluginRegistrarManager {
- public:
-  static PluginRegistrarManager* GetInstance();
-
-  // Prevent copying.
-  PluginRegistrarManager(PluginRegistrarManager const&) = delete;
-  PluginRegistrarManager& operator=(PluginRegistrarManager const&) = delete;
-
-  // Returns a plugin registrar wrapper of type T, which must be a kind of
-  // PluginRegistrar, creating it if necessary. The returned registrar will
-  // live as long as the underlying FlutterDesktopPluginRegistrarRef, so
-  // can be used to own plugin instances.
-  //
-  // Calling this multiple times for the same registrar_ref with different
-  // template types results in undefined behavior.
-  template <class T>
-  T* GetRegistrar(FlutterDesktopPluginRegistrarRef registrar_ref) {
-    auto insert_result =
-        registrars_.emplace(registrar_ref, std::make_unique<T>(registrar_ref));
-    auto& registrar_pair = *(insert_result.first);
-    FlutterDesktopRegistrarSetDestructionHandler(registrar_pair.first,
-                                                 OnRegistrarDestroyed);
-    return static_cast<T*>(registrar_pair.second.get());
-  }
-
- private:
-  PluginRegistrarManager();
-
-  using WrapperMap = std::map<FlutterDesktopPluginRegistrarRef,
-                              std::unique_ptr<PluginRegistrar>>;
-
-  static void OnRegistrarDestroyed(FlutterDesktopPluginRegistrarRef registrar);
-
-  WrapperMap* registrars() { return &registrars_; }
-
-  WrapperMap registrars_;
-};
-
 }  // namespace flutter
 
 #endif  // FLUTTER_SHELL_PLATFORM_COMMON_CPP_CLIENT_WRAPPER_INCLUDE_FLUTTER_PLUGIN_REGISTRAR_H_

shell/platform/common/cpp/client_wrapper/plugin_registrar.cc

@@ -138,7 +138,7 @@ void BinaryMessengerImpl::SetMessageHandler(const std::string& channel,
                                      ForwardToHandler, message_handler);
 }
 
-// ===== PluginRegistrar =====
+// PluginRegistrar:
 
 PluginRegistrar::PluginRegistrar(FlutterDesktopPluginRegistrarRef registrar)
     : registrar_(registrar) {
@@ -157,20 +157,4 @@ void PluginRegistrar::EnableInputBlockingForChannel(
   FlutterDesktopRegistrarEnableInputBlocking(registrar_, channel.c_str());
 }
 
-// ===== PluginRegistrarManager =====
-
-// static
-PluginRegistrarManager* PluginRegistrarManager::GetInstance() {
-  static PluginRegistrarManager* instance = new PluginRegistrarManager();
-  return instance;
-}
-
-PluginRegistrarManager::PluginRegistrarManager() = default;
-
-// static
-void PluginRegistrarManager::OnRegistrarDestroyed(
-    FlutterDesktopPluginRegistrarRef registrar) {
-  GetInstance()->registrars()->erase(registrar);
-}
-
 }  // namespace flutter

shell/platform/common/cpp/client_wrapper/plugin_registrar_unittests.cc

@@ -33,36 +33,28 @@ class TestApi : public testing::StubFlutterApi {
     return message_engine_result;
   }
 
+  // Called for FlutterDesktopMessengerSetCallback.
   void MessengerSetCallback(const char* channel,
                             FlutterDesktopMessageCallback callback,
                             void* user_data) override {
-    last_message_callback_set_ = callback;
-  }
-
-  void RegistrarSetDestructionHandler(
-      FlutterDesktopOnRegistrarDestroyed callback) override {
-    last_destruction_callback_set_ = callback;
+    last_callback_set_ = callback;
   }
 
   const uint8_t* last_data_sent() { return last_data_sent_; }
-  FlutterDesktopMessageCallback last_message_callback_set() {
-    return last_message_callback_set_;
-  }
-  FlutterDesktopOnRegistrarDestroyed last_destruction_callback_set() {
-    return last_destruction_callback_set_;
+  FlutterDesktopMessageCallback last_callback_set() {
+    return last_callback_set_;
   }
 
  private:
   const uint8_t* last_data_sent_ = nullptr;
-  FlutterDesktopMessageCallback last_message_callback_set_ = nullptr;
-  FlutterDesktopOnRegistrarDestroyed last_destruction_callback_set_ = nullptr;
+  FlutterDesktopMessageCallback last_callback_set_ = nullptr;
 };
 
 }  // namespace
 
 // Tests that the registrar returns a messenger that passes Send through to the
 // C API.
-TEST(PluginRegistrarTest, MessengerSend) {
+TEST(MethodCallTest, MessengerSend) {
   testing::ScopedStubFlutterApi scoped_api_stub(std::make_unique<TestApi>());
   auto test_api = static_cast<TestApi*>(scoped_api_stub.stub());
 
@@ -78,7 +70,7 @@ TEST(PluginRegistrarTest, MessengerSend) {
 
 // Tests that the registrar returns a messenger that passes callback
 // registration and unregistration through to the C API.
-TEST(PluginRegistrarTest, MessengerSetMessageHandler) {
+TEST(MethodCallTest, MessengerSetMessageHandler) {
   testing::ScopedStubFlutterApi scoped_api_stub(std::make_unique<TestApi>());
   auto test_api = static_cast<TestApi*>(scoped_api_stub.stub());
 
@@ -93,60 +85,11 @@ TEST(PluginRegistrarTest, MessengerSetMessageHandler) {
                                            const size_t message_size,
                                            BinaryReply reply) {};
   messenger->SetMessageHandler(channel_name, std::move(binary_handler));
-  EXPECT_NE(test_api->last_message_callback_set(), nullptr);
+  EXPECT_NE(test_api->last_callback_set(), nullptr);
 
   // Unregister.
   messenger->SetMessageHandler(channel_name, nullptr);
-  EXPECT_EQ(test_api->last_message_callback_set(), nullptr);
-}
-
-// Tests that the registrar manager returns the same instance when getting
-// the wrapper for the same reference.
-TEST(PluginRegistrarTest, ManagerSameInstance) {
-  testing::ScopedStubFlutterApi scoped_api_stub(std::make_unique<TestApi>());
-
-  auto dummy_registrar_handle =
-      reinterpret_cast<FlutterDesktopPluginRegistrarRef>(1);
-
-  PluginRegistrarManager* manager = PluginRegistrarManager::GetInstance();
-  EXPECT_EQ(manager->GetRegistrar<PluginRegistrar>(dummy_registrar_handle),
-            manager->GetRegistrar<PluginRegistrar>(dummy_registrar_handle));
-}
-
-// Tests that the registrar manager returns different objects for different
-// references.
-TEST(PluginRegistrarTest, ManagerDifferentInstances) {
-  testing::ScopedStubFlutterApi scoped_api_stub(std::make_unique<TestApi>());
-
-  auto dummy_registrar_handle_a =
-      reinterpret_cast<FlutterDesktopPluginRegistrarRef>(1);
-  auto dummy_registrar_handle_b =
-      reinterpret_cast<FlutterDesktopPluginRegistrarRef>(2);
-
-  PluginRegistrarManager* manager = PluginRegistrarManager::GetInstance();
-  EXPECT_NE(manager->GetRegistrar<PluginRegistrar>(dummy_registrar_handle_a),
-            manager->GetRegistrar<PluginRegistrar>(dummy_registrar_handle_b));
-}
-
-// Tests that the registrar manager deletes wrappers when the underlying
-// reference is destroyed.
-TEST(PluginRegistrarTest, ManagerRemovesOnDestruction) {
-  testing::ScopedStubFlutterApi scoped_api_stub(std::make_unique<TestApi>());
-  auto test_api = static_cast<TestApi*>(scoped_api_stub.stub());
-
-  auto dummy_registrar_handle =
-      reinterpret_cast<FlutterDesktopPluginRegistrarRef>(1);
-  PluginRegistrarManager* manager = PluginRegistrarManager::GetInstance();
-  auto* first_wrapper =
-      manager->GetRegistrar<PluginRegistrar>(dummy_registrar_handle);
-
-  // Simulate destruction of the reference.
-  EXPECT_NE(test_api->last_destruction_callback_set(), nullptr);
-  test_api->last_destruction_callback_set()(dummy_registrar_handle);
-
-  // Requesting the wrapper should now create a new object.
-  EXPECT_NE(manager->GetRegistrar<PluginRegistrar>(dummy_registrar_handle),
-            first_wrapper);
+  EXPECT_EQ(test_api->last_callback_set(), nullptr);
 }
 
 }  // namespace flutter

shell/platform/common/cpp/client_wrapper/testing/stub_flutter_api.cc

@@ -44,14 +44,6 @@ FlutterDesktopMessengerRef FlutterDesktopRegistrarGetMessenger(
   return reinterpret_cast<FlutterDesktopMessengerRef>(1);
 }
 
-void FlutterDesktopRegistrarSetDestructionHandler(
-    FlutterDesktopPluginRegistrarRef registrar,
-    FlutterDesktopOnRegistrarDestroyed callback) {
-  if (s_stub_implementation) {
-    s_stub_implementation->RegistrarSetDestructionHandler(callback);
-  }
-}
-
 void FlutterDesktopRegistrarEnableInputBlocking(
     FlutterDesktopPluginRegistrarRef registrar,
     const char* channel) {

shell/platform/common/cpp/client_wrapper/testing/stub_flutter_api.h

@@ -34,10 +34,6 @@ class StubFlutterApi {
 
   virtual ~StubFlutterApi() {}
 
-  // Called for FlutterDesktopRegistrarSetDestructionHandler.
-  virtual void RegistrarSetDestructionHandler(
-      FlutterDesktopOnRegistrarDestroyed callback) {}
-
   // Called for FlutterDesktopRegistrarEnableInputBlocking.
   virtual void RegistrarEnableInputBlocking(const char* channel) {}
 

shell/platform/common/cpp/public/flutter_plugin_registrar.h

@@ -18,19 +18,10 @@ extern "C" {
 // Opaque reference to a plugin registrar.
 typedef struct FlutterDesktopPluginRegistrar* FlutterDesktopPluginRegistrarRef;
 
-// Function pointer type for registrar destruction callback.
-typedef void (*FlutterDesktopOnRegistrarDestroyed)(
-    FlutterDesktopPluginRegistrarRef);
-
 // Returns the engine messenger associated with this registrar.
 FLUTTER_EXPORT FlutterDesktopMessengerRef
 FlutterDesktopRegistrarGetMessenger(FlutterDesktopPluginRegistrarRef registrar);
 
-// Registers a callback to be called when the plugin registrar is destroyed.
-FLUTTER_EXPORT void FlutterDesktopRegistrarSetDestructionHandler(
-    FlutterDesktopPluginRegistrarRef registrar,
-    FlutterDesktopOnRegistrarDestroyed callback);
-
 // Enables input blocking on the given channel.
 //
 // If set, then the Flutter window will disable input callbacks

shell/platform/glfw/flutter_glfw.cc

@@ -118,9 +118,6 @@ struct FlutterDesktopPluginRegistrar {
 
   // The handle for the window associated with this registrar.
   FlutterDesktopWindow* window;
-
-  // Callback to be called on registrar destruction.
-  FlutterDesktopOnRegistrarDestroyed destruction_handler;
 };
 
 // State associated with the messenger used to communicate with the engine.
@@ -685,11 +682,6 @@ FlutterDesktopWindowControllerRef FlutterDesktopCreateWindow(
 }
 
 void FlutterDesktopDestroyWindow(FlutterDesktopWindowControllerRef controller) {
-  FlutterDesktopPluginRegistrarRef registrar =
-      controller->plugin_registrar.get();
-  if (registrar->destruction_handler) {
-    registrar->destruction_handler(registrar);
-  }
   FlutterEngineShutdown(controller->engine);
   delete controller;
 }
@@ -840,12 +832,6 @@ FlutterDesktopMessengerRef FlutterDesktopRegistrarGetMessenger(
   return registrar->messenger.get();
 }
 
-void FlutterDesktopRegistrarSetDestructionHandler(
-    FlutterDesktopPluginRegistrarRef registrar,
-    FlutterDesktopOnRegistrarDestroyed callback) {
-  registrar->destruction_handler = callback;
-}
-
 FlutterDesktopWindowRef FlutterDesktopRegistrarGetWindow(
     FlutterDesktopPluginRegistrarRef registrar) {
   return registrar->window;

shell/platform/windows/flutter_windows.cc

@@ -248,12 +248,6 @@ FlutterDesktopMessengerRef FlutterDesktopRegistrarGetMessenger(
   return registrar->messenger.get();
 }
 
-void FlutterDesktopRegistrarSetDestructionHandler(
-    FlutterDesktopPluginRegistrarRef registrar,
-    FlutterDesktopOnRegistrarDestroyed callback) {
-  registrar->destruction_handler = callback;
-}
-
 FlutterDesktopViewRef FlutterDesktopRegistrarGetView(
     FlutterDesktopPluginRegistrarRef registrar) {
   return registrar->window;

shell/platform/windows/win32_flutter_window.cc

@@ -15,9 +15,6 @@ Win32FlutterWindow::Win32FlutterWindow(int width, int height) {
 
 Win32FlutterWindow::~Win32FlutterWindow() {
   DestroyRenderSurface();
-  if (plugin_registrar_ && plugin_registrar_->destruction_handler) {
-    plugin_registrar_->destruction_handler(plugin_registrar_.get());
-  }
 }
 
 FlutterDesktopViewControllerRef Win32FlutterWindow::CreateWin32FlutterWindow(