From 17dd9d0035c5a39436ac803c39589fc7766950a3 Mon Sep 17 00:00:00 2001 From: ngminhduong Date: Wed, 19 Feb 2020 21:46:39 +0700 Subject: [PATCH 1/6] Fix issue viewdidload call while init FlutterViewController --- .../darwin/ios/framework/Source/FlutterEngine.mm | 4 ++++ .../ios/framework/Source/FlutterEngine_Internal.h | 1 + .../ios/framework/Source/FlutterViewController.mm | 8 ++++++++ shell/platform/darwin/ios/platform_view_ios.h | 1 + shell/platform/darwin/ios/platform_view_ios.mm | 15 +++++++++------ 5 files changed, 23 insertions(+), 6 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm index 204dba1c6b2db..d72fdfca9f9f5 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm @@ -204,6 +204,10 @@ - (void)setViewController:(FlutterViewController*)viewController { } } +- (void)attachView { + self.iosPlatformView->attachView(); +} + - (void)setFlutterViewControllerWillDeallocObserver:(id)observer { if (observer != _flutterViewControllerWillDeallocObserver) { if (_flutterViewControllerWillDeallocObserver) { diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngine_Internal.h b/shell/platform/darwin/ios/framework/Source/FlutterEngine_Internal.h index 573333ea732e6..277950f9d68d0 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngine_Internal.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngine_Internal.h @@ -44,6 +44,7 @@ - (FlutterTextInputPlugin*)textInputPlugin; - (void)launchEngine:(NSString*)entrypoint libraryURI:(NSString*)libraryOrNil; - (BOOL)createShell:(NSString*)entrypoint libraryURI:(NSString*)libraryOrNil; +- (void)attachView; @end diff --git a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm index 1e082ec4ebd97..33428174df16a 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm @@ -63,6 +63,7 @@ @implementation FlutterViewController { BOOL _initialized; BOOL _viewOpaque; BOOL _engineNeedsLaunch; + BOOL _isAttchedView; NSMutableSet* _ongoingTouches; } @@ -450,6 +451,13 @@ - (void)viewWillAppear:(BOOL)animated { _engineNeedsLaunch = NO; } + if (!_isAttchedView) { + FML_DCHECK([_engine.get() viewController] != nil) + << "FlutterViewController::viewWillAppear:AttachView ViewController was nil"; + [_engine.get() attachView]; + _isAttchedView = YES; + } + // Send platform settings to Flutter, e.g., platform brightness. [self onUserSettingsChanged:nil]; diff --git a/shell/platform/darwin/ios/platform_view_ios.h b/shell/platform/darwin/ios/platform_view_ios.h index d4cc7b8c519bb..00e9130f70142 100644 --- a/shell/platform/darwin/ios/platform_view_ios.h +++ b/shell/platform/darwin/ios/platform_view_ios.h @@ -34,6 +34,7 @@ class PlatformViewIOS final : public PlatformView { fml::WeakPtr GetOwnerViewController() const; void SetOwnerViewController(fml::WeakPtr owner_controller); + void attachView(); void RegisterExternalTexture(int64_t id, NSObject* texture); diff --git a/shell/platform/darwin/ios/platform_view_ios.mm b/shell/platform/darwin/ios/platform_view_ios.mm index bb37fa9610b2b..4c6f40f9ea4e3 100644 --- a/shell/platform/darwin/ios/platform_view_ios.mm +++ b/shell/platform/darwin/ios/platform_view_ios.mm @@ -63,21 +63,24 @@ accessibility_bridge_.reset(); owner_controller_.reset(); }] retain]); + + // 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() { if (owner_controller_) { ios_surface_ = - [static_cast(owner_controller.get().view) createSurface:gl_context_]; + [static_cast(owner_controller_.get().view) createSurface:gl_context_]; FML_DCHECK(ios_surface_ != nullptr); if (accessibility_bridge_) { accessibility_bridge_.reset( new AccessibilityBridge(static_cast(owner_controller_.get().view), this, - [owner_controller.get() platformViewsController])); + [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. } } From 03056f82f84cd958fc8f0b5332d689c0d3416966 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 19 Feb 2020 11:06:23 -0800 Subject: [PATCH 2/6] added unit test --- .../framework/Source/FlutterViewControllerTest.m | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.m b/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.m index 31c2bcc077706..09ef7baa023fb 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.m +++ b/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.m @@ -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. @@ -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 From aac5baccf6be10ea57125181058b387aa40f694d Mon Sep 17 00:00:00 2001 From: ngminhduong Date: Thu, 20 Feb 2020 09:54:30 +0700 Subject: [PATCH 3/6] AttachView while SetOwnerViewController if viewLoaded and fix Spelling --- .../darwin/ios/framework/Source/FlutterViewController.mm | 6 +++--- shell/platform/darwin/ios/platform_view_ios.mm | 3 +++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm index 33428174df16a..0263f01dcfd61 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm @@ -63,7 +63,7 @@ @implementation FlutterViewController { BOOL _initialized; BOOL _viewOpaque; BOOL _engineNeedsLaunch; - BOOL _isAttchedView; + BOOL _isViewAttached; NSMutableSet* _ongoingTouches; } @@ -451,11 +451,11 @@ - (void)viewWillAppear:(BOOL)animated { _engineNeedsLaunch = NO; } - if (!_isAttchedView) { + if (!_isViewAttached) { FML_DCHECK([_engine.get() viewController] != nil) << "FlutterViewController::viewWillAppear:AttachView ViewController was nil"; [_engine.get() attachView]; - _isAttchedView = YES; + _isViewAttached = YES; } // Send platform settings to Flutter, e.g., platform brightness. diff --git a/shell/platform/darwin/ios/platform_view_ios.mm b/shell/platform/darwin/ios/platform_view_ios.mm index 4c6f40f9ea4e3..4fcd450646f63 100644 --- a/shell/platform/darwin/ios/platform_view_ios.mm +++ b/shell/platform/darwin/ios/platform_view_ios.mm @@ -64,6 +64,9 @@ owner_controller_.reset(); }] retain]); + 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 From 321d6b9488544f5d53419359a091d3e8264bb60b Mon Sep 17 00:00:00 2001 From: ngminhduong Date: Fri, 21 Feb 2020 09:06:09 +0700 Subject: [PATCH 4/6] Move attach viewcontroller to the engine and attachView to viewDidLoad instead of viewWillAppear --- .../framework/Source/FlutterViewController.mm | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm index 0263f01dcfd61..e4e9e9fa5d457 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm @@ -442,8 +442,8 @@ - (void)surfaceUpdated:(BOOL)appeared { #pragma mark - UIViewController lifecycle notifications -- (void)viewWillAppear:(BOOL)animated { - TRACE_EVENT0("flutter", "viewWillAppear"); +-(void)viewDidLoad { + TRACE_EVENT0("flutter", "viewDidLoad"); if (_engineNeedsLaunch) { [_engine.get() launchEngine:nil libraryURI:nil]; @@ -451,12 +451,16 @@ - (void)viewWillAppear:(BOOL)animated { _engineNeedsLaunch = NO; } - if (!_isViewAttached) { - FML_DCHECK([_engine.get() viewController] != nil) - << "FlutterViewController::viewWillAppear:AttachView ViewController was nil"; - [_engine.get() attachView]; - _isViewAttached = YES; - } + 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]; From fc916b13535ec90482163615fa698012ddf29446 Mon Sep 17 00:00:00 2001 From: ngminhduong Date: Tue, 25 Feb 2020 07:26:29 +0700 Subject: [PATCH 5/6] Format code and remove unused variable --- .../framework/Source/FlutterViewController.mm | 6 ++---- shell/platform/darwin/ios/platform_view_ios.mm | 16 ++++++++-------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm index e4e9e9fa5d457..6a03171e2dce8 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm @@ -63,7 +63,6 @@ @implementation FlutterViewController { BOOL _initialized; BOOL _viewOpaque; BOOL _engineNeedsLaunch; - BOOL _isViewAttached; NSMutableSet* _ongoingTouches; } @@ -442,7 +441,7 @@ - (void)surfaceUpdated:(BOOL)appeared { #pragma mark - UIViewController lifecycle notifications --(void)viewDidLoad { +- (void)viewDidLoad { TRACE_EVENT0("flutter", "viewDidLoad"); if (_engineNeedsLaunch) { @@ -452,13 +451,12 @@ -(void)viewDidLoad { } FML_DCHECK([_engine.get() viewController] != nil) - << "FlutterViewController::viewWillAppear:AttachView ViewController was nil"; + << "FlutterViewController::viewWillAppear:AttachView ViewController was nil"; [_engine.get() attachView]; [super viewDidLoad]; } - - (void)viewWillAppear:(BOOL)animated { TRACE_EVENT0("flutter", "viewWillAppear"); diff --git a/shell/platform/darwin/ios/platform_view_ios.mm b/shell/platform/darwin/ios/platform_view_ios.mm index 4fcd450646f63..e76671f06542e 100644 --- a/shell/platform/darwin/ios/platform_view_ios.mm +++ b/shell/platform/darwin/ios/platform_view_ios.mm @@ -63,14 +63,14 @@ accessibility_bridge_.reset(); owner_controller_.reset(); }] retain]); - - 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. + + 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() { From 078874d32634303d211696ac0f8eef9995be8e89 Mon Sep 17 00:00:00 2001 From: ngminhduong Date: Tue, 25 Feb 2020 09:12:40 +0700 Subject: [PATCH 6/6] Add FML_DCHECK in attachView fun --- .../platform/darwin/ios/platform_view_ios.mm | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/shell/platform/darwin/ios/platform_view_ios.mm b/shell/platform/darwin/ios/platform_view_ios.mm index e76671f06542e..3207744c3438d 100644 --- a/shell/platform/darwin/ios/platform_view_ios.mm +++ b/shell/platform/darwin/ios/platform_view_ios.mm @@ -74,16 +74,15 @@ } void PlatformViewIOS::attachView() { - if (owner_controller_) { - ios_surface_ = - [static_cast(owner_controller_.get().view) createSurface:gl_context_]; - FML_DCHECK(ios_surface_ != nullptr); - - if (accessibility_bridge_) { - accessibility_bridge_.reset( - new AccessibilityBridge(static_cast(owner_controller_.get().view), this, - [owner_controller_.get() platformViewsController])); - } + FML_DCHECK(owner_controller_); + ios_surface_ = + [static_cast(owner_controller_.get().view) createSurface:gl_context_]; + FML_DCHECK(ios_surface_ != nullptr); + + if (accessibility_bridge_) { + accessibility_bridge_.reset( + new AccessibilityBridge(static_cast(owner_controller_.get().view), this, + [owner_controller_.get() platformViewsController])); } }