Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions shell/platform/darwin/ios/framework/Source/FlutterEngine.mm
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,10 @@ - (void)setViewController:(FlutterViewController*)viewController {
}
}

- (void)attachView {
self.iosPlatformView->attachView();
}

- (void)setFlutterViewControllerWillDeallocObserver:(id<NSObject>)observer {
if (observer != _flutterViewControllerWillDeallocObserver) {
if (_flutterViewControllerWillDeallocObserver) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
- (FlutterTextInputPlugin*)textInputPlugin;
- (void)launchEngine:(NSString*)entrypoint libraryURI:(NSString*)libraryOrNil;
- (BOOL)createShell:(NSString*)entrypoint libraryURI:(NSString*)libraryOrNil;
- (void)attachView;

@end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -441,15 +441,25 @@ - (void)surfaceUpdated:(BOOL)appeared {

#pragma mark - UIViewController lifecycle notifications

- (void)viewWillAppear:(BOOL)animated {
Copy link
Member

Choose a reason for hiding this comment

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

I get that we moved a bunch of stuff out of PlatformViewIOS::SetOwnerViewController which is definitely the right thing to do. But I'm not sure I understood why the change from viewWillAppear to viewDidLoad. Strictly speaking, viewDidLoad doesn't signal intent. It'll be called even if you just do flutterViewController.view without ever trying to present the FlutterViewController and we'll end up with an IOSSurface and an AccessibilityBridge.

Is it just a convenient way of avoiding having to keep track of whether viewWillAppear was already called once per instance?

Copy link
Contributor

Choose a reason for hiding this comment

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

viewDidLoad means we have a valid view we can use. It's up to the developer to make sure they avoid loading the view earlier than they should, right?

Copy link
Member

Choose a reason for hiding this comment

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

probably. The side effects is not very obvious however. Might as well take away another foot gun.

TRACE_EVENT0("flutter", "viewWillAppear");
- (void)viewDidLoad {
TRACE_EVENT0("flutter", "viewDidLoad");

if (_engineNeedsLaunch) {
[_engine.get() launchEngine:nil libraryURI:nil];
[_engine.get() setViewController:self];
_engineNeedsLaunch = NO;
}

FML_DCHECK([_engine.get() viewController] != nil)
<< "FlutterViewController::viewWillAppear:AttachView ViewController was nil";
[_engine.get() attachView];

[super viewDidLoad];
}

- (void)viewWillAppear:(BOOL)animated {
TRACE_EVENT0("flutter", "viewWillAppear");

// Send platform settings to Flutter, e.g., platform brightness.
[self onUserSettingsChanged:nil];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@

FLUTTER_ASSERT_ARC

@interface FlutterEngine ()
- (BOOL)createShell:(NSString*)entrypoint libraryURI:(NSString*)libraryURI;
@end

extern NSNotificationName const FlutterViewControllerWillDealloc;

/// A simple mock class for FlutterEngine.
Expand Down Expand Up @@ -457,4 +461,16 @@ - (void)testWillDeallocNotification {
[self waitForExpectations:@[ expectation ] timeout:1.0];
}

- (void)testDoesntLoadViewInInit {
XCTestExpectation* expectation =
[[XCTestExpectation alloc] initWithDescription:@"notification called"];
FlutterDartProject* project = [[FlutterDartProject alloc] init];
FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"foobar" project:project];
[engine createShell:@"" libraryURI:@""];
FlutterViewController* realVC = [[FlutterViewController alloc] initWithEngine:engine
nibName:nil
bundle:nil];
XCTAssertFalse([realVC isViewLoaded], @"shouldn't have loaded since it hasn't been shown");
}

@end
1 change: 1 addition & 0 deletions shell/platform/darwin/ios/platform_view_ios.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class PlatformViewIOS final : public PlatformView {

fml::WeakPtr<FlutterViewController> GetOwnerViewController() const;
void SetOwnerViewController(fml::WeakPtr<FlutterViewController> owner_controller);
void attachView();

void RegisterExternalTexture(int64_t id, NSObject<FlutterTexture>* texture);

Expand Down
33 changes: 19 additions & 14 deletions shell/platform/darwin/ios/platform_view_ios.mm
Original file line number Diff line number Diff line change
Expand Up @@ -64,20 +64,25 @@
owner_controller_.reset();
}] retain]);

if (owner_controller_) {
ios_surface_ =
[static_cast<FlutterView*>(owner_controller.get().view) createSurface:gl_context_];
FML_DCHECK(ios_surface_ != nullptr);

if (accessibility_bridge_) {
accessibility_bridge_.reset(
new AccessibilityBridge(static_cast<FlutterView*>(owner_controller_.get().view), this,
[owner_controller.get() platformViewsController]));
}
// Do not call `NotifyCreated()` here - let FlutterViewController take care
// of that when its Viewport is sized. If `NotifyCreated()` is called here,
// it can occasionally get invoked before the viewport is sized resulting in
// a framebuffer that will not be able to completely attach.
if (owner_controller_ && [owner_controller_.get() isViewLoaded]) {
this->attachView();
}
// Do not call `NotifyCreated()` here - let FlutterViewController take care
// of that when its Viewport is sized. If `NotifyCreated()` is called here,
// it can occasionally get invoked before the viewport is sized resulting in
// a framebuffer that will not be able to completely attach.
}

void PlatformViewIOS::attachView() {
FML_DCHECK(owner_controller_);
ios_surface_ =
[static_cast<FlutterView*>(owner_controller_.get().view) createSurface:gl_context_];
FML_DCHECK(ios_surface_ != nullptr);

if (accessibility_bridge_) {
accessibility_bridge_.reset(
new AccessibilityBridge(static_cast<FlutterView*>(owner_controller_.get().view), this,
[owner_controller_.get() platformViewsController]));
}
}

Expand Down