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

Migrate FlutterView, FlutterPlatformViews, FlutterOverlayView to ARC #52535

Merged
merged 3 commits into from
May 10, 2024
Merged

Migrate FlutterView, FlutterPlatformViews, FlutterOverlayView to ARC #52535

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
4885b63
Migrate FlutterView, FlutterPlatformViews, FlutterOverlayView to ARC
jmagman Apr 19, 2024
f089c3b
clang-tidy
jmagman May 7, 2024
baa72bc
Rebase with https://github.com/flutter/engine/pull/52484
jmagman May 10, 2024
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
15 changes: 8 additions & 7 deletions shell/platform/darwin/ios/BUILD.gn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,11 @@ source_set("flutter_framework_source_arc") {
"framework/Source/FlutterKeyboardManager.mm",
"framework/Source/FlutterMetalLayer.h",
"framework/Source/FlutterMetalLayer.mm",
"framework/Source/FlutterOverlayView.h",
"framework/Source/FlutterOverlayView.mm",
"framework/Source/FlutterPlatformViews.mm",
"framework/Source/FlutterPlatformViews_Internal.h",
"framework/Source/FlutterPlatformViews_Internal.mm",
"framework/Source/FlutterRestorationPlugin.h",
"framework/Source/FlutterRestorationPlugin.mm",
"framework/Source/FlutterSpellCheckPlugin.h",
Expand All @@ -87,6 +92,9 @@ source_set("flutter_framework_source_arc") {
"framework/Source/FlutterUndoManagerDelegate.h",
"framework/Source/FlutterUndoManagerPlugin.h",
"framework/Source/FlutterUndoManagerPlugin.mm",
"framework/Source/FlutterView.h",
"framework/Source/FlutterView.mm",
"framework/Source/FlutterViewResponder.h",
Copy link
Member Author

@jmagman jmagman May 2, 2024

Choose a reason for hiding this comment

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

FlutterViewResponder.h wasn't included anywhere?

"framework/Source/KeyCodeMap.g.mm",
"framework/Source/KeyCodeMap_Internal.h",
"framework/Source/UIViewController+FlutterScreenAndSceneIfLoaded.h",
Expand Down Expand Up @@ -160,18 +168,11 @@ source_set("flutter_framework_source") {
"framework/Source/FlutterEngineGroup.mm",
"framework/Source/FlutterEngine_Internal.h",
"framework/Source/FlutterHeadlessDartRunner.mm",
"framework/Source/FlutterOverlayView.h",
"framework/Source/FlutterOverlayView.mm",
"framework/Source/FlutterPlatformPlugin.h",
"framework/Source/FlutterPlatformPlugin.mm",
"framework/Source/FlutterPlatformViews.mm",
"framework/Source/FlutterPlatformViews_Internal.h",
"framework/Source/FlutterPlatformViews_Internal.mm",
"framework/Source/FlutterPluginAppLifeCycleDelegate.mm",
"framework/Source/FlutterSemanticsScrollView.h",
"framework/Source/FlutterSemanticsScrollView.mm",
"framework/Source/FlutterView.h",
"framework/Source/FlutterView.mm",
"framework/Source/FlutterViewController.mm",
"framework/Source/FlutterViewController_Internal.h",
"framework/Source/SemanticsObject.h",
Expand Down
2 changes: 2 additions & 0 deletions shell/platform/darwin/ios/framework/Source/FlutterOverlayView.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterView.h"
#include "fml/platform/darwin/cf_utils.h"

FLUTTER_ASSERT_ARC

// This is mostly a duplication of FlutterView.
// TODO(amirh): once GL support is in evaluate if we can merge this with FlutterView.
@implementation FlutterOverlayView {
Expand Down
110 changes: 50 additions & 60 deletions 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 @@ -11,6 +11,8 @@
#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterView.h"
#import "flutter/shell/platform/darwin/ios/ios_surface.h"

FLUTTER_ASSERT_ARC

@implementation UIView (FirstResponder)
- (BOOL)flt_hasFirstResponderInViewHierarchySubtree {
if (self.isFirstResponder) {
Expand Down Expand Up @@ -91,7 +93,7 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,
overlay_view.reset([[FlutterOverlayView alloc] init]);
overlay_view_wrapper.reset([[FlutterOverlayView alloc] init]);

auto ca_layer = fml::scoped_nsobject<CALayer>{[[overlay_view.get() layer] retain]};
auto ca_layer = fml::scoped_nsobject<CALayer>{[overlay_view.get() layer]};
std::unique_ptr<IOSSurface> ios_surface = IOSSurface::Create(ios_context, ca_layer);
std::unique_ptr<Surface> surface = ios_surface->CreateGPUSurface();

Expand All @@ -105,7 +107,7 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,
overlay_view_wrapper.reset([[FlutterOverlayView alloc] initWithContentsScale:screenScale
pixelFormat:pixel_format]);

auto ca_layer = fml::scoped_nsobject<CALayer>{[[overlay_view.get() layer] retain]};
auto ca_layer = fml::scoped_nsobject<CALayer>{[overlay_view.get() layer]};
std::unique_ptr<IOSSurface> ios_surface = IOSSurface::Create(ios_context, ca_layer);
std::unique_ptr<Surface> surface = ios_surface->CreateGPUSurface(gr_context);

Expand Down Expand Up @@ -159,12 +161,12 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,
}

void FlutterPlatformViewsController::SetFlutterView(UIView* flutter_view) {
flutter_view_.reset([flutter_view retain]);
flutter_view_.reset(flutter_view);
}

void FlutterPlatformViewsController::SetFlutterViewController(
UIViewController<FlutterViewResponder>* flutter_view_controller) {
flutter_view_controller_.reset([flutter_view_controller retain]);
flutter_view_controller_.reset(flutter_view_controller);
}

UIViewController<FlutterViewResponder>* FlutterPlatformViewsController::getFlutterViewController() {
Expand Down Expand Up @@ -231,21 +233,19 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,
// Set a unique view identifier, so the platform view can be identified in unit tests.
platform_view.accessibilityIdentifier =
[NSString stringWithFormat:@"platform_view[%lld]", viewId];
views_[viewId] = fml::scoped_nsobject<NSObject<FlutterPlatformView>>([embedded_view retain]);
views_[viewId] = fml::scoped_nsobject<NSObject<FlutterPlatformView>>(embedded_view);

FlutterTouchInterceptingView* touch_interceptor = [[[FlutterTouchInterceptingView alloc]
FlutterTouchInterceptingView* touch_interceptor = [[FlutterTouchInterceptingView alloc]
initWithEmbeddedView:platform_view
platformViewsController:GetWeakPtr()
gestureRecognizersBlockingPolicy:gesture_recognizers_blocking_policies_[viewType]]
autorelease];
gestureRecognizersBlockingPolicy:gesture_recognizers_blocking_policies_[viewType]];

touch_interceptors_[viewId] =
fml::scoped_nsobject<FlutterTouchInterceptingView>([touch_interceptor retain]);
fml::scoped_nsobject<FlutterTouchInterceptingView>(touch_interceptor);

ChildClippingView* clipping_view =
[[[ChildClippingView alloc] initWithFrame:CGRectZero] autorelease];
ChildClippingView* clipping_view = [[ChildClippingView alloc] initWithFrame:CGRectZero];
[clipping_view addSubview:touch_interceptor];
root_views_[viewId] = fml::scoped_nsobject<UIView>([clipping_view retain]);
root_views_[viewId] = fml::scoped_nsobject<UIView>(clipping_view);

result(nil);
}
Expand Down Expand Up @@ -307,8 +307,7 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,
FlutterPlatformViewGestureRecognizersBlockingPolicy gestureRecognizerBlockingPolicy) {
std::string idString([factoryId UTF8String]);
FML_CHECK(factories_.count(idString) == 0);
factories_[idString] =
fml::scoped_nsobject<NSObject<FlutterPlatformViewFactory>>([factory retain]);
factories_[idString] = fml::scoped_nsobject<NSObject<FlutterPlatformViewFactory>>(factory);
gesture_recognizers_blocking_policies_[idString] = gestureRecognizerBlockingPolicy;
}

Expand Down Expand Up @@ -417,7 +416,7 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,

long FlutterPlatformViewsController::FindFirstResponderPlatformViewId() {
for (auto const& [id, root_view] : root_views_) {
if ((UIView*)(root_view.get()).flt_hasFirstResponderInViewHierarchySubtree) {
if (((UIView*)root_view.get()).flt_hasFirstResponderInViewHierarchySubtree) {
Copy link
Member Author

@jmagman jmagman May 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

error: cast of 'BOOL' (aka 'bool') to 'UIView *' is disallowed with ARC

../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm:419:9: error: cast of 'BOOL' (aka 'bool') to 'UIView *' is disallowed with ARC
  419 |     if ((UIView*)(root_view.get()).flt_hasFirstResponderInViewHierarchySubtree) {
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

This was somehow working before?

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

Lines 2771 to 2775 in ea83156

XCTAssertTrue(textField.isFirstResponder);
XCTAssertTrue(textField.flt_hasFirstResponderInViewHierarchySubtree);
[textField resignFirstResponder];
XCTAssertFalse(textField.isFirstResponder);
XCTAssertFalse(textField.flt_hasFirstResponderInViewHierarchySubtree);

#33093

Copy link
Contributor

@hellohuanlin hellohuanlin May 10, 2024

Choose a reason for hiding this comment

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

weird

return id;
}
}
Expand Down Expand Up @@ -460,7 +459,7 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,
ChildClippingView* clipView = (ChildClippingView*)embedded_view.superview;

SkMatrix transformMatrix;
NSMutableArray* blurFilters = [[[NSMutableArray alloc] init] autorelease];
NSMutableArray* blurFilters = [[NSMutableArray alloc] init];
FML_DCHECK(!clipView.maskView ||
[clipView.maskView isKindOfClass:[FlutterClippingMaskView class]]);
if (clipView.maskView) {
Expand Down Expand Up @@ -530,12 +529,11 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,
// is not supported in Quartz's gaussianBlur CAFilter, so it is not used
// to blur the PlatformView.
CGFloat blurRadius = (*iter)->GetFilterMutation().GetFilter().asBlur()->sigma_x();
UIVisualEffectView* visualEffectView = [[[UIVisualEffectView alloc]
initWithEffect:[UIBlurEffect effectWithStyle:UIBlurEffectStyleLight]] autorelease];
PlatformViewFilter* filter =
[[[PlatformViewFilter alloc] initWithFrame:frameInClipView
blurRadius:blurRadius
visualEffectView:visualEffectView] autorelease];
UIVisualEffectView* visualEffectView = [[UIVisualEffectView alloc]
initWithEffect:[UIBlurEffect effectWithStyle:UIBlurEffectStyleLight]];
PlatformViewFilter* filter = [[PlatformViewFilter alloc] initWithFrame:frameInClipView
blurRadius:blurRadius
visualEffectView:visualEffectView];
if (!filter) {
canApplyBlurBackdrop = NO;
} else {
Expand Down Expand Up @@ -950,11 +948,13 @@ @interface DelayingGestureRecognizer : UIGestureRecognizer <UIGestureRecognizerD

// Indicates that if the `DelayingGestureRecognizer`'s state should be set to
// `UIGestureRecognizerStateEnded` during next `touchesEnded` call.
@property(nonatomic) bool shouldEndInNextTouchesEnded;
@property(nonatomic) BOOL shouldEndInNextTouchesEnded;

// Indicates that the `DelayingGestureRecognizer`'s `touchesEnded` has been invoked without
// setting the state to `UIGestureRecognizerStateEnded`.
@property(nonatomic) bool touchedEndedWithoutBlocking;
@property(nonatomic) BOOL touchedEndedWithoutBlocking;

@property(nonatomic, readonly) UIGestureRecognizer* forwardingRecognizer;
Copy link
Member Author

@jmagman jmagman May 10, 2024

Choose a reason for hiding this comment

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

Swapping from scoped_nsobject to a property.

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

Lines 1073 to 1075 in ea83156

@implementation DelayingGestureRecognizer {
fml::scoped_nsobject<UIGestureRecognizer> _forwardingRecognizer;
}


- (instancetype)initWithTarget:(id)target
action:(SEL)action
Expand All @@ -977,11 +977,13 @@ - (instancetype)initWithTarget:(id)target
(fml::WeakPtr<flutter::FlutterPlatformViewsController>)platformViewsController;
@end

@implementation FlutterTouchInterceptingView {
fml::scoped_nsobject<DelayingGestureRecognizer> _delayingRecognizer;
FlutterPlatformViewGestureRecognizersBlockingPolicy _blockingPolicy;
UIView* _embeddedView;
}
@interface FlutterTouchInterceptingView ()
@property(nonatomic, weak, readonly) UIView* embeddedView;
Copy link
Member Author

@jmagman jmagman May 10, 2024

Choose a reason for hiding this comment

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

Not retained:

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

Line 993 in ea83156

_embeddedView = embeddedView;

@property(nonatomic, readonly) DelayingGestureRecognizer* delayingRecognizer;
Copy link
Member Author

@jmagman jmagman May 10, 2024

Choose a reason for hiding this comment

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

Swapping from scoped_nsobject to a property.

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

Line 981 in ea83156

fml::scoped_nsobject<DelayingGestureRecognizer> _delayingRecognizer;

@property(nonatomic, readonly) FlutterPlatformViewGestureRecognizersBlockingPolicy blockingPolicy;
@end

@implementation FlutterTouchInterceptingView
- (instancetype)initWithEmbeddedView:(UIView*)embeddedView
platformViewsController:
(fml::WeakPtr<flutter::FlutterPlatformViewsController>)platformViewsController
Expand All @@ -996,47 +998,42 @@ - (instancetype)initWithEmbeddedView:(UIView*)embeddedView

[self addSubview:embeddedView];

ForwardingGestureRecognizer* forwardingRecognizer = [[[ForwardingGestureRecognizer alloc]
initWithTarget:self
platformViewsController:std::move(platformViewsController)] autorelease];
ForwardingGestureRecognizer* forwardingRecognizer =
[[ForwardingGestureRecognizer alloc] initWithTarget:self
platformViewsController:platformViewsController];

_delayingRecognizer.reset([[DelayingGestureRecognizer alloc]
initWithTarget:self
action:nil
forwardingRecognizer:forwardingRecognizer]);
_delayingRecognizer = [[DelayingGestureRecognizer alloc] initWithTarget:self
action:nil
forwardingRecognizer:forwardingRecognizer];
_blockingPolicy = blockingPolicy;

[self addGestureRecognizer:_delayingRecognizer.get()];
[self addGestureRecognizer:_delayingRecognizer];
[self addGestureRecognizer:forwardingRecognizer];
}
return self;
}

- (UIView*)embeddedView {
return [[_embeddedView retain] autorelease];
}

- (void)releaseGesture {
_delayingRecognizer.get().state = UIGestureRecognizerStateFailed;
self.delayingRecognizer.state = UIGestureRecognizerStateFailed;
}

- (void)blockGesture {
switch (_blockingPolicy) {
case FlutterPlatformViewGestureRecognizersBlockingPolicyEager:
// We block all other gesture recognizers immediately in this policy.
_delayingRecognizer.get().state = UIGestureRecognizerStateEnded;
self.delayingRecognizer.state = UIGestureRecognizerStateEnded;
break;
case FlutterPlatformViewGestureRecognizersBlockingPolicyWaitUntilTouchesEnded:
if (_delayingRecognizer.get().touchedEndedWithoutBlocking) {
if (self.delayingRecognizer.touchedEndedWithoutBlocking) {
// If touchesEnded of the `DelayingGesureRecognizer` has been already invoked,
// we want to set the state of the `DelayingGesureRecognizer` to
// `UIGestureRecognizerStateEnded` as soon as possible.
_delayingRecognizer.get().state = UIGestureRecognizerStateEnded;
self.delayingRecognizer.state = UIGestureRecognizerStateEnded;
} else {
// If touchesEnded of the `DelayingGesureRecognizer` has not been invoked,
// We will set a flag to notify the `DelayingGesureRecognizer` to set the state to
// `UIGestureRecognizerStateEnded` when touchesEnded is called.
_delayingRecognizer.get().shouldEndInNextTouchesEnded = YES;
self.delayingRecognizer.shouldEndInNextTouchesEnded = YES;
}
break;
default:
Expand All @@ -1060,19 +1057,12 @@ - (void)touchesEnded:(NSSet*)touches withEvent:(UIEvent*)event {
}

- (id)accessibilityContainer {
return _flutterAccessibilityContainer;
}

- (void)dealloc {
[_flutterAccessibilityContainer release];
[super dealloc];
return self.flutterAccessibilityContainer;
Copy link
Member Author

@jmagman jmagman May 10, 2024

Choose a reason for hiding this comment

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

@tacck can you double-check me here re: #52484 ?

Copy link
Contributor

@tacck tacck May 10, 2024

Choose a reason for hiding this comment

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

I think it's LGTM!

}

@end

@implementation DelayingGestureRecognizer {
fml::scoped_nsobject<UIGestureRecognizer> _forwardingRecognizer;
}
@implementation DelayingGestureRecognizer

- (instancetype)initWithTarget:(id)target
action:(SEL)action
Expand All @@ -1082,9 +1072,9 @@ - (instancetype)initWithTarget:(id)target
self.delaysTouchesBegan = YES;
self.delaysTouchesEnded = YES;
self.delegate = self;
self.shouldEndInNextTouchesEnded = NO;
self.touchedEndedWithoutBlocking = NO;
_forwardingRecognizer.reset([forwardingRecognizer retain]);
_shouldEndInNextTouchesEnded = NO;
_touchedEndedWithoutBlocking = NO;
_forwardingRecognizer = forwardingRecognizer;
}
return self;
}
Expand All @@ -1093,7 +1083,7 @@ - (BOOL)gestureRecognizer:(UIGestureRecognizer*)gestureRecognizer
shouldBeRequiredToFailByGestureRecognizer:(UIGestureRecognizer*)otherGestureRecognizer {
// The forwarding gesture recognizer should always get all touch events, so it should not be
// required to fail by any other gesture recognizer.
return otherGestureRecognizer != _forwardingRecognizer.get() && otherGestureRecognizer != self;
return otherGestureRecognizer != _forwardingRecognizer && otherGestureRecognizer != self;
}

- (BOOL)gestureRecognizer:(UIGestureRecognizer*)gestureRecognizer
Expand Down Expand Up @@ -1158,7 +1148,7 @@ - (void)touchesBegan:(NSSet*)touches withEvent:(UIEvent*)event {
// At the start of each gesture sequence, we reset the `_flutterViewController`,
// so that all the touch events in the same sequence are forwarded to the same
// `_flutterViewController`.
_flutterViewController.reset([_platformViewsController->getFlutterViewController() retain]);
_flutterViewController.reset(_platformViewsController->getFlutterViewController());
}
[_flutterViewController.get() touchesBegan:touches withEvent:event];
_currentTouchPointersCount += touches.count;
Expand Down
Loading