-
Notifications
You must be signed in to change notification settings - Fork 6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
iOS: Reduce engine/view controller coupling #57151
base: main
Are you sure you want to change the base?
Conversation
Eliminates some cases where `FlutterViewController` was relying on `FlutterEngine` internals: * `[FlutterEngine shell]` * `[FlutterEngine platformView]` * `[FlutterEngine iosPlatformView]` Instead, `FlutterEngine` now exposes: * `installFirstFrameCallback:` * `enableSemantics:withFlags:` * `notifyViewCreated` * `notifyViewDestroyed` * `waitForFirstFrameSync:callback:` Also fixes a couple cases where we were relying on transitive header includes: * `FlutterAppController` relied on `FlutterViewController_Internal.h` for `sendDeepLinkToFramework:completionHandler:` This is a refactoring that introduces no semantic changes.
- (fml::WeakPtr<flutter::PlatformView>)platformView { | ||
if (!_shell) { | ||
return {}; | ||
- (void)installFirstFrameCallback:(void (^)(void))block { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here and below: moved from FlutterViewController basically verbatim.
} | ||
|
||
- (flutter::PlatformViewIOS*)iosPlatformView { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eliminated this. Now we just use platformView
. There's no real reason to have two different getters that return exactly the same object except casted to the subclass in one and the superclass in the other.
// |PlatformView| | ||
std::unique_ptr<std::vector<std::string>> ComputePlatformResolvedLocales( | ||
const std::vector<std::string>& supported_locale_data) override; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These were all overrides of methods that were public on the superclass, but we had made private on the subclass. Nothing was ever actually stopping us from calling them... we just had to use a pointer that was casted to the superclass instead of the subclass. self.platformView
always returns the subclass now so made them all public to mirror the declarations on the superclass.
@@ -25,22 +25,23 @@ | |||
#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterRestorationPlugin.h" | |||
#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputDelegate.h" | |||
#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.h" | |||
#import "flutter/shell/platform/darwin/ios/platform_view_ios.h" | |||
#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterView.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
Eliminates some cases where
FlutterViewController
was relying onFlutterEngine
internals:[FlutterEngine shell]
[FlutterEngine platformView]
[FlutterEngine iosPlatformView]
Instead,
FlutterEngine
now exposes:installFirstFrameCallback:
enableSemantics:withFlags:
notifyViewCreated
notifyViewDestroyed
waitForFirstFrameSync:callback:
Also fixes a couple cases where we were relying on transitive header includes:
FlutterAppController
relied onFlutterViewController_Internal.h
forsendDeepLinkToFramework:completionHandler:
This is a refactoring followup to #57099 that introduces no semantic changes.
Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.