From b7dadb92d15557a4271823053adba8ede4024259 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 15 Aug 2019 10:29:12 -0700 Subject: [PATCH 1/6] Fix first frame logic --- .../ios/framework/Source/FlutterEngine.mm | 5 ++ .../framework/Source/FlutterEngine_Internal.h | 1 + .../framework/Source/FlutterViewController.mm | 47 ++++++++++--------- 3 files changed, 32 insertions(+), 21 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm index aeb36b9ccabff..d6178f01aa96c 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm @@ -154,6 +154,11 @@ - (void)dispatchPointerDataPacket:(std::unique_ptr)p return _shell->GetTaskRunners().GetPlatformTaskRunner(); } +- (fml::RefPtr)gpuTaskRunner { + FML_DCHECK(_shell); + return _shell->GetTaskRunners().GetGPUTaskRunner(); +} + - (void)ensureSemanticsEnabled { self.iosPlatformView->SetSemanticsEnabled(true); } diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngine_Internal.h b/shell/platform/darwin/ios/framework/Source/FlutterEngine_Internal.h index 4994814d84415..db0aeab8cac2c 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngine_Internal.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngine_Internal.h @@ -32,6 +32,7 @@ - (void)dispatchPointerDataPacket:(std::unique_ptr)packet; - (fml::RefPtr)platformTaskRunner; +- (fml::RefPtr)gpuTaskRunner; - (fml::WeakPtr)platformView; diff --git a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm index 74f604d69fa1b..66628c464f05c 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm @@ -308,31 +308,36 @@ - (void)removeSplashScreenView:(dispatch_block_t _Nullable)onComplete { } - (void)installFirstFrameCallback { - auto weak_platform_view = [_engine.get() platformView]; + fml::WeakPtr weak_platform_view = [_engine.get() platformView]; if (!weak_platform_view) { return; } - __unsafe_unretained auto weak_flutter_view_controller = self; + // This is on the platform thread. - weak_platform_view->SetNextFrameCallback([weak_platform_view, weak_flutter_view_controller, - task_runner = [_engine.get() platformTaskRunner]]() { - // This is on the GPU thread. - task_runner->PostTask([weak_platform_view, weak_flutter_view_controller]() { - // We check if the weak platform view is alive. If it is alive, then the view controller - // also has to be alive since the view controller owns the platform view via the shell - // association. Thus, we are not convinced that the unsafe unretained weak object is in - // fact alive. - if (weak_platform_view) { - if (weak_flutter_view_controller->_splashScreenView) { - [weak_flutter_view_controller removeSplashScreenView:^{ - [weak_flutter_view_controller callViewRenderedCallback]; - }]; - } else { - [weak_flutter_view_controller callViewRenderedCallback]; - } - } - }); - }); + weak_platform_view->SetNextFrameCallback( + [weak_platform_view, platform_task_runner = [_engine.get() platformTaskRunner], + gpu_task_runner = [_engine.get() gpuTaskRunner]]() { + FML_DCHECK(gpu_task_runner->RunsTasksOnCurrentThread()); + platform_task_runner->PostTask([weak_platform_view]() { + // The weak platform view will have a weak pointer to self, if it is + // still alive. We can check if that weak pointer is still valid to + // know if we're still alive. + if (weak_platform_view) { + fml::WeakPtr weak_self = + weak_platform_view->GetOwnerViewController(); + if (!weak_self) { + return; + } + if (weak_self.get()->_splashScreenView) { + [weak_self.get() removeSplashScreenView:^{ + [weak_self.get() callViewRenderedCallback]; + }]; + } else { + [weak_self.get() callViewRenderedCallback]; + } + } + }); + }); } #pragma mark - Properties From 1aaf97a3191fc7db266035737bc979c839de3d02 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 15 Aug 2019 11:24:32 -0700 Subject: [PATCH 2/6] retain --- .../framework/Source/FlutterViewController.mm | 11 +++++--- .../FlutterViewControllerTest.m | 27 +++++++++++++++++++ 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm index 66628c464f05c..beeb1ddfa03d7 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm @@ -328,13 +328,16 @@ - (void)installFirstFrameCallback { if (!weak_self) { return; } - if (weak_self.get()->_splashScreenView) { - [weak_self.get() removeSplashScreenView:^{ - [weak_self.get() callViewRenderedCallback]; + FlutterViewController* flutter_view_controller = + [(FlutterViewController*)weak_self.get() retain]; + if (flutter_view_controller->_splashScreenView) { + [flutter_view_controller removeSplashScreenView:^{ + [flutter_view_controller callViewRenderedCallback]; }]; } else { - [weak_self.get() callViewRenderedCallback]; + [flutter_view_controller callViewRenderedCallback]; } + [flutter_view_controller release]; } }); }); diff --git a/testing/scenario_app/ios/Scenarios/ScenariosTests/FlutterViewControllerTest.m b/testing/scenario_app/ios/Scenarios/ScenariosTests/FlutterViewControllerTest.m index 15acf3fcd7939..2ee99314c9260 100644 --- a/testing/scenario_app/ios/Scenarios/ScenariosTests/FlutterViewControllerTest.m +++ b/testing/scenario_app/ios/Scenarios/ScenariosTests/FlutterViewControllerTest.m @@ -52,4 +52,31 @@ - (void)testFirstFrameCallback { [self waitForExpectationsWithTimeout:30.0 handler:nil]; } +- (void)testLateFirstFrameCallback { + FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"test" project:nil]; + [engine runWithEntrypoint:nil]; + self.flutterViewController = [[FlutterViewController alloc] initWithEngine:engine + nibName:nil + bundle:nil]; + + XCTAssertFalse(self.flutterViewController.isDisplayingFlutterUI); + + XCTestExpectation* displayingFlutterUIExpectation = + [self keyValueObservingExpectationForObject:self.flutterViewController + keyPath:@"displayingFlutterUI" + expectedValue:@YES]; + displayingFlutterUIExpectation.assertForOverFulfill = YES; + + [self.flutterViewController setFlutterViewDidRenderCallback:^{ + [firstFrameRendered fulfill]; + }]; + + AppDelegate* appDelegate = (AppDelegate*)UIApplication.sharedApplication.delegate; + UIViewController* rootVC = appDelegate.window.rootViewController; + [rootVC presentViewController:self.flutterViewController animated:NO completion:nil]; + + [self waitForExpectationsWithTimeout:30.0 handler:nil]; + +} + @end From c5f79efbccca36a27a813a053b885dc11afcc6cb Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 15 Aug 2019 11:33:59 -0700 Subject: [PATCH 3/6] format --- .../ScenariosTests/FlutterViewControllerTest.m | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/testing/scenario_app/ios/Scenarios/ScenariosTests/FlutterViewControllerTest.m b/testing/scenario_app/ios/Scenarios/ScenariosTests/FlutterViewControllerTest.m index 2ee99314c9260..62dee99a3e7dc 100644 --- a/testing/scenario_app/ios/Scenarios/ScenariosTests/FlutterViewControllerTest.m +++ b/testing/scenario_app/ios/Scenarios/ScenariosTests/FlutterViewControllerTest.m @@ -58,25 +58,24 @@ - (void)testLateFirstFrameCallback { self.flutterViewController = [[FlutterViewController alloc] initWithEngine:engine nibName:nil bundle:nil]; - + XCTAssertFalse(self.flutterViewController.isDisplayingFlutterUI); - + XCTestExpectation* displayingFlutterUIExpectation = - [self keyValueObservingExpectationForObject:self.flutterViewController - keyPath:@"displayingFlutterUI" - expectedValue:@YES]; + [self keyValueObservingExpectationForObject:self.flutterViewController + keyPath:@"displayingFlutterUI" + expectedValue:@YES]; displayingFlutterUIExpectation.assertForOverFulfill = YES; - + [self.flutterViewController setFlutterViewDidRenderCallback:^{ [firstFrameRendered fulfill]; }]; - + AppDelegate* appDelegate = (AppDelegate*)UIApplication.sharedApplication.delegate; UIViewController* rootVC = appDelegate.window.rootViewController; [rootVC presentViewController:self.flutterViewController animated:NO completion:nil]; - - [self waitForExpectationsWithTimeout:30.0 handler:nil]; + [self waitForExpectationsWithTimeout:30.0 handler:nil]; } @end From 2a079232ec1f9cbc6bfa45a21af7d577d078316d Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 15 Aug 2019 11:34:20 -0700 Subject: [PATCH 4/6] oops --- .../FlutterViewControllerTest.m | 26 ------------------- 1 file changed, 26 deletions(-) diff --git a/testing/scenario_app/ios/Scenarios/ScenariosTests/FlutterViewControllerTest.m b/testing/scenario_app/ios/Scenarios/ScenariosTests/FlutterViewControllerTest.m index 62dee99a3e7dc..15acf3fcd7939 100644 --- a/testing/scenario_app/ios/Scenarios/ScenariosTests/FlutterViewControllerTest.m +++ b/testing/scenario_app/ios/Scenarios/ScenariosTests/FlutterViewControllerTest.m @@ -52,30 +52,4 @@ - (void)testFirstFrameCallback { [self waitForExpectationsWithTimeout:30.0 handler:nil]; } -- (void)testLateFirstFrameCallback { - FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"test" project:nil]; - [engine runWithEntrypoint:nil]; - self.flutterViewController = [[FlutterViewController alloc] initWithEngine:engine - nibName:nil - bundle:nil]; - - XCTAssertFalse(self.flutterViewController.isDisplayingFlutterUI); - - XCTestExpectation* displayingFlutterUIExpectation = - [self keyValueObservingExpectationForObject:self.flutterViewController - keyPath:@"displayingFlutterUI" - expectedValue:@YES]; - displayingFlutterUIExpectation.assertForOverFulfill = YES; - - [self.flutterViewController setFlutterViewDidRenderCallback:^{ - [firstFrameRendered fulfill]; - }]; - - AppDelegate* appDelegate = (AppDelegate*)UIApplication.sharedApplication.delegate; - UIViewController* rootVC = appDelegate.window.rootViewController; - [rootVC presentViewController:self.flutterViewController animated:NO completion:nil]; - - [self waitForExpectationsWithTimeout:30.0 handler:nil]; -} - @end From a7df92e25a9b6a0803af04797e6194a6b079ad02 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 15 Aug 2019 12:35:13 -0700 Subject: [PATCH 5/6] feedback, further simplification --- .../ios/framework/Source/FlutterEngine.mm | 2 +- .../framework/Source/FlutterEngine_Internal.h | 2 +- .../framework/Source/FlutterViewController.mm | 52 ++++++++----------- 3 files changed, 24 insertions(+), 32 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm index d6178f01aa96c..ad7cfc263ee5d 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm @@ -154,7 +154,7 @@ - (void)dispatchPointerDataPacket:(std::unique_ptr)p return _shell->GetTaskRunners().GetPlatformTaskRunner(); } -- (fml::RefPtr)gpuTaskRunner { +- (fml::RefPtr)GPUTaskRunner { FML_DCHECK(_shell); return _shell->GetTaskRunners().GetGPUTaskRunner(); } diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngine_Internal.h b/shell/platform/darwin/ios/framework/Source/FlutterEngine_Internal.h index db0aeab8cac2c..868d2176a309b 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngine_Internal.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngine_Internal.h @@ -32,7 +32,7 @@ - (void)dispatchPointerDataPacket:(std::unique_ptr)packet; - (fml::RefPtr)platformTaskRunner; -- (fml::RefPtr)gpuTaskRunner; +- (fml::RefPtr)GPUTaskRunner; - (fml::WeakPtr)platformView; diff --git a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm index beeb1ddfa03d7..826b60a11e6ed 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm @@ -308,39 +308,31 @@ - (void)removeSplashScreenView:(dispatch_block_t _Nullable)onComplete { } - (void)installFirstFrameCallback { - fml::WeakPtr weak_platform_view = [_engine.get() platformView]; - if (!weak_platform_view) { + fml::WeakPtr weakPlatformView = [_engine.get() platformView]; + if (!weakPlatformView) { return; } - // This is on the platform thread. - weak_platform_view->SetNextFrameCallback( - [weak_platform_view, platform_task_runner = [_engine.get() platformTaskRunner], - gpu_task_runner = [_engine.get() gpuTaskRunner]]() { - FML_DCHECK(gpu_task_runner->RunsTasksOnCurrentThread()); - platform_task_runner->PostTask([weak_platform_view]() { - // The weak platform view will have a weak pointer to self, if it is - // still alive. We can check if that weak pointer is still valid to - // know if we're still alive. - if (weak_platform_view) { - fml::WeakPtr weak_self = - weak_platform_view->GetOwnerViewController(); - if (!weak_self) { - return; - } - FlutterViewController* flutter_view_controller = - [(FlutterViewController*)weak_self.get() retain]; - if (flutter_view_controller->_splashScreenView) { - [flutter_view_controller removeSplashScreenView:^{ - [flutter_view_controller callViewRenderedCallback]; - }]; - } else { - [flutter_view_controller callViewRenderedCallback]; - } - [flutter_view_controller release]; - } - }); - }); + // Start on the platform thread. + weakPlatformView->SetNextFrameCallback([weakSelf = [self getWeakPtr], + platformTaskRunner = [_engine.get() platformTaskRunner], + gpuTaskRunner = [_engine.get() GPUTaskRunner]]() { + FML_DCHECK(gpuTaskRunner->RunsTasksOnCurrentThread()); + // Get callback on GPU thread and jump back to platform thread. + platformTaskRunner->PostTask([weakSelf]() { + if (weakSelf) { + fml::scoped_nsobject flutterViewController( + [(FlutterViewController*)weakSelf.get() retain]); + if (flutterViewController.get()->_splashScreenView) { + [flutterViewController removeSplashScreenView:^{ + [flutterViewController callViewRenderedCallback]; + }]; + } else { + [flutterViewController callViewRenderedCallback]; + } + } + }); + }); } #pragma mark - Properties From 49c92b3943790a1ed6ea234f0383a81d68578562 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 15 Aug 2019 13:10:41 -0700 Subject: [PATCH 6/6] til --- .../darwin/ios/framework/Source/FlutterViewController.mm | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm index 826b60a11e6ed..2457cecad9960 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm @@ -320,9 +320,9 @@ - (void)installFirstFrameCallback { FML_DCHECK(gpuTaskRunner->RunsTasksOnCurrentThread()); // Get callback on GPU thread and jump back to platform thread. platformTaskRunner->PostTask([weakSelf]() { - if (weakSelf) { - fml::scoped_nsobject flutterViewController( - [(FlutterViewController*)weakSelf.get() retain]); + fml::scoped_nsobject flutterViewController( + [(FlutterViewController*)weakSelf.get() retain]); + if (flutterViewController) { if (flutterViewController.get()->_splashScreenView) { [flutterViewController removeSplashScreenView:^{ [flutterViewController callViewRenderedCallback];