From ef0bc66c152008a2f8f371997dd641a80e87c9ce Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Wed, 17 Apr 2024 13:05:38 +0200 Subject: [PATCH] [macOS] Handle reparenting platform views (flutter/engine#52152) Fixes https://github.com/flutter/flutter/issues/146810 This handles situation where the platform view reparents and replaces itself with a placeholder view. This commonly happens with WKWebView entering full screen. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I added new tests to check the change I am making or feature I am adding, or the PR is [test-exempt]. See [testing the engine] for instructions on writing and running engine tests. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I signed the [CLA]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat --- .../framework/Source/FlutterMutatorView.mm | 15 ++++++++++--- .../Source/FlutterMutatorViewTest.mm | 22 +++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterMutatorView.mm b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterMutatorView.mm index e5bfd3ad707..c9036ecb8ff 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterMutatorView.mm +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterMutatorView.mm @@ -602,9 +602,18 @@ NSMutableArray* ClipPathFromMutations(CGRect master_clip, const MutationVector& [containerSuperview addSubview:_platformViewContainer]; _platformViewContainer.frame = self.bounds; - // Nest the platform view in the PlatformViewContainer. - [_platformViewContainer addSubview:_platformView]; - _platformView.frame = untransformedBounds; + // Nest the platform view in the PlatformViewContainer, but only if the view doesn't have a + // superview yet. Sometimes the platform view reparents itself (WKWebView entering FullScreen) + // in which case do not forcefully move it back. + if (_platformView.superview == nil) { + [_platformViewContainer addSubview:_platformView]; + } + + // Originally first subview would be the _platformView. However during WKWebView full screen + // the platform view gets replaced with a placeholder. Given that _platformViewContainer does + // not contain any other views it is safe to assume that any subview found can be treated + // as the platform view. + _platformViewContainer.subviews.firstObject.frame = untransformedBounds; // Transform for the platform view is finalTransform adjusted for bounding rect origin. CATransform3D translation = diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterMutatorViewTest.mm b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterMutatorViewTest.mm index 576ab9b3d4d..d352eefe3ba 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterMutatorViewTest.mm +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterMutatorViewTest.mm @@ -562,6 +562,28 @@ TEST(FlutterMutatorViewTest, HitTestIgnoreRegion) { EXPECT_EQ([mutatorView hitTest:NSMakePoint(10, 50)], platformView); } +TEST(FlutterMutatorViewTest, ReparentingPlatformView) { + NSView* platformView = [[NSView alloc] init]; + FlutterMutatorView* mutatorView = [[FlutterMutatorView alloc] initWithPlatformView:platformView]; + ApplyFlutterLayer(mutatorView, FlutterSize{100, 100}, {}); + EXPECT_TRUE(platformView.superview == mutatorView.platformViewContainer); + EXPECT_TRUE(CGRectEqualToRect(platformView.frame, CGRectMake(0, 0, 100, 100))); + + // Reparent platform view and replace it with placeholder (mimicking WKWebKit going full screen) + NSView* newParent = [[NSView alloc] init]; + [newParent addSubview:platformView]; + platformView.frame = CGRectMake(10, 10, 200, 200); + + NSView* placeholderView = [[NSView alloc] init]; + [mutatorView.platformViewContainer addSubview:placeholderView]; + ApplyFlutterLayer(mutatorView, FlutterSize{100, 100}, {}); + + // Platform view should not be touched but the replacement view should be properly positioned. + EXPECT_TRUE(platformView.superview == newParent); + EXPECT_TRUE(CGRectEqualToRect(platformView.frame, CGRectMake(10, 10, 200, 200))); + EXPECT_TRUE(CGRectEqualToRect(placeholderView.frame, CGRectMake(0, 0, 100, 100))); +} + @interface FlutterCursorCoordinatorTest : NSObject @end