[platform_view] Only dispose view when it is removed from the composition order (flutter/engine#41521)

DisposeView happens every frame before laying out PlatformViews, with the order specified in `composition_order_`. Because `view_to_dispose_` is determined on UI thread and `composition_order_` is determined on the platform thread, there could be a race if the dart code on the UI thread runs faster than the rasterizer on the Platform thread, causing a view in `view_to_dispose_` is still in the `composition_order_`.

This PR delays the views in the `composition_order_` being disposed and wait for the next frame to dispose them. 

fixes https://github.com/flutter/flutter/issues/125223

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
This commit is contained in:
Chris Yang 2023-04-28 11:21:14 -07:00 committed by GitHub
parent 962cbf0a22
commit d8e93afbc2
2 changed files with 116 additions and 1 deletions

View File

@ -867,7 +867,14 @@ void FlutterPlatformViewsController::DisposeViews() {
FML_DCHECK([[NSThread currentThread] isMainThread]);
std::unordered_set<int64_t> views_to_composite(composition_order_.begin(),
composition_order_.end());
std::unordered_set<int64_t> views_to_delay_dispose;
for (int64_t viewId : views_to_dispose_) {
if (views_to_composite.count(viewId)) {
views_to_delay_dispose.insert(viewId);
continue;
}
UIView* root_view = root_views_[viewId].get();
[root_view removeFromSuperview];
views_.erase(viewId);
@ -877,7 +884,8 @@ void FlutterPlatformViewsController::DisposeViews() {
clip_count_.erase(viewId);
views_to_recomposite_.erase(viewId);
}
views_to_dispose_.clear();
views_to_dispose_ = std::move(views_to_delay_dispose);
}
void FlutterPlatformViewsController::BeginCATransaction() {

View File

@ -2790,4 +2790,111 @@ fml::RefPtr<fml::TaskRunner> CreateNewThread(std::string name) {
return NO;
}
- (void)testDisposingViewInCompositionOrderDoNotCrash {
flutter::FlutterPlatformViewsTestMockPlatformViewDelegate mock_delegate;
auto thread_task_runner = CreateNewThread("FlutterPlatformViewsTest");
flutter::TaskRunners runners(/*label=*/self.name.UTF8String,
/*platform=*/thread_task_runner,
/*raster=*/thread_task_runner,
/*ui=*/thread_task_runner,
/*io=*/thread_task_runner);
auto flutterPlatformViewsController = std::make_shared<flutter::FlutterPlatformViewsController>();
auto platform_view = std::make_unique<flutter::PlatformViewIOS>(
/*delegate=*/mock_delegate,
/*rendering_api=*/flutter::IOSRenderingAPI::kSoftware,
/*platform_views_controller=*/flutterPlatformViewsController,
/*task_runners=*/runners);
UIView* mockFlutterView = [[[UIView alloc] initWithFrame:CGRectMake(0, 0, 500, 500)] autorelease];
flutterPlatformViewsController->SetFlutterView(mockFlutterView);
FlutterPlatformViewsTestMockFlutterPlatformFactory* factory =
[[FlutterPlatformViewsTestMockFlutterPlatformFactory new] autorelease];
flutterPlatformViewsController->RegisterViewFactory(
factory, @"MockFlutterPlatformView",
FlutterPlatformViewGestureRecognizersBlockingPolicyEager);
FlutterResult result = ^(id result) {
};
flutterPlatformViewsController->OnMethodCall(
[FlutterMethodCall
methodCallWithMethodName:@"create"
arguments:@{@"id" : @0, @"viewType" : @"MockFlutterPlatformView"}],
result);
flutterPlatformViewsController->OnMethodCall(
[FlutterMethodCall
methodCallWithMethodName:@"create"
arguments:@{@"id" : @1, @"viewType" : @"MockFlutterPlatformView"}],
result);
{
// **** First frame, view id 0, 1 in the composition_order_, disposing view 0 is called. **** //
// No view should be disposed, or removed from the composition order.
flutterPlatformViewsController->BeginFrame(SkISize::Make(300, 300));
flutter::MutatorsStack stack;
SkMatrix finalMatrix;
auto embeddedViewParams0 =
std::make_unique<flutter::EmbeddedViewParams>(finalMatrix, SkSize::Make(300, 300), stack);
flutterPlatformViewsController->PrerollCompositeEmbeddedView(0, std::move(embeddedViewParams0));
flutterPlatformViewsController->CompositeEmbeddedView(0);
auto embeddedViewParams1 =
std::make_unique<flutter::EmbeddedViewParams>(finalMatrix, SkSize::Make(300, 300), stack);
flutterPlatformViewsController->PrerollCompositeEmbeddedView(1, std::move(embeddedViewParams1));
flutterPlatformViewsController->CompositeEmbeddedView(1);
XCTAssertEqual(flutterPlatformViewsController->EmbeddedViewCount(), 2UL);
XCTestExpectation* expectation = [self expectationWithDescription:@"dispose call ended."];
FlutterResult disposeResult = ^(id result) {
[expectation fulfill];
};
flutterPlatformViewsController->OnMethodCall(
[FlutterMethodCall methodCallWithMethodName:@"dispose" arguments:@0], disposeResult);
[self waitForExpectationsWithTimeout:30 handler:nil];
const SkImageInfo image_info = SkImageInfo::MakeN32Premul(1000, 1000);
sk_sp<SkSurface> mock_sk_surface = SkSurface::MakeRaster(image_info);
flutter::SurfaceFrame::FramebufferInfo framebuffer_info;
auto mock_surface = std::make_unique<flutter::SurfaceFrame>(
std::move(mock_sk_surface), framebuffer_info,
[](const flutter::SurfaceFrame& surface_frame, flutter::DlCanvas* canvas) { return true; },
/*frame_size=*/SkISize::Make(800, 600));
XCTAssertTrue(
flutterPlatformViewsController->SubmitFrame(nullptr, nullptr, std::move(mock_surface)));
// Disposing won't remove embedded views until the view is removed from the composition_order_
XCTAssertEqual(flutterPlatformViewsController->EmbeddedViewCount(), 2UL);
XCTAssertNotNil(flutterPlatformViewsController->GetPlatformViewByID(0));
XCTAssertNotNil(flutterPlatformViewsController->GetPlatformViewByID(1));
}
{
// **** Second frame, view id 1 in the composition_order_, no disposing view is called, **** //
// View 0 is removed from the composition order in this frame, hence also disposed.
flutterPlatformViewsController->BeginFrame(SkISize::Make(300, 300));
flutter::MutatorsStack stack;
SkMatrix finalMatrix;
auto embeddedViewParams1 =
std::make_unique<flutter::EmbeddedViewParams>(finalMatrix, SkSize::Make(300, 300), stack);
flutterPlatformViewsController->PrerollCompositeEmbeddedView(1, std::move(embeddedViewParams1));
flutterPlatformViewsController->CompositeEmbeddedView(1);
const SkImageInfo image_info = SkImageInfo::MakeN32Premul(1000, 1000);
sk_sp<SkSurface> mock_sk_surface = SkSurface::MakeRaster(image_info);
flutter::SurfaceFrame::FramebufferInfo framebuffer_info;
auto mock_surface = std::make_unique<flutter::SurfaceFrame>(
std::move(mock_sk_surface), framebuffer_info,
[](const flutter::SurfaceFrame& surface_frame, flutter::DlCanvas* canvas) { return true; },
/*frame_size=*/SkISize::Make(800, 600));
XCTAssertTrue(
flutterPlatformViewsController->SubmitFrame(nullptr, nullptr, std::move(mock_surface)));
// Disposing won't remove embedded views until the view is removed from the composition_order_
XCTAssertEqual(flutterPlatformViewsController->EmbeddedViewCount(), 1UL);
XCTAssertNil(flutterPlatformViewsController->GetPlatformViewByID(0));
XCTAssertNotNil(flutterPlatformViewsController->GetPlatformViewByID(1));
}
}
@end