replace deprecated [UIScreen mainScreen] in iOS #162785

dkyurtov · 2025-02-06T08:00:58Z

Replaced mainScreen in FlutterPlatformViewsController with the screeScale of the screen that the flutterViewController is attached to. Updated API usage of FlutterClippingMaskViewPool and OverlayLayerPool by providing the correct screenScale from the screen that the flutterViewController is attached to.
Replaced mainScreen in OverlayLayerPool by adding an API for the screenScale to be provided by the class user.
Replaced mainScreen in FlutterClippingMaskViewPool by adding an API for the screenScale to be provided by the class user.
Replaced mainScreen in FlutterPlatformViewsTest by adding a mocked FlutterViewController which has a mocked screen with a mocked screenScale. The flutterViewController is then provided the to the flutterPlatformViewsController.

All of the above changes allow platform views and their overlays displayed on an external screen to be properly scaled when rendered.

Fixes #130832. I believe this fixed #130825 since FlutterPlatformViews_Internal.mm does not exist but the definition of the interfaces declared in FlutterPlatformViews_Internal.h are part of the files modified by this pr. This pr also replaces mainScreen in overlay_layer_pool.mm.

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I updated/added new tests to check the change I am making, or this PR is test-exempt.
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.

google-cla · 2025-02-06T08:01:03Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

- Replaced mainScreen in FlutterPlatformViewsController with the screeScale of the screen that the flutterViewController is attached to. Updated API usage of FlutterClippingMaskViewPool and OverlayLayerPool by providing the correct screenScale from the screen that the flutterViewController is attached to. - Replaced mainScreen in OverlayLayerPool by adding an API for the screenScale to be provided by the class user. - Replaced mainScreen in FlutterClippingMaskViewPool by adding an API for the screenScale to be provided by the class user. - Replaced mainScreen FlutterPlatformViewsTest by adding a mocked FlutterViewController which has a mocked screen with a mocked screenScale. The viewController is then provided the to the flutterPlatformViewsController.

jmagman

@dkyurtov Thanks for the contribution! Out of curiosity, what led you tackle this particular issue?

dkyurtov · 2025-02-14T10:45:24Z

@dkyurtov Thanks for the contribution! Out of curiosity, what led you tackle this particular issue?

@jmagman Glad to contribute! We did not choose to tackle this issue, it chose us. We were experiencing issues with platform view scaling on a secondary display. Only after resolving it did we find that there is an open issue for that.

hellohuanlin · 2025-02-14T19:31:26Z

@dkyurtov Thanks for the contribution! Out of curiosity, what led you tackle this particular issue?

@jmagman Glad to contribute! We did not choose to tackle this issue, it chose us. We were experiencing issues with platform view scaling on a secondary display. Only after resolving it did we find that there is an open issue for that.

Oh I didn't know this actually fixes a bug. That's nice.

jmagman · 2025-02-18T21:25:20Z

@dkyurtov Sorry you're hitting a tricky infrastructure issue which is why "Merge Queue Guard" is stuck. Could you rebase/merge master and push again? Or give me permission to https://github.com/abaltatech/flutter/tree/replace-mainscreen-issue130832-issue130825 and I can do it?

dkyurtov · 2025-02-19T07:28:10Z

@dkyurtov Sorry you're hitting a tricky infrastructure issue which is why "Merge Queue Guard" is stuck. Could you rebase/merge master and push again? Or give me permission to https://github.com/abaltatech/flutter/tree/replace-mainscreen-issue130832-issue130825 and I can do it?

@jmagman No worries, I merged the master. Let me know if anything else is needed.

flutter-dashboard · 2025-02-19T07:29:31Z

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #162785 at sha 501ddb2

jmagman · 2025-02-19T20:45:14Z

@dkyurtov if you're further interested in this, I just spotted #134062.

dkyurtov · 2025-02-20T08:25:43Z

@dkyurtov if you're further interested in this, I just spotted #134062.

@jmagman Okay, thanks for looking out, I will take a look.

…ter#162785)

…)" This reverts commit 3d2ab87.

jmagman · 2025-03-28T16:18:18Z

Heads up @dkyurtov we had to revert this PR in #166080 due to crashes like #165648.
If this relands it will need to be thoroughly tested, we are missing app backgrounding test coverage.

dkyurtov · 2025-03-31T08:36:36Z

Heads up @dkyurtov we had to revert this PR in #166080 due to crashes like #165648. If this relands it will need to be thoroughly tested, we are missing app backgrounding test coverage.

@jmagman Thank you for letting me know. Really sorry to hear that the pr has caused those crashes. Can you provide more information on the crashes (links to the other crash issues that you mentioned or steps to reproduce the linked one) so I can understand the issue, fix it and make sure that it is not happening again. Going only based on the stack trace will be difficult.

If you could provide more information about the additional test coverage, that would also be great. I am not sure whether you mean simulating the background conditions that led to the bug in unit tests or write UI tests that truly send the app to the background (I assume that the crashes have something to do with backgrounding the app).

hellohuanlin · 2025-03-31T20:40:48Z

Heads up @dkyurtov we had to revert this PR in #166080 due to crashes like #165648. If this relands it will need to be thoroughly tested, we are missing app backgrounding test coverage.

@jmagman Thank you for letting me know. Really sorry to hear that the pr has caused those crashes. Can you provide more information on the crashes (links to the other crash issues that you mentioned or steps to reproduce the linked one) so I can understand the issue, fix it and make sure that it is not happening again. Going only based on the stack trace will be difficult.

If you could provide more information about the additional test coverage, that would also be great. I am not sure whether you mean simulating the background conditions that led to the bug in unit tests or write UI tests that truly send the app to the background (I assume that the crashes have something to do with backgrounding the app).

The crash happens internally for some customers, and I wasn't able to reproduce it. What I know is there are at least 2 divide-by-zero, both inside platform view logic.

Strawman idea: maybe cache the screen scale since it usually don't change? (unless in rare case when you switch to external monitor).

dkyurtov · 2025-04-01T07:07:31Z

Heads up @dkyurtov we had to revert this PR in #166080 due to crashes like #165648. If this relands it will need to be thoroughly tested, we are missing app backgrounding test coverage.

@jmagman Thank you for letting me know. Really sorry to hear that the pr has caused those crashes. Can you provide more information on the crashes (links to the other crash issues that you mentioned or steps to reproduce the linked one) so I can understand the issue, fix it and make sure that it is not happening again. Going only based on the stack trace will be difficult.
If you could provide more information about the additional test coverage, that would also be great. I am not sure whether you mean simulating the background conditions that led to the bug in unit tests or write UI tests that truly send the app to the background (I assume that the crashes have something to do with backgrounding the app).

The crash happens internally for some customers, and I wasn't able to reproduce it. What I know is there are at least 2 divide-by-zero, both inside platform view logic.

Strawman idea: maybe cache the screen scale since it usually don't change? (unless in rare case when you switch to external monitor).

Okay, thank you for the additional information. I would assume that the crash happened when flutterScreenIfViewLoaded returned nil. I will investigate when does that happen and how to handle this case. I will think about your idea but my initial thoughts are that it will problematic for the reasons that you mentioned in the brackets.

…ter#162785)

github-actions bot added platform-ios iOS applications specifically engine flutter/engine related. See also e: labels. labels Feb 6, 2025

dkyurtov force-pushed the replace-mainscreen-issue130832-issue130825 branch from 932fd68 to 5496c4a Compare February 11, 2025 13:43

github-actions bot added the team-ios Owned by iOS platform team label Feb 11, 2025

jmagman requested review from cbracken and hellohuanlin February 12, 2025 22:19

hellohuanlin approved these changes Feb 12, 2025

View reviewed changes

jmagman approved these changes Feb 14, 2025

View reviewed changes

jmagman mentioned this pull request Feb 14, 2025

Build Plan JSON workflow requires approval #163274

Closed

jmagman added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 14, 2025

Merge branch 'master' into replace-mainscreen-issue130832-issue130825

501ddb2

flutter-dashboard bot added the will affect goldens Changes to golden files label Feb 19, 2025

jmagman mentioned this pull request Feb 19, 2025

"Golden file changes have been found for this pull request", no images found #163647

Open

auto-submit bot added this pull request to the merge queue Feb 19, 2025

Merged via the queue into flutter:master with commit 3d2ab87 Feb 19, 2025
175 of 176 checks passed

flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Feb 19, 2025

dkyurtov deleted the replace-mainscreen-issue130832-issue130825 branch February 20, 2025 08:26

engine-flutter-autoroll mentioned this pull request Feb 20, 2025

Roll Flutter from e8f34a9eea90 to e6c333b555b5 (120 revisions) flutter/packages#8666

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 20, 2025

