Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Windows: Fix plugin destruction #30760

Merged
merged 1 commit into from
Mar 3, 2022
Merged

Windows: Fix plugin destruction #30760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion shell/platform/windows/flutter_windows.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ FlutterDesktopMessengerRef FlutterDesktopPluginRegistrarGetMessenger(
void FlutterDesktopPluginRegistrarSetDestructionHandler(
FlutterDesktopPluginRegistrarRef registrar,
FlutterDesktopOnPluginRegistrarDestroyed callback) {
registrar->engine->SetPluginRegistrarDestructionCallback(callback);
registrar->engine->AddPluginRegistrarDestructionCallback(callback, registrar);
}

bool FlutterDesktopMessengerSendWithReply(FlutterDesktopMessengerRef messenger,
Expand Down
12 changes: 7 additions & 5 deletions shell/platform/windows/flutter_windows_engine.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,8 +314,9 @@ bool FlutterWindowsEngine::RunWithEntrypoint(const char* entrypoint) {

bool FlutterWindowsEngine::Stop() {
if (engine_) {
if (plugin_registrar_destruction_callback_) {
plugin_registrar_destruction_callback_(plugin_registrar_.get());
for (const auto& [callback, registrar] :
plugin_registrar_destruction_callbacks_) {
callback(registrar);
}
FlutterEngineResult result = embedder_api_.Shutdown(engine_);
engine_ = nullptr;
Expand All @@ -333,9 +334,10 @@ FlutterDesktopPluginRegistrarRef FlutterWindowsEngine::GetRegistrar() {
return plugin_registrar_.get();
}

void FlutterWindowsEngine::SetPluginRegistrarDestructionCallback(
FlutterDesktopOnPluginRegistrarDestroyed callback) {
plugin_registrar_destruction_callback_ = callback;
void FlutterWindowsEngine::AddPluginRegistrarDestructionCallback(
FlutterDesktopOnPluginRegistrarDestroyed callback,
FlutterDesktopPluginRegistrarRef registrar) {
plugin_registrar_destruction_callbacks_[callback] = registrar;
}

void FlutterWindowsEngine::SendWindowMetricsEvent(
Expand Down
16 changes: 9 additions & 7 deletions shell/platform/windows/flutter_windows_engine.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,10 @@ class FlutterWindowsEngine {
// Returns the currently configured Plugin Registrar.
FlutterDesktopPluginRegistrarRef GetRegistrar();

// Sets |callback| to be called when the plugin registrar is destroyed.
void SetPluginRegistrarDestructionCallback(
FlutterDesktopOnPluginRegistrarDestroyed callback);
// Registers |callback| to be called when the plugin registrar is destroyed.
void AddPluginRegistrarDestructionCallback(
FlutterDesktopOnPluginRegistrarDestroyed callback,
FlutterDesktopPluginRegistrarRef registrar);

// Sets switches member to the given switches.
void SetSwitches(const std::vector<std::string>& switches);
Expand Down Expand Up @@ -215,10 +216,11 @@ class FlutterWindowsEngine {
// The MethodChannel used for communication with the Flutter engine.
std::unique_ptr<BasicMessageChannel<rapidjson::Document>> settings_channel_;

// A callback to be called when the engine (and thus the plugin registrar)
// is being destroyed.
FlutterDesktopOnPluginRegistrarDestroyed
plugin_registrar_destruction_callback_ = nullptr;
// Callbacks to be called when the engine (and thus the plugin registrar) is
// being destroyed.
std::map<FlutterDesktopOnPluginRegistrarDestroyed,
FlutterDesktopPluginRegistrarRef>
plugin_registrar_destruction_callbacks_;

bool semantics_enabled_ = false;

Expand Down
31 changes: 31 additions & 0 deletions shell/platform/windows/flutter_windows_engine_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -255,5 +256,35 @@ TEST(FlutterWindowsEngine, DispatchSemanticsAction) {
EXPECT_TRUE(called);
}

TEST(FlutterWindowsEngine, AddPluginRegistrarDestructionCallback) {
std::unique_ptr<FlutterWindowsEngine> engine = GetTestEngine();
EngineModifier modifier(engine.get());

MockEmbedderApiForKeyboard(modifier,
std::make_shared<MockKeyResponseController>());
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g Mar 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this test need a mock keyboard handler?

Copy link
Member Author

@jnschulze jnschulze Mar 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MockEmbedderApiForKeyboard mocks a couple of embedder API functions required by engine.RunWithEntrypoint. see

engine/shell/platform/windows/testing/test_keyboard.cc

Lines 100 to 192 in 8509510

void MockEmbedderApiForKeyboard(
EngineModifier& modifier,
std::shared_ptr<MockKeyResponseController> response_controller) {
stored_response_controller = response_controller;
// This mock handles channel messages.
modifier.embedder_api()
.SendPlatformMessage = [](FLUTTER_API_SYMBOL(FlutterEngine) engine,
const FlutterPlatformMessage* message) {
if (std::string(message->channel) == std::string("flutter/settings")) {
return kSuccess;
}
if (std::string(message->channel) == std::string("flutter/keyevent")) {
stored_response_controller->HandleChannelMessage([message](bool handled) {
auto response = _keyHandlingResponse(handled);
auto response_handle = message->response_handle;
if (response_handle->callback != nullptr) {
response_handle->callback(response->data(), response->size(),
response_handle->user_data);
}
});
return kSuccess;
}
if (std::string(message->channel) == std::string("flutter/textinput")) {
std::unique_ptr<rapidjson::Document> document =
flutter::JsonMessageCodec::GetInstance().DecodeMessage(
message->message, message->message_size);
if (document == nullptr) {
return kInvalidArguments;
}
stored_response_controller->HandleTextInputMessage(std::move(document));
return kSuccess;
}
return kSuccess;
};
// This mock handles key events sent through the embedder API.
modifier.embedder_api().SendKeyEvent =
[](FLUTTER_API_SYMBOL(FlutterEngine) engine, const FlutterKeyEvent* event,
FlutterKeyEventCallback callback, void* user_data) {
stored_response_controller->HandleEmbedderMessage(
event, [callback, user_data](bool handled) {
if (callback != nullptr) {
callback(handled, user_data);
}
});
return kSuccess;
};
// The following mocks enable channel mocking.
modifier.embedder_api().PlatformMessageCreateResponseHandle =
[](auto engine, auto data_callback, auto user_data, auto response_out) {
auto response_handle = new FlutterPlatformMessageResponseHandle();
response_handle->user_data = user_data;
response_handle->callback = data_callback;
*response_out = response_handle;
return kSuccess;
};
modifier.embedder_api().PlatformMessageReleaseResponseHandle =
[](FLUTTER_API_SYMBOL(FlutterEngine) engine,
FlutterPlatformMessageResponseHandle* response) {
delete response;
return kSuccess;
};
// The following mock disables responses for method channels sent from the
// embedding to the framework. (No code uses the response yet.)
modifier.embedder_api().SendPlatformMessageResponse =
[](FLUTTER_API_SYMBOL(FlutterEngine) engine,
const FlutterPlatformMessageResponseHandle* handle,
const uint8_t* data, size_t data_length) { return kSuccess; };
// The following mocks allows RunWithEntrypoint to be run, which creates a
// non-empty FlutterEngine and enables SendKeyEvent.
modifier.embedder_api().Run =
[](size_t version, const FlutterRendererConfig* config,
const FlutterProjectArgs* args, void* user_data,
FLUTTER_API_SYMBOL(FlutterEngine) * engine_out) {
*engine_out = reinterpret_cast<FLUTTER_API_SYMBOL(FlutterEngine)>(1);
return kSuccess;
};
modifier.embedder_api().UpdateLocales =
[](auto engine, const FlutterLocale** locales, size_t locales_count) {
return kSuccess;
};
modifier.embedder_api().SendWindowMetricsEvent =
[](auto engine, const FlutterWindowMetricsEvent* event) {
return kSuccess;
};
modifier.embedder_api().Shutdown = [](auto engine) { return kSuccess; };
}

Should I just mock the required functions explicitly?


engine->RunWithEntrypoint(nullptr);

// Verify that destruction handlers don't overwrite each other.
int result1 = 0;
int result2 = 0;
engine->AddPluginRegistrarDestructionCallback(
[](FlutterDesktopPluginRegistrarRef ref) {
auto result = reinterpret_cast<int*>(ref);
*result = 1;
},
reinterpret_cast<FlutterDesktopPluginRegistrarRef>(&result1));
engine->AddPluginRegistrarDestructionCallback(
[](FlutterDesktopPluginRegistrarRef ref) {
auto result = reinterpret_cast<int*>(ref);
*result = 2;
},
reinterpret_cast<FlutterDesktopPluginRegistrarRef>(&result2));

engine->Stop();
EXPECT_EQ(result1, 1);
EXPECT_EQ(result2, 2);
}

} // namespace testing
} // namespace flutter