[go: nahoru, domu]
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

[Embedder API] Remove view #51400

Merged
merged 3 commits into from
Mar 18, 2024
Merged

[Embedder API] Remove view #51400

merged 3 commits into from
Mar 18, 2024

Conversation

loic-sharma
Copy link
Member

Adds FlutterEngineRemoveView to the embedder API. This will be used to destroy a view.

The embedder API does not allow embedders to create multiple views yet.

Design doc: https://flutter.dev/go/multi-view-embedder-apis

Part of flutter/flutter#144806
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 14, 2024 00:11
dkwingsmt
dkwingsmt approved these changes Mar 14, 2024
View reviewed changes
Copy link
Contributor
@dkwingsmt dkwingsmt left a comment

Choose a reason for hiding this comment

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

LGTM

cbracken
cbracken approved these changes Mar 18, 2024
View reviewed changes
Copy link
Member
@cbracken cbracken left a comment

Choose a reason for hiding this comment

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

Overall lgtm. Just a couple minor nits!

lib/ui/window/platform_configuration.cc Outdated Show resolved Hide resolved
shell/platform/embedder/tests/embedder_unittests.cc Outdated Show resolved Hide resolved
@loic-sharma loic-sharma added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 18, 2024
@auto-submit auto-submit bot merged commit 12f2abc into flutter:main Mar 18, 2024
28 checks passed
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 18, 2024
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 18, 2024
@engine-flutter-autoroll
12f2abc8b [Embedder API] Remove view (flutter/engine#51400)
e886ecf
@loic-sharma loic-sharma deleted the embedder_api_remove_view branch March 18, 2024 20:00
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Mar 18, 2024
@engine-flutter-autoroll
Roll Flutter Engine from 016206de75bc to 3fde3678a357 (2 revisions) (#…
f704560 
…145350)

flutter/engine@016206d...3fde367

2024-03-18 jason-simmons@users.noreply.github.com Write recent logcat output to the LUCI log directory if an Android unit test fails (flutter/engine#51462)
2024-03-18 737941+loic-sharma@users.noreply.github.com [Embedder API] Remove view (flutter/engine#51400)

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 bdero@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 Apr 5, 2024
Open
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cbracken cbracken cbracken approved these changes

@dkwingsmt dkwingsmt dkwingsmt approved these changes

@chinmaygarde chinmaygarde Awaiting requested review from chinmaygarde chinmaygarde is a code owner

Assignees
No one assigned
Labels
autosubmit Merge PR when tree becomes green via auto submit App
Projects
None yet
Milestone
No milestone
3 participants
@loic-sharma @cbracken @dkwingsmt