From b66f593f07e2cfa444538923f2da52be73b60a5e Mon Sep 17 00:00:00 2001 From: Wu Zhong Date: Thu, 19 Nov 2020 03:43:02 +0800 Subject: [PATCH] [iOS] Fix platfotm view called multiple times (flutter/engine#19292) --- engine/src/flutter/flow/embedded_views.h | 2 +- .../framework/Source/FlutterPlatformViews.mm | 18 +++-- .../Source/FlutterPlatformViewsTest.mm | 68 +++++++++++++++++++ .../Source/FlutterPlatformViews_Internal.h | 7 +- .../ios/framework/Source/SemanticsObject.mm | 2 +- .../Scenarios/Scenarios/TextPlatformView.m | 8 +++ 6 files changed, 95 insertions(+), 10 deletions(-) diff --git a/engine/src/flutter/flow/embedded_views.h b/engine/src/flutter/flow/embedded_views.h index 4d6853a6c44..307282ff4fd 100644 --- a/engine/src/flutter/flow/embedded_views.h +++ b/engine/src/flutter/flow/embedded_views.h @@ -124,7 +124,7 @@ class Mutator { // // For example consider the following stack: [T1, T2, T3], where T1 is the top // of the stack and T3 is the bottom of the stack. Applying this mutators stack -// to a platform view P1 will result in T1(T2(T2(P1))). +// to a platform view P1 will result in T1(T2(T3(P1))). class MutatorsStack { public: MutatorsStack() = default; diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index feb58fbec9f..b8612946af2 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -157,13 +157,13 @@ void FlutterPlatformViewsController::OnCreate(FlutterMethodCall* call, FlutterRe NSObject* embedded_view = [factory createWithFrame:CGRectZero viewIdentifier:viewId arguments:params]; + UIView* platform_view = [embedded_view view]; // Set a unique view identifier, so the platform view can be identified in unit tests. - [embedded_view view].accessibilityIdentifier = - [NSString stringWithFormat:@"platform_view[%ld]", viewId]; + platform_view.accessibilityIdentifier = [NSString stringWithFormat:@"platform_view[%ld]", viewId]; views_[viewId] = fml::scoped_nsobject>([embedded_view retain]); FlutterTouchInterceptingView* touch_interceptor = [[[FlutterTouchInterceptingView alloc] - initWithEmbeddedView:embedded_view.view + initWithEmbeddedView:platform_view platformViewsController:GetWeakPtr() gestureRecognizersBlockingPolicy:gesture_recognizers_blocking_policies[viewType]] autorelease]; @@ -311,11 +311,11 @@ void FlutterPlatformViewsController::PrerollCompositeEmbeddedView( views_to_recomposite_.insert(view_id); } -NSObject* FlutterPlatformViewsController::GetPlatformViewByID(int view_id) { +UIView* FlutterPlatformViewsController::GetPlatformViewByID(int view_id) { if (views_.empty()) { return nil; } - return views_[view_id].get(); + return [touch_interceptors_[view_id].get() embeddedView]; } std::vector FlutterPlatformViewsController::GetCurrentCanvases() { @@ -451,7 +451,7 @@ void FlutterPlatformViewsController::Reset() { } SkRect FlutterPlatformViewsController::GetPlatformViewRect(int view_id) { - UIView* platform_view = [views_[view_id].get() view]; + UIView* platform_view = GetPlatformViewByID(view_id); UIScreen* screen = [UIScreen mainScreen]; CGRect platform_view_cgrect = [platform_view convertRect:platform_view.bounds toView:flutter_view_]; @@ -747,6 +747,7 @@ void FlutterPlatformViewsController::CommitCATransactionIfNeeded() { @implementation FlutterTouchInterceptingView { fml::scoped_nsobject _delayingRecognizer; FlutterPlatformViewGestureRecognizersBlockingPolicy _blockingPolicy; + UIView* _embeddedView; } - (instancetype)initWithEmbeddedView:(UIView*)embeddedView platformViewsController: @@ -756,6 +757,7 @@ void FlutterPlatformViewsController::CommitCATransactionIfNeeded() { self = [super initWithFrame:embeddedView.frame]; if (self) { self.multipleTouchEnabled = YES; + _embeddedView = embeddedView; embeddedView.autoresizingMask = (UIViewAutoresizingFlexibleWidth | UIViewAutoresizingFlexibleHeight); @@ -777,6 +779,10 @@ void FlutterPlatformViewsController::CommitCATransactionIfNeeded() { return self; } +- (UIView*)embeddedView { + return [[_embeddedView retain] autorelease]; +} + - (void)releaseGesture { _delayingRecognizer.get().state = UIGestureRecognizerStateFailed; } diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm index 31a19b29b6e..43b9243fd3c 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm @@ -39,6 +39,7 @@ const float kFloatCompareEpsilon = 0.001; @interface FlutterPlatformViewsTestMockFlutterPlatformView : NSObject @property(nonatomic, strong) UIView* view; +@property(nonatomic, assign) BOOL viewCreated; @end @implementation FlutterPlatformViewsTestMockFlutterPlatformView @@ -46,10 +47,23 @@ const float kFloatCompareEpsilon = 0.001; - (instancetype)init { if (self = [super init]) { _view = [[FlutterPlatformViewsTestMockPlatformView alloc] init]; + _viewCreated = NO; } return self; } +- (UIView*)view { + [self checkViewCreatedOnce]; + return _view; +} + +- (void)checkViewCreatedOnce { + if (self.viewCreated) { + abort(); + } + self.viewCreated = YES; +} + - (void)dealloc { [_view release]; _view = nil; @@ -107,6 +121,60 @@ fml::RefPtr CreateNewThread(std::string name) { @implementation FlutterPlatformViewsTest +- (void)testFlutterViewOnlyCreateOnceInOneFrame { + flutter::FlutterPlatformViewsTestMockPlatformViewDelegate mock_delegate; + auto thread_task_runner = CreateNewThread("FlutterPlatformViewsTest"); + flutter::TaskRunners runners(/*label=*/self.name.UTF8String, + /*platform=*/thread_task_runner, + /*raster=*/thread_task_runner, + /*ui=*/thread_task_runner, + /*io=*/thread_task_runner); + auto flutterPlatformViewsController = std::make_shared(); + auto platform_view = std::make_unique( + /*delegate=*/mock_delegate, + /*rendering_api=*/flutter::IOSRenderingAPI::kSoftware, + /*platform_views_controller=*/flutterPlatformViewsController, + /*task_runners=*/runners); + + FlutterPlatformViewsTestMockFlutterPlatformFactory* factory = + [[FlutterPlatformViewsTestMockFlutterPlatformFactory new] autorelease]; + flutterPlatformViewsController->RegisterViewFactory( + factory, @"MockFlutterPlatformView", + FlutterPlatformViewGestureRecognizersBlockingPolicyEager); + FlutterResult result = ^(id result) { + }; + flutterPlatformViewsController->OnMethodCall( + [FlutterMethodCall + methodCallWithMethodName:@"create" + arguments:@{@"id" : @2, @"viewType" : @"MockFlutterPlatformView"}], + result); + UIView* mockFlutterView = [[[UIView alloc] initWithFrame:CGRectMake(0, 0, 500, 500)] autorelease]; + flutterPlatformViewsController->SetFlutterView(mockFlutterView); + // Create embedded view params + flutter::MutatorsStack stack; + // Layer tree always pushes a screen scale factor to the stack + SkMatrix screenScaleMatrix = + SkMatrix::MakeScale([UIScreen mainScreen].scale, [UIScreen mainScreen].scale); + stack.PushTransform(screenScaleMatrix); + // Push a translate matrix + SkMatrix translateMatrix = SkMatrix::MakeTrans(100, 100); + stack.PushTransform(translateMatrix); + SkMatrix finalMatrix; + finalMatrix.setConcat(screenScaleMatrix, translateMatrix); + + auto embeddedViewParams = + std::make_unique(finalMatrix, SkSize::Make(300, 300), stack); + + flutterPlatformViewsController->PrerollCompositeEmbeddedView(2, std::move(embeddedViewParams)); + flutterPlatformViewsController->CompositeEmbeddedView(2); + + flutterPlatformViewsController->GetPlatformViewRect(2); + + XCTAssertNotNil(gMockPlatformView); + + flutterPlatformViewsController->Reset(); +} + - (void)testCanCreatePlatformViewWithoutFlutterView { flutter::FlutterPlatformViewsTestMockPlatformViewDelegate mock_delegate; auto thread_task_runner = CreateNewThread("FlutterPlatformViewsTest"); diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h index 41925624c45..7a49334410b 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h @@ -150,12 +150,12 @@ class FlutterPlatformViewsController { void PrerollCompositeEmbeddedView(int view_id, std::unique_ptr params); - // Returns the `FlutterPlatformView` object associated with the view_id. + // Returns the `FlutterPlatformView`'s `view` object associated with the view_id. // // If the `FlutterPlatformViewsController` does not contain any `FlutterPlatformView` object or // a `FlutterPlatformView` object asscociated with the view_id cannot be found, the method // returns nil. - NSObject* GetPlatformViewByID(int view_id); + UIView* GetPlatformViewByID(int view_id); PostPrerollResult PostPrerollAction(fml::RefPtr raster_thread_merger); @@ -331,6 +331,9 @@ class FlutterPlatformViewsController { // Prevent the touch sequence from ever arriving to the embedded view. - (void)blockGesture; + +// Get embedded view +- (UIView*)embeddedView; @end #endif // FLUTTER_SHELL_PLATFORM_DARWIN_IOS_FRAMEWORK_SOURCE_FLUTTERPLATFORMVIEWS_INTERNAL_H_ diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm index 76593f3fa74..e8e54c6cd3f 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/SemanticsObject.mm @@ -559,7 +559,7 @@ flutter::SemanticsAction GetSemanticsActionForScrollDirection( _semanticsObject = object; auto controller = object.bridge->GetPlatformViewsController(); if (controller) { - _platformView = [[controller->GetPlatformViewByID(object.node.platformViewId) view] retain]; + _platformView = [controller->GetPlatformViewByID(object.node.platformViewId) retain]; } } return self; diff --git a/engine/src/flutter/testing/scenario_app/ios/Scenarios/Scenarios/TextPlatformView.m b/engine/src/flutter/testing/scenario_app/ios/Scenarios/Scenarios/TextPlatformView.m index e01bca6c61e..84c6a1691d9 100644 --- a/engine/src/flutter/testing/scenario_app/ios/Scenarios/Scenarios/TextPlatformView.m +++ b/engine/src/flutter/testing/scenario_app/ios/Scenarios/Scenarios/TextPlatformView.m @@ -67,6 +67,7 @@ @implementation TextPlatformView { UITextView* _textView; FlutterMethodChannel* _channel; + BOOL _viewCreated; } - (instancetype)initWithFrame:(CGRect)frame @@ -87,11 +88,18 @@ [_textView addGestureRecognizer:gestureRecognizer]; gestureRecognizer.testTapGestureRecognizerDelegate = self; _textView.accessibilityLabel = @""; + + _viewCreated = NO; } return self; } - (UIView*)view { + // Makes sure the engine only calls this method once. + if (_viewCreated) { + abort(); + } + _viewCreated = YES; return _textView; }