From 2eaeffe9c38a9166745694d9f3857cd17e08b24b Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Thu, 20 Apr 2023 10:34:24 -0700 Subject: [PATCH 1/3] # This is a combination of 3 commits. # This is the 1st commit message: fix remove from slice test # This is the commit message #2: remove comment # This is the commit message #3: dispose view after it is removed from the compositon order --- .../framework/Source/FlutterPlatformViews.mm | 11 +- .../Source/FlutterPlatformViewsTest.mm | 107 ++++++++++++++++++ 2 files changed, 117 insertions(+), 1 deletion(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index 5978c1137a4b0..a0cf37a0fbca2 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -867,7 +867,14 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect, FML_DCHECK([[NSThread currentThread] isMainThread]); + std::unordered_set views_to_composite(composition_order_.begin(), + composition_order_.end()); + std::unordered_set views_to_dispose_next_frame; for (int64_t viewId : views_to_dispose_) { + if (views_to_composite.count(viewId)) { + views_to_dispose_next_frame.insert(viewId); + continue; + } UIView* root_view = root_views_[viewId].get(); [root_view removeFromSuperview]; views_.erase(viewId); @@ -876,8 +883,10 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect, current_composition_params_.erase(viewId); clip_count_.erase(viewId); views_to_recomposite_.erase(viewId); + slices_.erase(viewId); } - views_to_dispose_.clear(); + + views_to_dispose_ = views_to_dispose_next_frame; } void FlutterPlatformViewsController::BeginCATransaction() { diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm index b3522ec48a21e..97285cfcc78be 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm @@ -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(); + auto platform_view = std::make_unique( + /*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(finalMatrix, SkSize::Make(300, 300), stack); + flutterPlatformViewsController->PrerollCompositeEmbeddedView(0, std::move(embeddedViewParams0)); + flutterPlatformViewsController->CompositeEmbeddedView(0); + + auto embeddedViewParams1 = + std::make_unique(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 mock_sk_surface = SkSurface::MakeRaster(image_info); + flutter::SurfaceFrame::FramebufferInfo framebuffer_info; + auto mock_surface = std::make_unique( + 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(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 mock_sk_surface = SkSurface::MakeRaster(image_info); + flutter::SurfaceFrame::FramebufferInfo framebuffer_info; + auto mock_surface = std::make_unique( + 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 From a113c4b3930642cf1ac89eb5d145e345a8e34b92 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Thu, 27 Apr 2023 22:40:55 -0700 Subject: [PATCH 2/3] std move --- .../darwin/ios/framework/Source/FlutterPlatformViews.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index a0cf37a0fbca2..e45c095b47962 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -886,7 +886,7 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect, slices_.erase(viewId); } - views_to_dispose_ = views_to_dispose_next_frame; + views_to_dispose_ = std::move(views_to_dispose_next_frame); } void FlutterPlatformViewsController::BeginCATransaction() { From c8aa713cc7ffdd137c7e622bbbc8dfbffccb2248 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Fri, 28 Apr 2023 10:34:13 -0700 Subject: [PATCH 3/3] renamefix --- .../darwin/ios/framework/Source/FlutterPlatformViews.mm | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index e45c095b47962..1a85f14119060 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -869,10 +869,10 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect, std::unordered_set views_to_composite(composition_order_.begin(), composition_order_.end()); - std::unordered_set views_to_dispose_next_frame; + std::unordered_set views_to_delay_dispose; for (int64_t viewId : views_to_dispose_) { if (views_to_composite.count(viewId)) { - views_to_dispose_next_frame.insert(viewId); + views_to_delay_dispose.insert(viewId); continue; } UIView* root_view = root_views_[viewId].get(); @@ -883,10 +883,9 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect, current_composition_params_.erase(viewId); clip_count_.erase(viewId); views_to_recomposite_.erase(viewId); - slices_.erase(viewId); } - views_to_dispose_ = std::move(views_to_dispose_next_frame); + views_to_dispose_ = std::move(views_to_delay_dispose); } void FlutterPlatformViewsController::BeginCATransaction() {