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

[platform_view] Only dispose view when it is removed from the composition order #41521

Merged
merged 3 commits into from
Apr 28, 2023
Merged

[platform_view] Only dispose view when it is removed from the composition order #41521

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2eaeffe
# This is a combination of 3 commits.
Apr 20, 2023
a113c4b
std move
Apr 28, 2023
c8aa713
renamefix
Apr 28, 2023
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
10 changes: 9 additions & 1 deletion shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -867,7 +867,14 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,

FML_DCHECK([[NSThread currentThread] isMainThread]);
Copy link
Contributor

Choose a reason for hiding this comment

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

thx for the chat just now. just to check if i understand it correctly (let me know if i miss anything):

Dart layer sends separate messages (info about views_to_dispose and composition_order) to the engine. The composition_order contains what should be rendered on the current frame, and views_to_dispose contains views that needs to be disposed. We need separate messages because a platform view could be just hidden and not disposed, so it won't show up in composition_order.

The crash happens when views_to_dispose message is delivered earlier than the new composition_order (due to the async nature of the message channel). So when rendering the current frame, a platform view is already disposed, but it still exists in composition_order. The fix here is simply to delay the views_to_dispose if it's still part of composition_order, and try again next frame. Alternative fix is to remove the view from composition_order, but we decided that composition_order should be the source of truth of the current frame.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes.


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);
Expand All @@ -877,7 +884,8 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,
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() {
Expand Down
107 changes: 107 additions & 0 deletions shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2790,4 +2790,111 @@ - (BOOL)validateOneVisualEffectView:(UIView*)visualEffectView
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