Skip to content

Commit

Permalink
Reland: iOS: Migrate PlatformViewsController to Objective-C (flutter#…
Browse files Browse the repository at this point in the history
…56828)

This migrates PlatformViewController from C++ to Objective-C. Generally, we try to keep the embedder interfaces and components written in Objective-C except for the few places where C++ interfaces are requried to interface with engine APIs such as Shell and PlatformView (e.g. the PlatformViewIOS subclass). Now that the implementation is Objective-C, the class and file are renamed to match Objective-C naming conventions.

This allows us to take advantage of ARC and weak references, which eliminates the need for std::shared_ptr, fml::WeakPtr etc. Further, this eliminates some particularly unintuitive behaviour wherein this class was owned via a std::shared_ptr held by FlutterEngine, and injected into many other classes (e.g. AccessibilityBridge) via a std::shared_ptr& reference -- such that only one instance of the std::shared_ptr actually ever existed, presumably to avoid std::shared_ptr refcounting overhead. Given that this overhead was only incurred a single time at engine initialisation, this seems like overkill. One might ask why it wasn't therefore held in a `std::unique_ptr` and a `std::unique_ptr&` reference passed around. Likely, this was because we wanted to take a `fml::WeakPtr` reference on it.

Regardless, none of this is necessary any longer now that we can inject `__weak FlutterPlatformViewsController*` instances to classes that use it.

To be clear, this patch makes no attempt whatsoever to simplify or clean up the interface or implementation of this class. This class ties together far too many concepts and is injected into far too many places, and we should break it up and simplify it. However, the goal of this patch was simply to port to an Objective-C interface that plays nicely with the rest of the iOS embedder. This does include a couple minor cleanups in `#include`/`#import` order and usage to match our style guide.

This is a reland with a one-line fix of a lambda-capture block to ensure `self` and any local variables are captured by value rather than by reference:
* In the case where this method is called on the platform thread (i.e. where the UI and platform thread are merged), we use the latch to pause the calling thread until the lambda completes, in which case all locals could be passed by reference since the locals are guaranteed to hang around until the lambda completes and signals the latch.
* In the case where this method is called from the UI thread (i.e. where UI and platform thread are not merged), locals may have gone out of scope by the time the lambda executes, leading to undefined behaviour if passed by reference; thus we always pass by value to be sure; since `latch` must be shared between threads, it's passed held in a `std::shared_ptr` so the underlying latch/mutex is shared but it's kept live until it goes out of scope in both threads.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
  • Loading branch information
cbracken authored Nov 27, 2024
1 parent 10ba734 commit b9474a9
Show file tree
Hide file tree
Showing 24 changed files with 2,294 additions and 1,974 deletions.
8 changes: 4 additions & 4 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -44571,6 +44571,8 @@ ORIGIN: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatf
ORIGIN: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformPlugin.mm + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformPluginTest.mm + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsController.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsController.mm + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPluginAppLifeCycleDelegate.mm + ../../../flutter/LICENSE
Expand Down Expand Up @@ -44632,8 +44634,6 @@ ORIGIN: ../../../flutter/shell/platform/darwin/ios/framework/Source/overlay_laye
ORIGIN: ../../../flutter/shell/platform/darwin/ios/framework/Source/overlay_layer_pool.mm + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/darwin/ios/framework/Source/platform_message_response_darwin.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/darwin/ios/framework/Source/platform_message_response_darwin.mm + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/darwin/ios/framework/Source/platform_views_controller.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/darwin/ios/framework/Source/platform_views_controller.mm + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/darwin/ios/framework/Source/profiler_metrics_ios.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/darwin/ios/framework/Source/profiler_metrics_ios.mm + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/darwin/ios/framework/Source/vsync_waiter_ios.h + ../../../flutter/LICENSE
Expand Down Expand Up @@ -47514,6 +47514,8 @@ FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatfor
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformPlugin.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformPluginTest.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsController.h
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsController.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPluginAppLifeCycleDelegate.mm
Expand Down Expand Up @@ -47575,8 +47577,6 @@ FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/overlay_layer_
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/overlay_layer_pool.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/platform_message_response_darwin.h
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/platform_message_response_darwin.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/platform_views_controller.h
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/platform_views_controller.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/profiler_metrics_ios.h
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/profiler_metrics_ios.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/vsync_waiter_ios.h
Expand Down
4 changes: 2 additions & 2 deletions shell/platform/darwin/ios/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ source_set("flutter_framework_source") {
"framework/Source/FlutterPlatformPlugin.h",
"framework/Source/FlutterPlatformPlugin.mm",
"framework/Source/FlutterPlatformViews.mm",
"framework/Source/FlutterPlatformViewsController.h",
"framework/Source/FlutterPlatformViewsController.mm",
"framework/Source/FlutterPlatformViews_Internal.h",
"framework/Source/FlutterPluginAppLifeCycleDelegate.mm",
"framework/Source/FlutterRestorationPlugin.h",
Expand Down Expand Up @@ -120,8 +122,6 @@ source_set("flutter_framework_source") {
"framework/Source/overlay_layer_pool.mm",
"framework/Source/platform_message_response_darwin.h",
"framework/Source/platform_message_response_darwin.mm",
"framework/Source/platform_views_controller.h",
"framework/Source/platform_views_controller.mm",
"framework/Source/profiler_metrics_ios.h",
"framework/Source/profiler_metrics_ios.mm",
"framework/Source/vsync_waiter_ios.h",
Expand Down
39 changes: 18 additions & 21 deletions shell/platform/darwin/ios/framework/Source/FlutterEngine.mm
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ @interface FlutterEngine () <FlutterIndirectScribbleDelegate,
@property(nonatomic, readonly, assign) BOOL allowHeadlessExecution;
@property(nonatomic, readonly, assign) BOOL restorationEnabled;

@property(nonatomic, strong) FlutterPlatformViewsController* platformViewsController;

// Maintains a dictionary of plugin names that have registered with the engine. Used by
// FlutterEngineRegistrar to implement a FlutterPluginRegistrar.
@property(nonatomic, readonly) NSMutableDictionary* pluginPublications;
Expand Down Expand Up @@ -150,7 +152,6 @@ @implementation FlutterEngine {
std::shared_ptr<flutter::ThreadHost> _threadHost;
std::unique_ptr<flutter::Shell> _shell;

std::shared_ptr<flutter::PlatformViewsController> _platformViewsController;
flutter::IOSRenderingAPI _renderingApi;
std::shared_ptr<flutter::SamplingProfiler> _profiler;

Expand Down Expand Up @@ -211,7 +212,7 @@ - (instancetype)initWithName:(NSString*)labelPrefix

_pluginPublications = [[NSMutableDictionary alloc] init];
_registrars = [[NSMutableDictionary alloc] init];
[self recreatePlatformViewController];
[self recreatePlatformViewsController];
_binaryMessenger = [[FlutterBinaryMessengerRelay alloc] initWithParent:self];
_textureRegistry = [[FlutterTextureRegistryRelay alloc] initWithParent:self];
_connections.reset(new flutter::ConnectionCollection());
Expand Down Expand Up @@ -262,9 +263,9 @@ - (void)setUpApplicationLifecycleNotifications:(NSNotificationCenter*)center {
object:nil];
}

- (void)recreatePlatformViewController {
- (void)recreatePlatformViewsController {
_renderingApi = flutter::GetRenderingAPIForProcess(FlutterView.forceSoftwareRendering);
_platformViewsController.reset(new flutter::PlatformViewsController());
_platformViewsController = [[FlutterPlatformViewsController alloc] init];
}

- (flutter::IOSRenderingAPI)platformViewsRenderingAPI {
Expand Down Expand Up @@ -452,11 +453,7 @@ - (void)destroyContext {
_shell.reset();
_profiler.reset();
_threadHost.reset();
_platformViewsController.reset();
}

- (std::shared_ptr<flutter::PlatformViewsController>&)platformViewsController {
return _platformViewsController;
_platformViewsController = nil;
}

- (NSURL*)observatoryUrl {
Expand Down Expand Up @@ -635,7 +632,7 @@ - (void)maybeSetupPlatformViewChannels {
[self.platformViewsChannel
setMethodCallHandler:^(FlutterMethodCall* call, FlutterResult result) {
if (weakSelf) {
weakSelf.platformViewsController->OnMethodCall(call, result);
[weakSelf.platformViewsController onMethodCall:call result:result];
}
}];

Expand Down Expand Up @@ -777,11 +774,11 @@ - (BOOL)createShell:(NSString*)entrypoint
if (!strongSelf) {
return std::unique_ptr<flutter::PlatformViewIOS>();
}
[strongSelf recreatePlatformViewController];
strongSelf->_platformViewsController->SetTaskRunner(
shell.GetTaskRunners().GetPlatformTaskRunner());
[strongSelf recreatePlatformViewsController];
strongSelf.platformViewsController.taskRunner =
shell.GetTaskRunners().GetPlatformTaskRunner();
return std::make_unique<flutter::PlatformViewIOS>(
shell, strongSelf->_renderingApi, strongSelf->_platformViewsController,
shell, strongSelf->_renderingApi, strongSelf.platformViewsController,
shell.GetTaskRunners(), shell.GetConcurrentWorkerTaskRunner(),
shell.GetIsGpuDisabledSyncSwitch());
};
Expand Down Expand Up @@ -1103,7 +1100,7 @@ - (void)flutterTextInputView:(FlutterTextInputView*)textInputView
// Have to check in the next run loop, because iOS requests the previous first responder to
// resign before requesting the next view to become first responder.
dispatch_async(dispatch_get_main_queue(), ^(void) {
long platform_view_id = self.platformViewsController->FindFirstResponderPlatformViewId();
long platform_view_id = [self.platformViewsController firstResponderPlatformViewId];
if (platform_view_id == -1) {
return;
}
Expand Down Expand Up @@ -1400,11 +1397,10 @@ - (FlutterEngine*)spawnWithEntrypoint:(/*nullable*/ NSString*)entrypoint
// create call is synchronous.
flutter::Shell::CreateCallback<flutter::PlatformView> on_create_platform_view =
[result, context](flutter::Shell& shell) {
[result recreatePlatformViewController];
result->_platformViewsController->SetTaskRunner(
shell.GetTaskRunners().GetPlatformTaskRunner());
[result recreatePlatformViewsController];
result.platformViewsController.taskRunner = shell.GetTaskRunners().GetPlatformTaskRunner();
return std::make_unique<flutter::PlatformViewIOS>(
shell, context, result->_platformViewsController, shell.GetTaskRunners());
shell, context, result.platformViewsController, shell.GetTaskRunners());
};

flutter::Shell::CreateCallback<flutter::Rasterizer> on_create_rasterizer =
Expand Down Expand Up @@ -1499,8 +1495,9 @@ - (void)registerViewFactory:(NSObject<FlutterPlatformViewFactory>*)factory
withId:(NSString*)factoryId
gestureRecognizersBlockingPolicy:
(FlutterPlatformViewGestureRecognizersBlockingPolicy)gestureRecognizersBlockingPolicy {
[_flutterEngine platformViewsController]->RegisterViewFactory(factory, factoryId,
gestureRecognizersBlockingPolicy);
[_flutterEngine.platformViewsController registerViewFactory:factory
withId:factoryId
gestureRecognizersBlockingPolicy:gestureRecognizersBlockingPolicy];
}

@end
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ NS_ASSUME_NONNULL_BEGIN
base64Encode:(bool)base64Encode;

- (FlutterPlatformPlugin*)platformPlugin;
- (std::shared_ptr<flutter::PlatformViewsController>&)platformViewsController;
- (FlutterTextInputPlugin*)textInputPlugin;
- (FlutterRestorationPlugin*)restorationPlugin;
- (void)launchEngine:(nullable NSString*)entrypoint
Expand Down Expand Up @@ -81,6 +80,7 @@ NS_ASSUME_NONNULL_BEGIN
userData:(nullable void*)userData;

@property(nonatomic, readonly) FlutterDartProject* project;

@end

NS_ASSUME_NONNULL_END
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -518,8 +518,7 @@ @interface FlutterTouchInterceptingView ()

@implementation FlutterTouchInterceptingView
- (instancetype)initWithEmbeddedView:(UIView*)embeddedView
platformViewsController:
(fml::WeakPtr<flutter::PlatformViewsController>)platformViewsController
platformViewsController:(FlutterPlatformViewsController*)platformViewsController
gestureRecognizersBlockingPolicy:
(FlutterPlatformViewGestureRecognizersBlockingPolicy)blockingPolicy {
self = [super initWithFrame:embeddedView.frame];
Expand Down Expand Up @@ -667,7 +666,7 @@ @implementation ForwardingGestureRecognizer {
// outlives the FlutterViewController. And ForwardingGestureRecognizer is owned by a subview of
// FlutterView, so the ForwardingGestureRecognizer never out lives FlutterViewController.
// Therefore, `_platformViewsController` should never be nullptr.
fml::WeakPtr<flutter::PlatformViewsController> _platformViewsController;
__weak FlutterPlatformViewsController* _platformViewsController;
// Counting the pointers that has started in one touch sequence.
NSInteger _currentTouchPointersCount;
// We can't dispatch events to the framework without this back pointer.
Expand All @@ -678,21 +677,20 @@ @implementation ForwardingGestureRecognizer {
}

- (instancetype)initWithTarget:(id)target
platformViewsController:
(fml::WeakPtr<flutter::PlatformViewsController>)platformViewsController {
platformViewsController:(FlutterPlatformViewsController*)platformViewsController {
self = [super initWithTarget:target action:nil];
if (self) {
self.delegate = self;
FML_DCHECK(platformViewsController.get() != nullptr);
_platformViewsController = std::move(platformViewsController);
FML_DCHECK(platformViewsController);
_platformViewsController = platformViewsController;
_currentTouchPointersCount = 0;
}
return self;
}

- (ForwardingGestureRecognizer*)recreateRecognizerWithTarget:(id)target {
return [[ForwardingGestureRecognizer alloc] initWithTarget:target
platformViewsController:std::move(_platformViewsController)];
platformViewsController:_platformViewsController];
}

- (void)touchesBegan:(NSSet*)touches withEvent:(UIEvent*)event {
Expand All @@ -701,7 +699,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 = _platformViewsController->GetFlutterViewController();
_flutterViewController = _platformViewsController.flutterViewController;
}
[_flutterViewController touchesBegan:touches withEvent:event];
_currentTouchPointersCount += touches.count;
Expand Down
Loading

0 comments on commit b9474a9

Please sign in to comment.