Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fuchsia: Remove display device availability check from Flutter #21495

Merged
merged 1 commit into from
Oct 1, 2020

Conversation

gnoliyil
Copy link
Contributor

@gnoliyil gnoliyil commented Sep 29, 2020

Description

When Flutter engine connects to Scenic, Scenic has already checked the display and graphics device availability before Scenic starts; so it is guaranteed that display devices are available and surface is valid when it is created.

Thus this change removes the device watching details from flutter surface on Fuchsia so that it doesn't need to do duplicated checks and hides the device-specific details.

Related Issues

b/169632318
http://fxbug.dev/23795

Tests

TEST= PlatformViewTests.CreateSurfaceTest passed on Fuchsia.

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the [contributor guide] and followed the process outlined there for submitting PRs.
  • I signed the [CLA].
  • I read and followed the [C++, Objective-C, Java style guides] for the engine.
  • I read the [tree hygiene] wiki page, which explains my responsibilities.
  • I updated/added relevant documentation.
  • All existing and new tests are passing.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read [handling breaking changes].

@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 to this rule, contact Hixie on the #hackers channel in Chat.

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

@gnoliyil
Copy link
Contributor Author

@jason-simmons PTAL

@arbreng
Copy link
Contributor

arbreng commented Sep 30, 2020

Can we remove the display watching behavior entirely and eliminate this complexity? Scenic should handle this for us, right?

When Flutter engine connects to Scenic, Scenic has already
checked the display and graphics device availability before
Scenic starts; so it is guaranteed that display devices are
available and surface is valid when it is created.

Thus this change removes the device watching details from
flutter surface on Fuchsia so that it doesn't need to do
duplicated checks and hides the device-specific details.
@gnoliyil gnoliyil changed the title fuchsia: Watch display-controller on x64 platforms fuchsia: Remove display device availability check from Flutter Sep 30, 2020
@gnoliyil
Copy link
Contributor Author

Can we remove the display watching behavior entirely and eliminate this complexity? Scenic should handle this for us, right?

Done. Modified the CL to remove the device watching logic. @jason-simmons PTAL

@jason-simmons
Copy link
Member

LGTM

@jason-simmons jason-simmons merged commit 995965d into flutter:master Oct 1, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 1, 2020
gspencergoog pushed a commit to gspencergoog/engine that referenced this pull request Oct 20, 2020
…ter#21495)

When Flutter engine connects to Scenic, Scenic has already
checked the display and graphics device availability before
Scenic starts; so it is guaranteed that display devices are
available and surface is valid when it is created.

Thus this change removes the device watching details from
flutter surface on Fuchsia so that it doesn't need to do
duplicated checks and hides the device-specific details.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants