From 1eae5e70172847e1cace64e79c6d0cc553d74001 Mon Sep 17 00:00:00 2001 From: hellohuanlin <41930132+hellohuanlin@users.noreply.github.com> Date: Wed, 26 Mar 2025 21:03:02 -0700 Subject: [PATCH] [ios][pv]check UIScreen to be nil in platform view overlay setState call (#166024) A follow up to https://github.com/flutter/flutter/pull/165525 This checks against: 1. flutterView being nil 2. flutterView's screen being nil 3. flutterView's screen's scale being 0 Again, since we can't repro, I am justing guessing here. *List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.* Internal only (See previous PR linked above) *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].* ## 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], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [ ] 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/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --- .../ios/framework/Source/FlutterPlatformViewsTest.mm | 9 +++++++++ .../darwin/ios/framework/Source/overlay_layer_pool.mm | 11 +++++++---- 2 files changed, 16 insertions(+), 4 deletions(-) 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 8d2269c141..6299474ff4 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 @@ -16,6 +16,7 @@ #include "flutter/fml/thread.h" #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/FlutterEngine_Internal.h" #import "flutter/shell/platform/darwin/ios/framework/Source/FlutterEngine_Test.h" #import "flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsController.h" #import "flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h" @@ -4662,6 +4663,14 @@ fml::RefPtr GetDefaultTaskRunner() { layer->UpdateViewState(nil, SkRect::MakeXYWH(1, 2, 3, 4), 0, 0); // Should not update the view state (e.g. overlay_view_wrapper's frame) when FlutterView is nil. XCTAssertTrue(CGRectEqualToRect(layer->overlay_view_wrapper.frame, CGRectZero)); + + FlutterView* flutterView = [[FlutterView alloc] initWithDelegate:engine + opaque:YES + enableWideGamut:NO]; + layer->UpdateViewState(flutterView, SkRect::MakeXYWH(1, 2, 3, 4), 0, 0); + // Should not update the view state (e.g. overlay_view_wrapper's frame) when FlutterView's screen + // is nil. + XCTAssertTrue(CGRectEqualToRect(layer->overlay_view_wrapper.frame, CGRectZero)); } - (void)testFlutterPlatformViewControllerSubmitFramePreservingFrameDamage { diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/overlay_layer_pool.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/overlay_layer_pool.mm index 8cd0e4f1f6..7e12cfbbf3 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/overlay_layer_pool.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/overlay_layer_pool.mm @@ -23,12 +23,15 @@ void OverlayLayer::UpdateViewState(UIView* flutter_view, SkRect rect, int64_t view_id, int64_t overlay_id) { - // There can be a race where UpdateViewState() is called when flutter_view is nil when app is - // backgrounded. - if (!flutter_view) { + FlutterView* flutterView = (FlutterView*)flutter_view; + // There can be a race where UpdateViewState() is called when flutter_view or flutter_view's + // screen is nil when app is backgrounded. + // It's unlikely for scale to be 0, but just to be safe here since it's used in a scaling + // calculation division below + if (!flutterView || !flutterView.screen || flutterView.screen.scale == 0.0f) { return; } - auto screenScale = ((FlutterView*)flutter_view).screen.scale; + CGFloat screenScale = flutterView.screen.scale; // Set the size of the overlay view wrapper. // This wrapper view masks the overlay view. overlay_view_wrapper.frame = CGRectMake(rect.x() / screenScale, rect.y() / screenScale,