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.

Sign up for GitHub

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

Jump to bottom

[Windows] Don't always stop engine on view destruction #51681

Merged
merged 1 commit into from
Mar 29, 2024
Merged

[Windows] Don't always stop engine on view destruction #51681

merged 1 commit into from
Mar 29, 2024

Conversation

loic-sharma
Copy link
Member

@loic-sharma loic-sharma commented Mar 26, 2024 •
edited
Loading

Currently destroying a view also shuts down the engine. This makes sense as in single-view world where the view also always owns the engine. However, in a multi-view world the views will share the engine. Destroying one view shouldn't necessarily shut down the engine unless that view owns the engine.

Part of flutter/flutter#142845

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.

@loic-sharma loic-sharma marked this pull request as ready for review March 28, 2024 03:10
@loic-sharma loic-sharma added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 29, 2024
@auto-submit auto-submit bot merged commit 8f0a2ee into flutter:main Mar 29, 2024
28 checks passed
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 29, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 29, 2024
@engine-flutter-autoroll
8f0a2ee0e [Windows] Don't always stop engine on view destruction (flu…
d5050b3 
…tter/engine#51681)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 29, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 29, 2024
@engine-flutter-autoroll
8f0a2ee0e [Windows] Don't always stop engine on view destruction (flu…
8dafb15 
…tter/engine#51681)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 29, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 29, 2024
@engine-flutter-autoroll
8f0a2ee0e [Windows] Don't always stop engine on view destruction (flu…
c06a059 
…tter/engine#51681)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 29, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 29, 2024
@engine-flutter-autoroll
8f0a2ee0e [Windows] Don't always stop engine on view destruction (flu…
2f8ba6c 
…tter/engine#51681)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 29, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 29, 2024
@engine-flutter-autoroll
8f0a2ee0e [Windows] Don't always stop engine on view destruction (flu…
b88695b 
…tter/engine#51681)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 29, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 29, 2024
@engine-flutter-autoroll
8f0a2ee0e [Windows] Don't always stop engine on view destruction (flu…
45a92e8 
…tter/engine#51681)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 29, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 29, 2024
@engine-flutter-autoroll
8f0a2ee0e [Windows] Don't always stop engine on view destruction (flu…
bbb15b6 
…tter/engine#51681)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 29, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 29, 2024
@engine-flutter-autoroll
8f0a2ee0e [Windows] Don't always stop engine on view destruction (flu…
f44514d 
…tter/engine#51681)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 29, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 29, 2024
@engine-flutter-autoroll
8f0a2ee0e [Windows] Don't always stop engine on view destruction (flu…
9cfdfa2 
…tter/engine#51681)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 29, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 29, 2024
@engine-flutter-autoroll
8f0a2ee0e [Windows] Don't always stop engine on view destruction (flu…
2df988e 
…tter/engine#51681)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 29, 2024
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 29, 2024
@engine-flutter-autoroll
8f0a2ee0e [Windows] Don't always stop engine on view destruction (flu…
868a9cf 
…tter/engine#51681)
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Mar 29, 2024
@engine-flutter-autoroll
Roll Flutter Engine from 68aa9ba386e1 to b16c0f136cdd (10 revisions) (#…
9d6b4f1 
…145990)

flutter/engine@68aa9ba...b16c0f1

2024-03-29 skia-flutter-autoroll@skia.org Roll Skia from 94c0af1297ae to a12e40efacea (3 revisions) (flutter/engine#51776)
2024-03-29 skia-flutter-autoroll@skia.org Roll Skia from b2a01ae03cdf to 94c0af1297ae (1 revision) (flutter/engine#51773)
2024-03-29 skia-flutter-autoroll@skia.org Roll Skia from 6042ad386bcf to b2a01ae03cdf (4 revisions) (flutter/engine#51772)
2024-03-29 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from uzI3wnbEGlZ_dtO0Z... to 3vgfbp1vjXkdMZ09m... (flutter/engine#51767)
2024-03-29 jacksongardner@google.com [skwasm] Fix `toString` methods on Paint and ImageFilter/ColorFilter (flutter/engine#51766)
2024-03-29 john@johnmccutchan.com Reland flutter/engine#51391 (flutter/engine#51764)
2024-03-29 737941+loic-sharma@users.noreply.github.com Add completion callback to `Shell::AddView` (flutter/engine#51659)
2024-03-29 737941+loic-sharma@users.noreply.github.com [Windows] Move keyboard initialization (flutter/engine#51758)
2024-03-29 737941+loic-sharma@users.noreply.github.com [Windows] Don't always stop engine on view destruction (flutter/engine#51681)
2024-03-28 dkwingsmt@users.noreply.github.com [macOS] Group per-view information in `FlutterCompositor` into a class (flutter/engine#51738)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from uzI3wnbEGlZ_ to 3vgfbp1vjXkd

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 jacksongardner@google.com,rmistry@google.com,zra@google.com 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
@loic-sharma loic-sharma mentioned this pull request Mar 29, 2024
Merged
8 tasks
auto-submit bot pushed a commit that referenced this pull request Mar 29, 2024
@loic-sharma
[Windows] Fix EGL surface destruction race (#51781)
0ca0ef6 
This fixes the `WindowsTest.EngineCanTransitionToHeadless` flakiness reported by @matanlurey.

EGL surfaces can only be used by a single thread at a time. Concurrent operations are unsafe.

Previously, the EGL surface was destroyed on the platform thread. This was safe as this always happened after the engine was shutdown and the raster thread was stopped. However, in a multi-view world a view can be destroyed while the engine is running. There may be pending raster tasks that operate on the render surface. Thus, the EGL surfaces should be destroyed on the raster thread when it is available.

This bug was introduced by #51681
Part of flutter/flutter#142845

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yaakovschectman yaakovschectman yaakovschectman approved these changes

@cbracken cbracken Awaiting requested review from cbracken

Assignees
No one assigned
Labels
affects: desktop autosubmit Merge PR when tree becomes green via auto submit App platform-windows
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@loic-sharma @yaakovschectman