3d2ab87 replace deprecated [UIScreen mainScreen] in iOS (flutter/flut…

d5ee037

…ter#162785)

engine-flutter-autoroll mentioned this pull request Feb 20, 2025

Roll Flutter from e8f34a9eea90 to 7df3f8fc06e3 (125 revisions) flutter/packages#8667

Closed

engine-flutter-autoroll mentioned this pull request Feb 21, 2025

Roll Flutter from 39b4951f8f0b to 911aa7547ed7 (56 revisions) flutter/packages#8674

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 22, 2025

3d2ab87 replace deprecated [UIScreen mainScreen] in iOS (flutter/flut…

569be33

…ter#162785)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 22, 2025

3d2ab87 replace deprecated [UIScreen mainScreen] in iOS (flutter/flut…

e5988b7

…ter#162785)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 22, 2025

3d2ab87 replace deprecated [UIScreen mainScreen] in iOS (flutter/flut…

4af1972

…ter#162785)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 22, 2025

3d2ab87 replace deprecated [UIScreen mainScreen] in iOS (flutter/flut…

b35bbe6

…ter#162785)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 23, 2025

3d2ab87 replace deprecated [UIScreen mainScreen] in iOS (flutter/flut…

b75eb77

…ter#162785)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 23, 2025

3d2ab87 replace deprecated [UIScreen mainScreen] in iOS (flutter/flut…

d000bba

…ter#162785)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 24, 2025

3d2ab87 replace deprecated [UIScreen mainScreen] in iOS (flutter/flut…

fa85762

…ter#162785)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 24, 2025

3d2ab87 replace deprecated [UIScreen mainScreen] in iOS (flutter/flut…

d324599

…ter#162785)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 24, 2025

3d2ab87 replace deprecated [UIScreen mainScreen] in iOS (flutter/flut…

cfb0d8e

…ter#162785)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 24, 2025

3d2ab87 replace deprecated [UIScreen mainScreen] in iOS (flutter/flut…

f6743e0

…ter#162785)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 24, 2025

3d2ab87 replace deprecated [UIScreen mainScreen] in iOS (flutter/flut…

415cd32

…ter#162785)

jmagman mentioned this pull request Mar 27, 2025

[DO NOT LAND][LAST RESORT] [ios][pv] regression on screen scale revert to deprecated api #166036

Closed

9 tasks

hellohuanlin added a commit to hellohuanlin/flutter that referenced this pull request Mar 27, 2025

Revert "replace deprecated [UIScreen mainScreen] in iOS (flutter#162785…

bc522da

…)" This reverts commit 3d2ab87.

hellohuanlin mentioned this pull request Mar 27, 2025

[ios][pv]fully revert the UIScreen.main deprecated API change #166080

Merged

9 tasks

hellohuanlin added a commit to hellohuanlin/flutter that referenced this pull request Mar 28, 2025

Revert "replace deprecated [UIScreen mainScreen] in iOS (flutter#162785…

c003171

…)" This reverts commit 3d2ab87.

jmagman mentioned this pull request Mar 28, 2025

Replace deprecated [UIScreen mainScreen] in FlutterPlatformViews.mm and FlutterPlatformViewsTest.mm #130832

Open

2 tasks

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025

3d2ab87 replace deprecated [UIScreen mainScreen] in iOS (flutter/flut…

d92932c

…ter#162785)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025

3d2ab87 replace deprecated [UIScreen mainScreen] in iOS (flutter/flut…

2457ab5

…ter#162785)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 21, 2025

3d2ab87 replace deprecated [UIScreen mainScreen] in iOS (flutter/flut…

2640587

…ter#162785)

replace deprecated [UIScreen mainScreen] in iOS #162785

replace deprecated [UIScreen mainScreen] in iOS #162785

Uh oh!

Conversation

dkyurtov commented Feb 6, 2025

Pre-launch Checklist

Uh oh!

google-cla bot commented Feb 6, 2025

Uh oh!

jmagman left a comment

Choose a reason for hiding this comment

Uh oh!

dkyurtov commented Feb 14, 2025

Uh oh!

hellohuanlin commented Feb 14, 2025

Uh oh!

jmagman commented Feb 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dkyurtov commented Feb 19, 2025

Uh oh!

flutter-dashboard bot commented Feb 19, 2025

Uh oh!

jmagman commented Feb 19, 2025

Uh oh!

Uh oh!

dkyurtov commented Feb 20, 2025

Uh oh!

jmagman commented Mar 28, 2025

Uh oh!

dkyurtov commented Mar 31, 2025

Uh oh!

hellohuanlin commented Mar 31, 2025

Uh oh!

dkyurtov commented Apr 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jmagman commented Feb 18, 2025 •

edited

Loading