DEV/flutter_flutter
1
0
Fork 0
mirror of https://github.com/flutter/flutter.git synced 2026-02-20 02:29:02 +08:00
Code Issues Packages Projects Releases Wiki Activity

[Windows] Add/remove view failures should not hang (flutter/engine#52164)

Browse Source 
If the embedder API's `FlutterEngineAddView` and `FlutterEngineRemoveView` don't return `kSuccess`, their callbacks won't be invoked. See: [flutter.dev/go/multi-view-embedder-apis](https://flutter.dev/go/multi-view-embedder-apis).

Previously, the Windows embedder would hang in this scenario as it blocks until the callbacks are invoked. Now, the Windows embedder only blocks if these embedder APIs return `kSuccess`.

Kudos to @dkwingsmt for catching this!

Part of https://github.com/flutter/flutter/issues/144810
Part of https://github.com/flutter/flutter/issues/142845

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
This commit is contained in:
Loïc Sharma 2024-04-17 09:44:28 -07:00 committed by GitHub
parent e22b8448cb
commit e1b38345d3
2 changed files with 70 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
engine/src/flutter/shell/platform/windows/flutter_windows_engine.cc
View File

@ -532,7 +532,15 @@ std::unique_ptr<FlutterWindowsView> FlutterWindowsEngine::CreateView(
captures->latch.Signal();
};
embedder_api_.AddView(engine_, &info);
FlutterEngineResult result = embedder_api_.AddView(engine_, &info);
if (result != kSuccess) {
FML_LOG(ERROR)
<< "Starting the add view operation failed. FlutterEngineAddView "
"returned an unexpected result: "
<< result << ". This indicates a bug in the Windows embedder.";
FML_DCHECK(false);
return nullptr;
}
// Block the platform thread until the engine has added the view.
// TODO(loicsharma): This blocks the platform thread eagerly and can
@ -573,15 +581,24 @@ void FlutterWindowsEngine::RemoveView(FlutterViewId view_id) {
info.view_id = view_id;
info.user_data = &captures;
info.remove_view_callback = [](const FlutterRemoveViewResult* result) {
// This is invoked on the raster thread, the same thread that the present
// callback is invoked. If |FlutterRemoveViewResult.removed| is `true`,
// the engine guarantees the view won't be presented.
// This is invoked on an engine thread. If
// |FlutterRemoveViewResult.removed| is `true`, the engine guarantees the
// view won't be presented.
Captures* captures = reinterpret_cast<Captures*>(result->user_data);
captures->removed = result->removed;
captures->latch.Signal();
};
embedder_api_.RemoveView(engine_, &info);
FlutterEngineResult result = embedder_api_.RemoveView(engine_, &info);
if (result != kSuccess) {
FML_LOG(ERROR) << "Starting the remove view operation failed. "
"FlutterEngineRemoveView "
"returned an unexpected result: "
<< result
<< ". This indicates a bug in the Windows embedder.";
FML_DCHECK(false);
return;
}
// Block the platform thread until the engine has removed the view.
// TODO(loicsharma): This blocks the platform thread eagerly and can

48
engine/src/flutter/shell/platform/windows/flutter_windows_engine_unittests.cc
View File

@ -1265,5 +1265,53 @@ TEST_F(FlutterWindowsEngineTest, ReceivePlatformViewMessage) {
}
}
TEST_F(FlutterWindowsEngineTest, AddViewFailureDoesNotHang) {
FlutterWindowsEngineBuilder builder{GetContext()};
auto engine = builder.Build();
EngineModifier modifier{engine.get()};
modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; };
modifier.embedder_api().AddView = MOCK_ENGINE_PROC(
AddView,
[](FLUTTER_API_SYMBOL(FlutterEngine) engine,
const FlutterAddViewInfo* info) { return kInternalInconsistency; });
ASSERT_TRUE(engine->Run());
// Create the first view. This is the implicit view and isn't added to the
// engine.
auto implicit_window = std::make_unique<NiceMock<MockWindowBindingHandler>>();
std::unique_ptr<FlutterWindowsView> implicit_view =
engine->CreateView(std::move(implicit_window));
EXPECT_TRUE(implicit_view);
// Create a second view. The embedder attempts to add it to the engine.
auto second_window = std::make_unique<NiceMock<MockWindowBindingHandler>>();
EXPECT_DEBUG_DEATH(engine->CreateView(std::move(second_window)),
"FlutterEngineAddView returned an unexpected result");
}
TEST_F(FlutterWindowsEngineTest, RemoveViewFailureDoesNotHang) {
FlutterWindowsEngineBuilder builder{GetContext()};
builder.SetDartEntrypoint("sendCreatePlatformViewMethod");
auto engine = builder.Build();
EngineModifier modifier{engine.get()};
modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; };
modifier.embedder_api().RemoveView = MOCK_ENGINE_PROC(
RemoveView,
[](FLUTTER_API_SYMBOL(FlutterEngine) engine,
const FlutterRemoveViewInfo* info) { return kInternalInconsistency; });
ASSERT_TRUE(engine->Run());
EXPECT_DEBUG_DEATH(engine->RemoveView(123),
"FlutterEngineRemoveView returned an unexpected result");
}
} // namespace testing
} // namespace flutter