Revert "[ios_platform_view] only recycle maskView when the view is applying mutators" (#42080)

Reverts #41573

Crashing on Framework CI

Author: zanderso

shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm

@@ -18,6 +18,8 @@
 #import "flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController_Internal.h"
 #import "flutter/shell/platform/darwin/ios/ios_surface.h"
 
+static const NSUInteger kFlutterClippingMaskViewPoolCapacity = 5;
+
 @implementation UIView (FirstResponder)
 - (BOOL)flt_hasFirstResponderInViewHierarchySubtree {
   if (self.isFirstResponder) {
@@ -445,8 +447,6 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,
   clipView.maskView = [mask_view_pool_.get() getMaskViewWithFrame:frame];
 }
 
-// This method is only called when the `embedded_view` needs to be re-composited at the current
-// frame. See: `CompositeWithParams` for details.
 void FlutterPlatformViewsController::ApplyMutators(const MutatorsStack& mutators_stack,
                                                    UIView* embedded_view,
                                                    const SkRect& bounding_rect) {
@@ -461,10 +461,12 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,
   NSMutableArray* blurFilters = [[[NSMutableArray alloc] init] autorelease];
   FML_DCHECK(!clipView.maskView ||
              [clipView.maskView isKindOfClass:[FlutterClippingMaskView class]]);
-  if (clipView.maskView) {
-    [mask_view_pool_.get() insertViewToPoolIfNeeded:(FlutterClippingMaskView*)(clipView.maskView)];
-    clipView.maskView = nil;
+  if (mask_view_pool_.get() == nil) {
+    mask_view_pool_.reset([[FlutterClippingMaskViewPool alloc]
+        initWithCapacity:kFlutterClippingMaskViewPoolCapacity]);
   }
+  [mask_view_pool_.get() recycleMaskViews];
+  clipView.maskView = nil;
   CGFloat screenScale = [UIScreen mainScreen].scale;
   auto iter = mutators_stack.Begin();
   while (iter != mutators_stack.End()) {
@@ -568,14 +570,6 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,
   embedded_view.layer.transform = flutter::GetCATransform3DFromSkMatrix(transformMatrix);
 }
 
-// Composite the PlatformView with `view_id`.
-//
-// Every frame, during the paint traversal of the layer tree, this method is called for all
-// the PlatformViews in `views_to_recomposite_`.
-//
-// Note that `views_to_recomposite_` does not represent all the views in the view hierarchy,
-// if a PlatformView does not change its composition parameter from last frame, it is not
-// included in the `views_to_recomposite_`.
 void FlutterPlatformViewsController::CompositeWithParams(int64_t view_id,
                                                          const EmbeddedViewParams& params) {
   CGRect frame = CGRectMake(0, 0, params.sizePoints().width(), params.sizePoints().height());

shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm

@@ -2649,8 +2649,7 @@ - (void)testFlutterClippingMaskViewPoolReuseViewsAfterRecycle {
       [[[FlutterClippingMaskViewPool alloc] initWithCapacity:2] autorelease];
   FlutterClippingMaskView* view1 = [pool getMaskViewWithFrame:CGRectZero];
   FlutterClippingMaskView* view2 = [pool getMaskViewWithFrame:CGRectZero];
-  [pool insertViewToPoolIfNeeded:view1];
-  [pool insertViewToPoolIfNeeded:view2];
+  [pool recycleMaskViews];
   CGRect newRect = CGRectMake(0, 0, 10, 10);
   FlutterClippingMaskView* view3 = [pool getMaskViewWithFrame:newRect];
   FlutterClippingMaskView* view4 = [pool getMaskViewWithFrame:newRect];
@@ -2728,17 +2727,17 @@ - (void)testClipMaskViewIsReused {
   auto embeddedViewParams1 = std::make_unique<flutter::EmbeddedViewParams>(
       screenScaleMatrix, SkSize::Make(10, 10), stack1);
 
+  flutter::MutatorsStack stack2;
+  auto embeddedViewParams2 = std::make_unique<flutter::EmbeddedViewParams>(
+      screenScaleMatrix, SkSize::Make(10, 10), stack2);
+
   flutterPlatformViewsController->PrerollCompositeEmbeddedView(1, std::move(embeddedViewParams1));
   flutterPlatformViewsController->CompositeEmbeddedView(1);
   UIView* childClippingView1 = gMockPlatformView.superview.superview;
   UIView* maskView1 = childClippingView1.maskView;
   XCTAssertNotNil(maskView1);
 
   // Composite a new frame.
-  flutterPlatformViewsController->BeginFrame(SkISize::Make(100, 100));
-  flutter::MutatorsStack stack2;
-  auto embeddedViewParams2 = std::make_unique<flutter::EmbeddedViewParams>(
-      screenScaleMatrix, SkSize::Make(10, 10), stack2);
   auto embeddedViewParams3 = std::make_unique<flutter::EmbeddedViewParams>(
       screenScaleMatrix, SkSize::Make(10, 10), stack2);
   flutterPlatformViewsController->PrerollCompositeEmbeddedView(1, std::move(embeddedViewParams3));
@@ -2764,77 +2763,6 @@ - (void)testClipMaskViewIsReused {
   XCTAssertNil(childClippingView1.maskView);
 }
 
-- (void)testDifferentClipMaskViewIsUsedForEachView {
-  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);
-
-  FlutterPlatformViewsTestMockFlutterPlatformFactory* factory =
-      [[FlutterPlatformViewsTestMockFlutterPlatformFactory new] autorelease];
-  flutterPlatformViewsController->RegisterViewFactory(
-      factory, @"MockFlutterPlatformView",
-      FlutterPlatformViewGestureRecognizersBlockingPolicyEager);
-  FlutterResult result = ^(id result) {
-  };
-
-  flutterPlatformViewsController->OnMethodCall(
-      [FlutterMethodCall
-          methodCallWithMethodName:@"create"
-                         arguments:@{@"id" : @1, @"viewType" : @"MockFlutterPlatformView"}],
-      result);
-  UIView* view1 = gMockPlatformView;
-
-  // This overwrites `gMockPlatformView` to another view.
-  flutterPlatformViewsController->OnMethodCall(
-      [FlutterMethodCall
-          methodCallWithMethodName:@"create"
-                         arguments:@{@"id" : @2, @"viewType" : @"MockFlutterPlatformView"}],
-      result);
-  UIView* view2 = gMockPlatformView;
-
-  XCTAssertNotNil(gMockPlatformView);
-  UIView* mockFlutterView = [[[UIView alloc] initWithFrame:CGRectMake(0, 0, 10, 10)] autorelease];
-  flutterPlatformViewsController->SetFlutterView(mockFlutterView);
-  // Create embedded view params
-  flutter::MutatorsStack stack1;
-  // Layer tree always pushes a screen scale factor to the stack
-  SkMatrix screenScaleMatrix =
-      SkMatrix::Scale([UIScreen mainScreen].scale, [UIScreen mainScreen].scale);
-  stack1.PushTransform(screenScaleMatrix);
-  // Push a clip rect
-  SkRect rect = SkRect::MakeXYWH(2, 2, 3, 3);
-  stack1.PushClipRect(rect);
-
-  auto embeddedViewParams1 = std::make_unique<flutter::EmbeddedViewParams>(
-      screenScaleMatrix, SkSize::Make(10, 10), stack1);
-
-  flutter::MutatorsStack stack2;
-  stack2.PushClipRect(rect);
-  auto embeddedViewParams2 = std::make_unique<flutter::EmbeddedViewParams>(
-      screenScaleMatrix, SkSize::Make(10, 10), stack2);
-
-  flutterPlatformViewsController->PrerollCompositeEmbeddedView(1, std::move(embeddedViewParams1));
-  flutterPlatformViewsController->CompositeEmbeddedView(1);
-  UIView* childClippingView1 = view1.superview.superview;
-
-  flutterPlatformViewsController->PrerollCompositeEmbeddedView(2, std::move(embeddedViewParams2));
-  flutterPlatformViewsController->CompositeEmbeddedView(2);
-  UIView* childClippingView2 = view2.superview.superview;
-  UIView* maskView1 = childClippingView1.maskView;
-  UIView* maskView2 = childClippingView2.maskView;
-  XCTAssertNotEqual(maskView1, maskView2);
-}
-
 // Return true if a correct visual effect view is found. It also implies all the validation in this
 // method passes.
 //

shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h

@@ -56,7 +56,7 @@
 // in the pool. If there are none available, a new FlutterClippingMaskView is constructed. If the
 // capacity is reached, the newly constructed FlutterClippingMaskView is not added to the pool.
 //
-// Call |insertViewToPoolIfNeeded:| to return a maskView to the pool.
+// Call |recycleMaskViews| to mark all the FlutterClippingMaskViews in the pool available.
 @interface FlutterClippingMaskViewPool : NSObject
 
 // Initialize the pool with `capacity`. When the `capacity` is reached, a FlutterClippingMaskView is
@@ -66,8 +66,8 @@
 // Reuse a maskView from the pool, or allocate a new one.
 - (FlutterClippingMaskView*)getMaskViewWithFrame:(CGRect)frame;
 
-// Insert the `maskView` into the pool.
-- (void)insertViewToPoolIfNeeded:(FlutterClippingMaskView*)maskView;
+// Mark all the maskViews available.
+- (void)recycleMaskViews;
 
 @end
 
@@ -291,20 +291,27 @@ class FlutterPlatformViewsController {
   int CountClips(const MutatorsStack& mutators_stack);
 
   void ClipViewSetMaskView(UIView* clipView);
-
   // Applies the mutators in the mutators_stack to the UIView chain that was constructed by
   // `ReconstructClipViewsChain`
   //
-  // Clips are applied to the `embedded_view`'s super view(|ChildClippingView|) using a
-  // |FlutterClippingMaskView|. Transforms are applied to `embedded_view`
+  // Clips are applied to the super view with a CALayer mask. Transforms are applied to the
+  // current view that's at the head of the chain. For example the following mutators stack [T_1,
+  // C_2, T_3, T_4, C_5, T_6] where T denotes a transform and C denotes a clip, will result in the
+  // following UIView tree:
+  //
+  // C_2 -> C_5 -> PLATFORM_VIEW
+  // (PLATFORM_VIEW is a subview of C_5 which is a subview of C_2)
+  //
+  // T_1 is applied to C_2, T_3 and T_4 are applied to C_5, and T_6 is applied to PLATFORM_VIEW.
+  //
+  // After each clip operation, we update the head to the super view of the current head.
   //
   // The `bounding_rect` is the final bounding rect of the PlatformView
   // (EmbeddedViewParams::finalBoundingRect). If a clip mutator's rect contains the final bounding
   // rect of the PlatformView, the clip mutator is not applied for performance optimization.
   void ApplyMutators(const MutatorsStack& mutators_stack,
                      UIView* embedded_view,
                      const SkRect& bounding_rect);
-
   void CompositeWithParams(int64_t view_id, const EmbeddedViewParams& params);
 
   // Allocates a new FlutterPlatformViewLayer if needed, draws the pixels within the rect from

shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.mm

@@ -9,7 +9,6 @@
 #import "flutter/shell/platform/darwin/ios/ios_surface.h"
 
 static int kMaxPointsInVerb = 4;
-static const NSUInteger kFlutterClippingMaskViewPoolCapacity = 5;
 
 namespace flutter {
 
@@ -27,10 +26,7 @@
 
 FlutterPlatformViewsController::FlutterPlatformViewsController()
     : layer_pool_(std::make_unique<FlutterPlatformViewLayerPool>()),
-      weak_factory_(std::make_unique<fml::WeakPtrFactory<FlutterPlatformViewsController>>(this)) {
-  mask_view_pool_.reset(
-      [[FlutterClippingMaskViewPool alloc] initWithCapacity:kFlutterClippingMaskViewPoolCapacity]);
-};
+      weak_factory_(std::make_unique<fml::WeakPtrFactory<FlutterPlatformViewsController>>(this)){};
 
 FlutterPlatformViewsController::~FlutterPlatformViewsController() = default;
 
@@ -462,53 +458,58 @@ @interface FlutterClippingMaskViewPool ()
 // The maximum number of `FlutterClippingMaskView` the pool can contain.
 // This prevents the pool to grow infinately and limits the maximum memory a pool can use.
 @property(assign, nonatomic) NSUInteger capacity;
-
-// The pool contains the views that are available to use.
-// The number of items in the pool must not excceds `capacity`.
-@property(retain, nonatomic) NSMutableSet<FlutterClippingMaskView*>* pool;
+@property(retain, nonatomic) NSMutableArray<FlutterClippingMaskView*>* pool;
+// The index points to the first available FlutterClippingMaskView in the `pool`.
+@property(assign, nonatomic) NSUInteger availableIndex;
 
 @end
 
 @implementation FlutterClippingMaskViewPool : NSObject
 
 - (instancetype)initWithCapacity:(NSInteger)capacity {
   if (self = [super init]) {
-    // Most of cases, there are only one PlatformView in the scene.
-    // Thus init with the capacity of 1.
-    _pool = [[NSMutableSet alloc] initWithCapacity:1];
+    _pool = [[NSMutableArray alloc] initWithCapacity:capacity];
     _capacity = capacity;
+    _availableIndex = 0;
   }
   return self;
 }
 
 - (FlutterClippingMaskView*)getMaskViewWithFrame:(CGRect)frame {
-  FML_DCHECK(self.pool.count <= self.capacity);
+  FML_DCHECK(self.availableIndex <= self.capacity);
   FlutterClippingMaskView* maskView;
-  if (self.pool.count == 0) {
-    // The pool is empty, alloc a new one.
+  if (self.availableIndex == self.capacity) {
+    // The pool is full, alloc a new one.
     maskView =
         [[[FlutterClippingMaskView alloc] initWithFrame:frame
                                             screenScale:[UIScreen mainScreen].scale] autorelease];
     return maskView;
   }
-  maskView = [self.pool anyObject];
-  maskView.frame = frame;
-  [maskView reset];
-  [self.pool removeObject:maskView];
+
+  if (self.availableIndex >= self.pool.count) {
+    // The pool doesn't have enough maskViews, alloc a new one and add to the pool.
+    maskView =
+        [[[FlutterClippingMaskView alloc] initWithFrame:frame
+                                            screenScale:[UIScreen mainScreen].scale] autorelease];
+    [self.pool addObject:maskView];
+    FML_DCHECK(self.pool.count <= self.capacity);
+  } else {
+    // Reuse a maskView from the pool.
+    maskView = [self.pool objectAtIndex:self.availableIndex];
+    maskView.frame = frame;
+    [maskView reset];
+  }
+  self.availableIndex++;
   return maskView;
 }
 
-- (void)insertViewToPoolIfNeeded:(FlutterClippingMaskView*)maskView {
-  FML_DCHECK(![self.pool containsObject:maskView]);
-  FML_DCHECK(self.pool.count <= self.capacity);
-  if (self.pool.count == self.capacity) {
-    return;
-  }
-  [self.pool addObject:maskView];
+- (void)recycleMaskViews {
+  self.availableIndex = 0;
 }
 
 - (void)dealloc {
   [_pool release];
+  _pool = nil;
 
   [super dealloc];
 }