From f265745bc233fc1e38dd6b6898bd240e036da822 Mon Sep 17 00:00:00 2001 From: Chris Yang Date: Thu, 6 Jul 2023 10:33:07 -0700 Subject: [PATCH] Revert "[iOS][Keyboard] Wait vsync on UI thread and update viewport inset to avoid jitter." (flutter/engine#43422) Reverts flutter/engine#42312 Original PR caused crash https://github.com/flutter/flutter/issues/130028 Will reopen https://github.com/flutter/flutter/issues/120555 --- .../ios/framework/Source/FlutterEngine.mm | 7 +- .../framework/Source/FlutterEngine_Internal.h | 3 +- .../framework/Source/FlutterViewController.mm | 115 +++++++----------- .../Source/FlutterViewControllerTest.mm | 58 ++++----- .../Source/FlutterViewControllerTest_mrc.mm | 31 +---- .../Source/FlutterViewController_Internal.h | 2 - 6 files changed, 67 insertions(+), 149 deletions(-) diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm index 52e6af116b..85e1f7d4d5 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm @@ -333,12 +333,7 @@ static constexpr int kNumProfilerSamplesPerSec = 5; return _shell->GetTaskRunners().GetPlatformTaskRunner(); } -- (fml::RefPtr)uiTaskRunner { - FML_DCHECK(_shell); - return _shell->GetTaskRunners().GetUITaskRunner(); -} - -- (fml::RefPtr)rasterTaskRunner { +- (fml::RefPtr)RasterTaskRunner { FML_DCHECK(_shell); return _shell->GetTaskRunners().GetRasterTaskRunner(); } diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngine_Internal.h b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngine_Internal.h index 2d476e93eb..6fe25e406a 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngine_Internal.h +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngine_Internal.h @@ -39,8 +39,7 @@ extern NSString* const kFlutterEngineWillDealloc; - (void)dispatchPointerDataPacket:(std::unique_ptr)packet; - (fml::RefPtr)platformTaskRunner; -- (fml::RefPtr)uiTaskRunner; -- (fml::RefPtr)rasterTaskRunner; +- (fml::RefPtr)RasterTaskRunner; - (fml::WeakPtr)platformView; diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm index 2828ea1681..5f565b7cf0 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm @@ -69,11 +69,11 @@ typedef struct MouseState { /** * Keyboard animation properties */ -@property(nonatomic, assign) CGFloat targetViewInsetBottom; -@property(nonatomic, assign) CGFloat originalViewInsetBottom; +@property(nonatomic, assign) double targetViewInsetBottom; @property(nonatomic, retain) VSyncClient* keyboardAnimationVSyncClient; @property(nonatomic, assign) BOOL keyboardAnimationIsShowing; @property(nonatomic, assign) fml::TimePoint keyboardAnimationStartTime; +@property(nonatomic, assign) CGFloat originalViewInsetBottom; @property(nonatomic, assign) BOOL isKeyboardInOrTransitioningFromBackground; /// VSyncClient for touch events delivery frame rate correction. @@ -575,8 +575,8 @@ static void SendFakeTouchEvent(FlutterEngine* engine, // Start on the platform thread. weakPlatformView->SetNextFrameCallback([weakSelf = [self getWeakPtr], platformTaskRunner = [_engine.get() platformTaskRunner], - rasterTaskRunner = [_engine.get() rasterTaskRunner]]() { - FML_DCHECK(rasterTaskRunner->RunsTasksOnCurrentThread()); + RasterTaskRunner = [_engine.get() RasterTaskRunner]]() { + FML_DCHECK(RasterTaskRunner->RunsTasksOnCurrentThread()); // Get callback on raster thread and jump back to platform thread. platformTaskRunner->PostTask([weakSelf]() { if (weakSelf) { @@ -1605,55 +1605,7 @@ static flutter::PointerData::DeviceKind DeviceKindFromTouchType(UITouch* touch) // Invalidate old vsync client if old animation is not completed. [self invalidateKeyboardAnimationVSyncClient]; - - fml::WeakPtr weakSelf = [self getWeakPtr]; - FlutterKeyboardAnimationCallback keyboardAnimationCallback = ^( - fml::TimePoint keyboardAnimationTargetTime) { - if (!weakSelf) { - return; - } - fml::scoped_nsobject flutterViewController( - [(FlutterViewController*)weakSelf.get() retain]); - if (!flutterViewController) { - return; - } - - // If the view controller's view is not loaded, bail out. - if (!flutterViewController.get().isViewLoaded) { - return; - } - // If the view for tracking keyboard animation is nil, means it is not - // created, bail out. - if ([flutterViewController keyboardAnimationView] == nil) { - return; - } - // If keyboardAnimationVSyncClient is nil, means the animation ends. - // And should bail out. - if (flutterViewController.get().keyboardAnimationVSyncClient == nil) { - return; - } - - if ([flutterViewController keyboardAnimationView].superview == nil) { - // Ensure the keyboardAnimationView is in view hierarchy when animation running. - [flutterViewController.get().view addSubview:[flutterViewController keyboardAnimationView]]; - } - - if ([flutterViewController keyboardSpringAnimation] == nil) { - if (flutterViewController.get().keyboardAnimationView.layer.presentationLayer) { - flutterViewController.get()->_viewportMetrics.physical_view_inset_bottom = - flutterViewController.get() - .keyboardAnimationView.layer.presentationLayer.frame.origin.y; - [flutterViewController updateViewportMetricsIfNeeded]; - } - } else { - fml::TimeDelta timeElapsed = - keyboardAnimationTargetTime - flutterViewController.get().keyboardAnimationStartTime; - flutterViewController.get()->_viewportMetrics.physical_view_inset_bottom = - [[flutterViewController keyboardSpringAnimation] curveFunction:timeElapsed.ToSecondsF()]; - [flutterViewController updateViewportMetricsIfNeeded]; - } - }; - [self setupKeyboardAnimationVsyncClient:keyboardAnimationCallback]; + [self setupKeyboardAnimationVsyncClient]; VSyncClient* currentVsyncClient = _keyboardAnimationVSyncClient; [UIView animateWithDuration:duration @@ -1696,28 +1648,45 @@ static flutter::PointerData::DeviceKind DeviceKindFromTouchType(UITouch* touch) toValue:self.targetViewInsetBottom]); } -- (void)setupKeyboardAnimationVsyncClient: - (FlutterKeyboardAnimationCallback)keyboardAnimationCallback { - if (!keyboardAnimationCallback) { - return; - } +- (void)setupKeyboardAnimationVsyncClient { + auto callback = [weakSelf = + [self getWeakPtr]](std::unique_ptr recorder) { + if (!weakSelf) { + return; + } + fml::scoped_nsobject flutterViewController( + [(FlutterViewController*)weakSelf.get() retain]); + if (!flutterViewController) { + return; + } + + if ([flutterViewController keyboardAnimationView].superview == nil) { + // Ensure the keyboardAnimationView is in view hierarchy when animation running. + [flutterViewController.get().view addSubview:[flutterViewController keyboardAnimationView]]; + } + + if ([flutterViewController keyboardSpringAnimation] == nil) { + if (flutterViewController.get().keyboardAnimationView.layer.presentationLayer) { + flutterViewController.get()->_viewportMetrics.physical_view_inset_bottom = + flutterViewController.get() + .keyboardAnimationView.layer.presentationLayer.frame.origin.y; + [flutterViewController updateViewportMetricsIfNeeded]; + } + } else { + fml::TimeDelta timeElapsed = recorder.get()->GetVsyncTargetTime() - + flutterViewController.get().keyboardAnimationStartTime; + + flutterViewController.get()->_viewportMetrics.physical_view_inset_bottom = + [[flutterViewController keyboardSpringAnimation] curveFunction:timeElapsed.ToSecondsF()]; + [flutterViewController updateViewportMetricsIfNeeded]; + } + }; + flutter::Shell& shell = [_engine.get() shell]; NSAssert(_keyboardAnimationVSyncClient == nil, @"_keyboardAnimationVSyncClient must be nil when setup"); - - // Make sure the new viewport metrics get sent after the begin frame event has processed. - fml::scoped_nsprotocol animationCallback( - [keyboardAnimationCallback copy]); - auto uiCallback = [animationCallback, - engine = _engine](std::unique_ptr recorder) { - fml::TimeDelta frameInterval = recorder->GetVsyncTargetTime() - recorder->GetVsyncStartTime(); - fml::TimePoint keyboardAnimationTargetTime = recorder->GetVsyncTargetTime() + frameInterval; - [engine platformTaskRunner]->PostTask([animationCallback, keyboardAnimationTargetTime] { - animationCallback.get()(keyboardAnimationTargetTime); - }); - }; - - _keyboardAnimationVSyncClient = [[VSyncClient alloc] initWithTaskRunner:[_engine uiTaskRunner] - callback:uiCallback]; + _keyboardAnimationVSyncClient = + [[VSyncClient alloc] initWithTaskRunner:shell.GetTaskRunners().GetPlatformTaskRunner() + callback:callback]; _keyboardAnimationVSyncClient.allowPauseAfterVsync = NO; [_keyboardAnimationVSyncClient await]; } diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.mm index 54bca88f57..f2b8d15ee4 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest.mm @@ -26,7 +26,6 @@ FLUTTER_ASSERT_ARC - (void)sendKeyEvent:(const FlutterKeyEvent&)event callback:(nullable FlutterKeyEventCallback)callback userData:(nullable void*)userData; -- (fml::RefPtr)uiTaskRunner; @end /// Sometimes we have to use a custom mock to avoid retain cycles in OCMock. @@ -136,11 +135,10 @@ extern NSNotificationName const FlutterViewControllerWillDealloc; - (FlutterKeyboardMode)calculateKeyboardAttachMode:(NSNotification*)notification; - (CGFloat)calculateMultitaskingAdjustment:(CGRect)screenRect keyboardFrame:(CGRect)keyboardFrame; - (void)startKeyBoardAnimation:(NSTimeInterval)duration; +- (void)setupKeyboardAnimationVsyncClient; - (UIView*)keyboardAnimationView; - (SpringAnimation*)keyboardSpringAnimation; - (void)setupKeyboardSpringAnimationIfNeeded:(CAAnimation*)keyboardAnimation; -- (void)setupKeyboardAnimationVsyncClient: - (FlutterKeyboardAnimationCallback)keyboardAnimationCallback; - (void)ensureViewportMetricsIsCorrect; - (void)invalidateKeyboardAnimationVSyncClient; - (void)addInternalPlugins; @@ -199,6 +197,18 @@ extern NSNotificationName const FlutterViewControllerWillDealloc; OCMVerify([viewControllerMock createTouchRateCorrectionVSyncClientIfNeeded]); } +- (void)testStartKeyboardAnimationWillInvokeSetupKeyboardAnimationVsyncClient { + FlutterEngine* engine = [[FlutterEngine alloc] init]; + [engine runWithEntrypoint:nil]; + FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:engine + nibName:nil + bundle:nil]; + FlutterViewController* viewControllerMock = OCMPartialMock(viewController); + viewControllerMock.targetViewInsetBottom = 100; + [viewControllerMock startKeyBoardAnimation:0.25]; + OCMVerify([viewControllerMock setupKeyboardAnimationVsyncClient]); +} + - (void)testStartKeyboardAnimationWillInvokeSetupKeyboardSpringAnimationIfNeeded { FlutterEngine* engine = [[FlutterEngine alloc] init]; [engine runWithEntrypoint:nil]; @@ -441,34 +451,6 @@ extern NSNotificationName const FlutterViewControllerWillDealloc; } } -- (void)testKeyboardAnimationWillWaitUIThreadVsync { - // We need to make sure the new viewport metrics get sent after the - // begin frame event has processed. And this test is to expect that the callback - // will sync with UI thread. So just simulate a lot of works on UI thread and - // test the keyboard animation callback will execute until UI task completed. - // Related issue: https://github.com/flutter/flutter/issues/120555. - - FlutterEngine* engine = [[FlutterEngine alloc] init]; - [engine runWithEntrypoint:nil]; - FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:engine - nibName:nil - bundle:nil]; - // Post a task to UI thread to block the thread. - const int delayTime = 1; - [engine uiTaskRunner]->PostTask([] { sleep(delayTime); }); - XCTestExpectation* expectation = [self expectationWithDescription:@"keyboard animation callback"]; - - __block CFTimeInterval fulfillTime; - FlutterKeyboardAnimationCallback callback = ^(fml::TimePoint targetTime) { - fulfillTime = CACurrentMediaTime(); - [expectation fulfill]; - }; - CFTimeInterval startTime = CACurrentMediaTime(); - [viewController setupKeyboardAnimationVsyncClient:callback]; - [self waitForExpectationsWithTimeout:5.0 handler:nil]; - XCTAssertTrue(fulfillTime - startTime > delayTime); -} - - (void)testCalculateKeyboardAttachMode { FlutterEngine* mockEngine = OCMPartialMock([[FlutterEngine alloc] init]); [mockEngine createShell:@"" libraryURI:@"" initialRoute:nil]; @@ -647,9 +629,9 @@ extern NSNotificationName const FlutterViewControllerWillDealloc; } - (void)testHandleKeyboardNotification { - FlutterEngine* engine = [[FlutterEngine alloc] init]; - [engine runWithEntrypoint:nil]; - FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:engine + FlutterEngine* mockEngine = OCMPartialMock([[FlutterEngine alloc] init]); + [mockEngine createShell:@"" libraryURI:@"" initialRoute:nil]; + FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:mockEngine nibName:nil bundle:nil]; // keyboard is empty @@ -670,9 +652,11 @@ extern NSNotificationName const FlutterViewControllerWillDealloc; [self setupMockMainScreenAndView:viewControllerMock viewFrame:viewFrame convertedFrame:viewFrame]; viewControllerMock.targetViewInsetBottom = 0; XCTestExpectation* expectation = [self expectationWithDescription:@"update viewport"]; - OCMStub([viewControllerMock updateViewportMetricsIfNeeded]).andDo(^(NSInvocation* invocation) { - [expectation fulfill]; - }); + OCMStub([mockEngine updateViewportMetrics:flutter::ViewportMetrics()]) + .ignoringNonObjectArgs() + .andDo(^(NSInvocation* invocation) { + [expectation fulfill]; + }); [viewControllerMock handleKeyboardNotification:notification]; XCTAssertTrue(viewControllerMock.targetViewInsetBottom == 320 * UIScreen.mainScreen.scale); diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest_mrc.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest_mrc.mm index c19486f0a9..e18b0b55c9 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest_mrc.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewControllerTest_mrc.mm @@ -7,7 +7,6 @@ #import "flutter/shell/platform/darwin/common/framework/Headers/FlutterMacros.h" #import "flutter/shell/platform/darwin/ios/framework/Headers/FlutterViewController.h" -#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController_Internal.h" #import "flutter/shell/platform/darwin/ios/framework/Source/vsync_waiter_ios.h" FLUTTER_ASSERT_NOT_ARC @@ -32,9 +31,7 @@ FLUTTER_ASSERT_NOT_ARC @property(nonatomic, retain) VSyncClient* touchRateCorrectionVSyncClient; - (void)createTouchRateCorrectionVSyncClientIfNeeded; -- (void)setupKeyboardAnimationVsyncClient: - (FlutterKeyboardAnimationCallback)keyboardAnimationCallback; -- (void)startKeyBoardAnimation:(NSTimeInterval)duration; +- (void)setupKeyboardAnimationVsyncClient; - (void)triggerTouchRateCorrectionIfNeeded:(NSSet*)touches; @end @@ -56,9 +53,7 @@ FLUTTER_ASSERT_NOT_ARC FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:engine nibName:nil bundle:nil]; - FlutterKeyboardAnimationCallback callback = ^(fml::TimePoint targetTime) { - }; - [viewController setupKeyboardAnimationVsyncClient:callback]; + [viewController setupKeyboardAnimationVsyncClient]; XCTAssertNotNil(viewController.keyboardAnimationVSyncClient); CADisplayLink* link = [viewController.keyboardAnimationVSyncClient getDisplayLink]; XCTAssertNotNil(link); @@ -178,26 +173,4 @@ FLUTTER_ASSERT_NOT_ARC XCTAssertFalse(link.isPaused); } -- (void)testFlutterViewControllerStartKeyboardAnimationWillCreateVsyncClientCorrectly { - FlutterEngine* engine = [[FlutterEngine alloc] init]; - [engine runWithEntrypoint:nil]; - FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:engine - nibName:nil - bundle:nil]; - viewController.targetViewInsetBottom = 100; - [viewController startKeyBoardAnimation:0.25]; - XCTAssertNotNil(viewController.keyboardAnimationVSyncClient); -} - -- (void) - testSetupKeyboardAnimationVsyncClientWillNotCreateNewVsyncClientWhenKeyboardAnimationCallbackIsNil { - FlutterEngine* engine = [[FlutterEngine alloc] init]; - [engine runWithEntrypoint:nil]; - FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:engine - nibName:nil - bundle:nil]; - [viewController setupKeyboardAnimationVsyncClient:nil]; - XCTAssertNil(viewController.keyboardAnimationVSyncClient); -} - @end diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController_Internal.h b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController_Internal.h index 09cb7c4554..052cf3323b 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController_Internal.h +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController_Internal.h @@ -33,8 +33,6 @@ typedef NS_ENUM(NSInteger, FlutterKeyboardMode) { FlutterKeyboardModeFloating = 2, }; -typedef void (^FlutterKeyboardAnimationCallback)(fml::TimePoint); - @interface FlutterViewController () @property(class, nonatomic, readonly) BOOL accessibilityIsOnOffSwitchLabelsEnabled;