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

Conversation

@stuartmorgan-g
Copy link
Contributor

Renames all FLE* classes in the macOS embedding to Flutter*. With the exception
of -[FlutterDartProject engineSwitches], which is very clearly called out in the
comment, the APIs should be stable at this point, so the marker prefix is no
longer needed.

This is a breaking change for macOS embedders, but going forward breaking
changes at the source level for the macOS API should now be rare.

Some of these classes will likely merge with the iOS versions in the future (e.g.,
FlutterDartProject), but that will be an implementation detail that will not affect
clients.

Fixes flutter/flutter#31735

Copy link
Contributor

@iskakaushik iskakaushik left a comment

Choose a reason for hiding this comment

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

LGTM

* Controls embedder plugins and communication with the underlying Flutter engine, managing a view
* intended to handle key inputs and drawing protocols (see |view|).
*
* Can be launched headless (no managed view), at which point a Dart executable will be run on the
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this no longer be headless? I was curious why this got removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This comment was cruft from an API fix that already happened. The original FDE APIs had grafted headless mode onto the view controller, which made no sense. That has since been fixed to align the overall API with iOS, where there's a FlutterEngine that you use directly for headless execution. I forgot to update this comment when I made that change, and noticed it while doing this rename.

@stuartmorgan-g stuartmorgan-g added affects: desktop waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. labels Aug 14, 2019
@stuartmorgan-g stuartmorgan-g merged commit c3e9c14 into flutter:master Aug 14, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 15, 2019
engine-flutter-autoroll added a commit to flutter/flutter that referenced this pull request Aug 15, 2019
[email protected]:flutter/engine.git/compare/c5e30553cd1a...5e155c6

git log c5e3055..5e155c6 --no-merges --oneline
2019-08-14 [email protected] some drive-by docs while I was reading the embedding classes (flutter/engine#9341)
2019-08-14 [email protected] Initialize the engine in the running state to match the animator's default state (flutter/engine#11011)
2019-08-14 [email protected] Trace RasterCacheResult::Draw (flutter/engine#11004)
2019-08-14 [email protected] Rename macOS FLE* classes to Flutter* (flutter/engine#11010)
2019-08-14 [email protected] [Windows] Alternative Windows shell platform implementation (flutter/engine#9835)

The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff ([email protected]), and stop
the roller if necessary.
stuartmorgan-g added a commit to google/flutter-desktop-embedding that referenced this pull request Aug 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

affects: desktop cla: yes platform-macos waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Eliminate FLE* classes from macOS shell

3 participants