Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Remove usages of WindowManager's getDefaultDisplay #55002

Merged
merged 12 commits into from
Sep 25, 2024

Conversation

jesswrd
Copy link
Contributor

@jesswrd jesswrd commented Sep 6, 2024

Removed deprecated usages of WindowManager's getDefaultDisplay() and replaced them with DisplayManager's getDisplay().

Note: Decided to keep a usage of getDefaultDisplay(), which can be found in FlutterView.java because it is expected to be deleted as a part of V1 embedding removal. No changes were made to that file.

path to FlutterView.java: shell/platform/android/io/flutter/embedding/android/FlutterView.java

Fixes #99421

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 and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • 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.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

@jesswrd jesswrd changed the title Remove display func Remove usages of WindowManager's getDefaultDisplay Sep 6, 2024
@chinmaygarde chinmaygarde requested a review from a team September 9, 2024 18:03
Copy link
Contributor

@yaakovschectman yaakovschectman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@reidbaker reidbaker self-requested a review September 9, 2024 19:21
@reidbaker
Copy link
Contributor

before landing remember to update the pre launch checklist in the pull request body.

@reidbaker
Copy link
Contributor

Should this pr be held until flutter/flutter#154746 is resolved?

@jesswrd
Copy link
Contributor Author

jesswrd commented Sep 9, 2024

Should this pr be held until flutter/flutter#154746 is resolved?

Yeah I'll wait for FlutterViewTest to reland before submitting my PR. Need to add my tests in that file anyhow.

@jesswrd jesswrd force-pushed the remove_display_func branch 3 times, most recently from 58b7d5a to 84b4419 Compare September 12, 2024 01:10
@jesswrd jesswrd changed the title Remove usages of WindowManager's getDefaultDisplay [WIP] Remove usages of WindowManager's getDefaultDisplay Sep 13, 2024
@jesswrd jesswrd force-pushed the remove_display_func branch from 84b4419 to 1144638 Compare September 23, 2024 15:14
Copy link
Contributor

@reidbaker reidbaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved % api level documentation and visibility change.

