Replace deprecated [UIScreen mainScreen] in FlutterViewController.mm and FlutterViewControllerTest.mm#43690
Conversation
mossmana
changed the title
mossmana
force-pushed
the
issue128260
branch
6 times, most recently
from
be2b56d to
4236cdd
Compare
| if (!windowScene) { | ||
| // When the view is not loaded, it does not make sense to access the interface | ||
| // orientation, bail. | ||
| FML_LOG(WARNING) << "Trying to access the window scene before the view is loaded."; |
There was a problem hiding this comment.
is this intended change?
There was a problem hiding this comment.
The log is a bit redundant, since the windowSceneIfViewLoaded function that is called in the line above already logs the message.
There was a problem hiding this comment.
Although, I guess there could be a case where the windowScene could be null for other reasons. Maybe rather than delete the line, I should log out the comment instead?
There was a problem hiding this comment.
yeah even if both are logged, one is for window scene, and the other is for screen, so not exactly duplicate.
| viewFrame:(CGRect)viewFrame | ||
| convertedFrame:(CGRect)convertedFrame { | ||
| OCMStub([viewControllerMock mainScreenIfViewLoaded]).andReturn(UIScreen.mainScreen); | ||
| - (id)setupMockScreen { |
There was a problem hiding this comment.
nits: setUp
(We do have a lot of misuse of "setup" in our engine code. I think I can create a PR to clean them up)
cc @stuartmorgan
There was a problem hiding this comment.
Ah yes, I wasn't even considering the appropriate verb vs. the noun usage when I modeled against existing functions. I'll go ahead and make the change for this test.
There was a problem hiding this comment.
Or createMockScreen may be more accurate about what the function is doing
There was a problem hiding this comment.
Historically, in the context of a test (not just for iOS), I've usually seen setUp more than create. They can be considered synonyms so, I think I'll go with setUp.
… actual behavior in prep for refactor
|
I went through the places you early return if view is not loaded - I think they are all safe to return, since they are all called after the view is loaded, e.g. scroll callbacks. CC @cyanglaz to double check, especially for add-to-app scenario that I'm not familiar with |
|
Circling back from the triage meeting with chris and stuart - the add-to-app scenario should be fine. i think we are good to land this one. |
…ller.mm and FlutterViewControllerTest.mm (flutter/engine#43690)
flutter/engine@a489c74...ff02fa7 2023-07-24 [email protected] Replace deprecated [UIScreen mainScreen] in FlutterViewController.mm and FlutterViewControllerTest.mm (flutter/engine#43690) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
) flutter/engine@a489c74...ff02fa7 2023-07-24 [email protected] Replace deprecated [UIScreen mainScreen] in FlutterViewController.mm and FlutterViewControllerTest.mm (flutter/engine#43690) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
) flutter/engine@a489c74...ff02fa7 2023-07-24 [email protected] Replace deprecated [UIScreen mainScreen] in FlutterViewController.mm and FlutterViewControllerTest.mm (flutter/engine#43690) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
3 participants
Issue: flutter/flutter#128260
Pre-launch Checklist
///).