@@ -8,6 +8,7 @@
public class Build {
/** For use in place of the Android Build.VERSION_CODES class. */
public static class API_LEVELS {
public static final int FLUTTER_MIN = 21;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add some documentation for why this value exists and add pointers to it from the other places we change the minimum value. Also I think we should make this value visible for testing. I think in production/shipped code we want to set an actual bound and if minimum value gets higher than a split review that code and remove one branch. But for tests this lets us say "this method should support all api versions that flutter supports".

https://github.com/flutter/flutter/blob/b5749a7d65be14d655a3de4fe9d82cc758c3b85e/docs/platforms/android/New-Android-version.md?plain=1 is a good starting point for finding where we might need to link to and from this file.

// The minimum android api that flutter supports. Intended for use in tests that don't actually care about the api level but want to set a minSdk bound that does not need to regularly be updated when the minimum version changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doc is located in the flutter/flutter repo. Will make a different PR to update the docs.

@jesswrd jesswrd force-pushed the remove_display_func branch 2 times, most recently from 352fad5 to 18e7620 Compare September 25, 2024 16:54
*
* @return some ZeroSides enum
*/
@SuppressWarnings("deprecated")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fyi I think this needs to be

Suggested change
@SuppressWarnings("deprecated")
@SuppressWarnings("deprecation")

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

at least I assume that's why the lint check is failing

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done. Changed baseline.xml file to suppress linter


if (orientation == Configuration.ORIENTATION_LANDSCAPE) {
int rotation =
((DisplayManager) context.getSystemService(Context.DISPLAY_SERVICE))
.getDisplay(0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: intent might be clearer as

Suggested change
.getDisplay(0)
.getDisplay(Display.DEFAULT_DISPLAY)

https://developer.android.com/reference/android/view/Display#constants_1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

@jesswrd jesswrd force-pushed the remove_display_func branch from 18e7620 to d51d1fc Compare September 25, 2024 18:08
@jesswrd jesswrd force-pushed the remove_display_func branch from d51d1fc to 9221a94 Compare September 25, 2024 19:27
@gmackall
Copy link
Member

nit: remove [wip] from title before merging

@jesswrd jesswrd changed the title [WIP] Remove usages of WindowManager's getDefaultDisplay Remove usages of WindowManager's getDefaultDisplay Sep 25, 2024
@jesswrd
Copy link
Contributor Author

jesswrd commented Sep 25, 2024

nit: remove [wip] from title before merging

done.

@jesswrd jesswrd merged commit c0b7827 into flutter:main Sep 25, 2024
29 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 25, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 25, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 25, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 25, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 26, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 26, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 26, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 26, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Sep 26, 2024
…155733)

flutter/engine@d6d5fdb...d4850c1

2024-09-26 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Dart SDK from c2728b947e46 to 016368ee313d (1 revision) (#55444)" (flutter/engine#55454)
2024-09-26 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Add `SurfaceProducer#onSurfaceAvailable`, deprecate `onSurfaceCreated`. (#55418)" (flutter/engine#55450)
2024-09-25 [email protected] Reland "[canvaskit] Further improve overlay optimization by splitting pictures" (flutter/engine#55402)
2024-09-25 [email protected] Move lint suppression from `baseline.xml` to `@SuppressLint`. (flutter/engine#55447)
2024-09-25 [email protected] [engine] set platform thread name to ui. (flutter/engine#55362)
2024-09-25 [email protected] Roll Dart SDK from c2728b947e46 to 016368ee313d (1 revision) (flutter/engine#55444)
2024-09-25 [email protected] [Impeller] actually fix external texture for GLES. (flutter/engine#55414)
2024-09-25 [email protected] Roll Skia from e623a37de332 to 9f3b32b7b772 (2 revisions) (flutter/engine#55443)
2024-09-25 [email protected] Remove usages of WindowManager's getDefaultDisplay (flutter/engine#55002)
2024-09-25 [email protected] Fixes blend + color filter (flutter/engine#55411)
2024-09-25 [email protected] Add a boolean that exposes rotation/crop metadata capability. (flutter/engine#55434)
2024-09-25 [email protected] Roll Skia from 9af762100cf1 to e623a37de332 (1 revision) (flutter/engine#55439)
2024-09-25 [email protected] [scenario_app] delete get bitmap activity. (flutter/engine#55436)
2024-09-25 [email protected] [Flutter GPU] Use vm.Vector4 for clear color instead of ui.Color. (flutter/engine#55416)
2024-09-25 [email protected] Roll Dart SDK from dd73afd20be5 to c2728b947e46 (1 revision) (flutter/engine#55437)
2024-09-25 [email protected] adds more tasks to the engine workspace (flutter/engine#55435)
2024-09-25 [email protected] Roll Skia from 79e652aad7a9 to 9af762100cf1 (2 revisions) (flutter/engine#55433)
2024-09-25 [email protected] Add `SurfaceProducer#onSurfaceAvailable`, deprecate `onSurfaceCreated`. (flutter/engine#55418)

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] 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://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
thejitenpatel pushed a commit to thejitenpatel/flutter that referenced this pull request Sep 26, 2024
…lutter#155733)

flutter/engine@d6d5fdb...d4850c1

2024-09-26 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Dart SDK from c2728b947e46 to 016368ee313d (1 revision) (flutter#55444)" (flutter/engine#55454)
2024-09-26 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Add `SurfaceProducer#onSurfaceAvailable`, deprecate `onSurfaceCreated`. (flutter#55418)" (flutter/engine#55450)
2024-09-25 [email protected] Reland "[canvaskit] Further improve overlay optimization by splitting pictures" (flutter/engine#55402)
2024-09-25 [email protected] Move lint suppression from `baseline.xml` to `@SuppressLint`. (flutter/engine#55447)
2024-09-25 [email protected] [engine] set platform thread name to ui. (flutter/engine#55362)
2024-09-25 [email protected] Roll Dart SDK from c2728b947e46 to 016368ee313d (1 revision) (flutter/engine#55444)
2024-09-25 [email protected] [Impeller] actually fix external texture for GLES. (flutter/engine#55414)
2024-09-25 [email protected] Roll Skia from e623a37de332 to 9f3b32b7b772 (2 revisions) (flutter/engine#55443)
2024-09-25 [email protected] Remove usages of WindowManager's getDefaultDisplay (flutter/engine#55002)
2024-09-25 [email protected] Fixes blend + color filter (flutter/engine#55411)
2024-09-25 [email protected] Add a boolean that exposes rotation/crop metadata capability. (flutter/engine#55434)
2024-09-25 [email protected] Roll Skia from 9af762100cf1 to e623a37de332 (1 revision) (flutter/engine#55439)
2024-09-25 [email protected] [scenario_app] delete get bitmap activity. (flutter/engine#55436)
2024-09-25 [email protected] [Flutter GPU] Use vm.Vector4 for clear color instead of ui.Color. (flutter/engine#55416)
2024-09-25 [email protected] Roll Dart SDK from dd73afd20be5 to c2728b947e46 (1 revision) (flutter/engine#55437)
2024-09-25 [email protected] adds more tasks to the engine workspace (flutter/engine#55435)
2024-09-25 [email protected] Roll Skia from 79e652aad7a9 to 9af762100cf1 (2 revisions) (flutter/engine#55433)
2024-09-25 [email protected] Add `SurfaceProducer#onSurfaceAvailable`, deprecate `onSurfaceCreated`. (flutter/engine#55418)

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] 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://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
thejitenpatel pushed a commit to thejitenpatel/flutter that referenced this pull request Sep 26, 2024
…lutter#155733)

flutter/engine@d6d5fdb...d4850c1

2024-09-26 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Dart SDK from c2728b947e46 to 016368ee313d (1 revision) (flutter#55444)" (flutter/engine#55454)
2024-09-26 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Add `SurfaceProducer#onSurfaceAvailable`, deprecate `onSurfaceCreated`. (flutter#55418)" (flutter/engine#55450)
2024-09-25 [email protected] Reland "[canvaskit] Further improve overlay optimization by splitting pictures" (flutter/engine#55402)
2024-09-25 [email protected] Move lint suppression from `baseline.xml` to `@SuppressLint`. (flutter/engine#55447)
2024-09-25 [email protected] [engine] set platform thread name to ui. (flutter/engine#55362)
2024-09-25 [email protected] Roll Dart SDK from c2728b947e46 to 016368ee313d (1 revision) (flutter/engine#55444)
2024-09-25 [email protected] [Impeller] actually fix external texture for GLES. (flutter/engine#55414)
2024-09-25 [email protected] Roll Skia from e623a37de332 to 9f3b32b7b772 (2 revisions) (flutter/engine#55443)
2024-09-25 [email protected] Remove usages of WindowManager's getDefaultDisplay (flutter/engine#55002)
2024-09-25 [email protected] Fixes blend + color filter (flutter/engine#55411)
2024-09-25 [email protected] Add a boolean that exposes rotation/crop metadata capability. (flutter/engine#55434)
2024-09-25 [email protected] Roll Skia from 9af762100cf1 to e623a37de332 (1 revision) (flutter/engine#55439)
2024-09-25 [email protected] [scenario_app] delete get bitmap activity. (flutter/engine#55436)
2024-09-25 [email protected] [Flutter GPU] Use vm.Vector4 for clear color instead of ui.Color. (flutter/engine#55416)
2024-09-25 [email protected] Roll Dart SDK from dd73afd20be5 to c2728b947e46 (1 revision) (flutter/engine#55437)
2024-09-25 [email protected] adds more tasks to the engine workspace (flutter/engine#55435)
2024-09-25 [email protected] Roll Skia from 79e652aad7a9 to 9af762100cf1 (2 revisions) (flutter/engine#55433)
2024-09-25 [email protected] Add `SurfaceProducer#onSurfaceAvailable`, deprecate `onSurfaceCreated`. (flutter/engine#55418)

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] 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://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
thejitenpatel pushed a commit to thejitenpatel/flutter that referenced this pull request Sep 27, 2024
…lutter#155733)

flutter/engine@d6d5fdb...d4850c1

2024-09-26 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Dart SDK from c2728b947e46 to 016368ee313d (1 revision) (flutter#55444)" (flutter/engine#55454)
2024-09-26 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Add `SurfaceProducer#onSurfaceAvailable`, deprecate `onSurfaceCreated`. (flutter#55418)" (flutter/engine#55450)
2024-09-25 [email protected] Reland "[canvaskit] Further improve overlay optimization by splitting pictures" (flutter/engine#55402)
2024-09-25 [email protected] Move lint suppression from `baseline.xml` to `@SuppressLint`. (flutter/engine#55447)
2024-09-25 [email protected] [engine] set platform thread name to ui. (flutter/engine#55362)
2024-09-25 [email protected] Roll Dart SDK from c2728b947e46 to 016368ee313d (1 revision) (flutter/engine#55444)
2024-09-25 [email protected] [Impeller] actually fix external texture for GLES. (flutter/engine#55414)
2024-09-25 [email protected] Roll Skia from e623a37de332 to 9f3b32b7b772 (2 revisions) (flutter/engine#55443)
2024-09-25 [email protected] Remove usages of WindowManager's getDefaultDisplay (flutter/engine#55002)
2024-09-25 [email protected] Fixes blend + color filter (flutter/engine#55411)
2024-09-25 [email protected] Add a boolean that exposes rotation/crop metadata capability. (flutter/engine#55434)
2024-09-25 [email protected] Roll Skia from 9af762100cf1 to e623a37de332 (1 revision) (flutter/engine#55439)
2024-09-25 [email protected] [scenario_app] delete get bitmap activity. (flutter/engine#55436)
2024-09-25 [email protected] [Flutter GPU] Use vm.Vector4 for clear color instead of ui.Color. (flutter/engine#55416)
2024-09-25 [email protected] Roll Dart SDK from dd73afd20be5 to c2728b947e46 (1 revision) (flutter/engine#55437)
2024-09-25 [email protected] adds more tasks to the engine workspace (flutter/engine#55435)
2024-09-25 [email protected] Roll Skia from 79e652aad7a9 to 9af762100cf1 (2 revisions) (flutter/engine#55433)
2024-09-25 [email protected] Add `SurfaceProducer#onSurfaceAvailable`, deprecate `onSurfaceCreated`. (flutter/engine#55418)

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] 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://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
thejitenpatel pushed a commit to thejitenpatel/flutter that referenced this pull request Sep 27, 2024
…lutter#155733)

flutter/engine@d6d5fdb...d4850c1

2024-09-26 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Dart SDK from c2728b947e46 to 016368ee313d (1 revision) (flutter#55444)" (flutter/engine#55454)
2024-09-26 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Add `SurfaceProducer#onSurfaceAvailable`, deprecate `onSurfaceCreated`. (flutter#55418)" (flutter/engine#55450)
2024-09-25 [email protected] Reland "[canvaskit] Further improve overlay optimization by splitting pictures" (flutter/engine#55402)
2024-09-25 [email protected] Move lint suppression from `baseline.xml` to `@SuppressLint`. (flutter/engine#55447)
2024-09-25 [email protected] [engine] set platform thread name to ui. (flutter/engine#55362)
2024-09-25 [email protected] Roll Dart SDK from c2728b947e46 to 016368ee313d (1 revision) (flutter/engine#55444)
2024-09-25 [email protected] [Impeller] actually fix external texture for GLES. (flutter/engine#55414)
2024-09-25 [email protected] Roll Skia from e623a37de332 to 9f3b32b7b772 (2 revisions) (flutter/engine#55443)
2024-09-25 [email protected] Remove usages of WindowManager's getDefaultDisplay (flutter/engine#55002)
2024-09-25 [email protected] Fixes blend + color filter (flutter/engine#55411)
2024-09-25 [email protected] Add a boolean that exposes rotation/crop metadata capability. (flutter/engine#55434)
2024-09-25 [email protected] Roll Skia from 9af762100cf1 to e623a37de332 (1 revision) (flutter/engine#55439)
2024-09-25 [email protected] [scenario_app] delete get bitmap activity. (flutter/engine#55436)
2024-09-25 [email protected] [Flutter GPU] Use vm.Vector4 for clear color instead of ui.Color. (flutter/engine#55416)
2024-09-25 [email protected] Roll Dart SDK from dd73afd20be5 to c2728b947e46 (1 revision) (flutter/engine#55437)
2024-09-25 [email protected] adds more tasks to the engine workspace (flutter/engine#55435)
2024-09-25 [email protected] Roll Skia from 79e652aad7a9 to 9af762100cf1 (2 revisions) (flutter/engine#55433)
2024-09-25 [email protected] Add `SurfaceProducer#onSurfaceAvailable`, deprecate `onSurfaceCreated`. (flutter/engine#55418)

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] 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://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove usages of Android's getDefaultDisplay()
5 participants