🚫 **BLOCKED** : Do not merge until
https://github.com/flutter/flutter/issues/161261 (additional test
coverage).
---
This PR eliminates (1) `scenario_app/android`, and all references to an
Android `scenario_app`, including documentation, CI configuration, build
rules, test runner, Android-side test application, and (2), to verify
and disambiguate the remains, renames the folder `ios_scenario_app`,
which is now accurate.
It also eliminates elements that were _only_ used in the Android-side
scenario_app, such as Firebase Test Lab uploading.
I would be open to doing this in phases if we thought it was better to
do so, but given its mostly a mechanical change (and by renaming the
directory, references can be checked merely by looking for
`/\bscenario_app/`, I believe this is safe to iterate on and eventually
merge after the holidays.
## Background
As of the merged mono-repo, there is no longer a requirement for the
engine to be testable as a standalone unit.
As an example,
[`%ENGINE%/testing/scenario_app`](3762f2e973/engine/src/flutter/testing/scenario_app),
which was intended to _emulate_ the Flutter framework (and some of
`flutter_tools` tooling), load the iOS and Android embedder, and run
various "scenarios" (which ran a combination of `dart:ui` code and
Android Java/iOS Obj-C) verifying golden-file screenshots.
Instead, it is now possible to write and run _real_ (full) Flutter apps
the same way that an end-user (or our own tests) would. One such example
is
[`dev/native_driver_test`](https://github.com/flutter/flutter/tree/master/dev/integration_tests/native_driver_test),
which is a full-fledged Flutter app, which uses standard tooling (i.e.
`flutter`), to test most of the same elements that previously were only
tested in the Android version of `scenario_app`.
This PR introduces a `NativeAssetsManifest.json` next to the `AssetManifest.json` and `FontManifest.json`. This removes the need for embedding the native assets mapping inside the kernel file and will enable decoupling native assets building and bundling from the kernel compilation in flutter tools. This will then allow us to remove dry-run from the build hook protocol.
(It also means all isolate groups will have the same native assets. However, since Flutter does not support `Isolate.spawnUri` from kernel files anyways, this is not a regression.)
This manifest is parsed eagerly on startup by the engine in a manner similar to how the font manifest is parsed. The manifest contents need to be available in the callback for resolving assets, which does not have access to the engine. Therefore the parsed manifest is `NativeAssetsManager` stored in the `IsolateGroupData`. The engine passes it in on isolate group creation, and the FFI callbacks access it from the isolate group data.
Issue:
* https://github.com/flutter/flutter/issues/154425
Related PRs:
* https://dart-review.googlesource.com/c/sdk/+/388161
Follow up work:
* This PR does not yet remove the engine callbacks registered via the dart_api that rely on kernel embedding. If we were to do that in this PR, it would require a manual roll of the engine into flutter/flutter with the PR that switches flutter_tools to emit the native assets manifest instead of embedding in kernel, and a manual roll into g3 to switch emitting a manifest instead of embedding in kernel. A TODO is left in the code for those callbacks to be removed.
## Testing
Most of this PR cannot be tested in isolation. The code in this PR is heavily exercised in the follow up flutter_tools PR which creates the `NativeAssetsManifest.json` and removes the embedding of native assets in kernel files.
* This PR adds a unit test for parsing the JSON manifest.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
fml::scoped_nsobject and fml::scoped_nsprotocol classes are no longer
necessary now that the entire codebase has been migrated to ARC.
This also eliminates the fml_arc_unittests target which tests that code.
All references to that target in documentation, run_tests.py have been
removed. This target was used in various `et` build/query unit tests and
has been replaced with the fml_unittests target in those cases.
Issue: https://github.com/flutter/flutter/issues/137801
The MockCanvas API was originally used to test the output of rendering methods to match them against a reference set of "MockData" structures. This API has been superseded by just using a DisplayListBuilder to record reference operations and then to compare the resulting DisplayList objects.
This PR deletes the last remaining uses of MockCanvas as well as the mechanism itself.
Based on the (internal) discussion around converging on using the official Dart style guide, the design discussion https://flutter.dev/go/use-dart-style-in-flutter-engine, and with the exception of the code that gets published under `dart:ui` (as that is user-facing, and we'd like to evolve the code style in conjunction with the framework), we're going to be (gradually) adopting the Dart style guide in `flutter/engine`.
For now, we:
- Remove `always_specify_types` (except for `lib/ui`, i.e. `dart:ui`)
- Re-enable `type_annotate_public_apis` (which is now relevant since the above is disabled)
- Announce our _intent_ to re-format and apply the Dart formatter (which we'll land in the near future)
---
I also took the opportunity to specify more about our style guide use in general, mostly to make it easier to understand our conventions, and also call out known problem areas (notably, our over-use of `shared_ptr` and `auto` in some cases). I am happy to split those up, but it was easier to make the markdown changes at once.
I also took @cbracken and folks advice and clarified directly that explicit types in Dart are _not_ bad (with examples).
`engine_dev_setup` moved to markdown
*Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.*
*List which issues are fixed by this PR. You must list at least one issue.*
*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Reverts: flutter/engine#52859
Initiated by: zanderso
Reason for reverting: This sort of change *is* in scope for a discussion at the Dash forum.
Original PR Author: matanlurey
Reviewed By: {cbracken, gaaclarke, johnmccutchan}
This change reverts the following previous change:
Based on the (internal) discussion around converging on using the official Dart style guide, with the exception of the code that gets published under `dart:ui`, as that is user-facing, and we'd like to evolve the code style in conjunction with the framework.
I also took the opportunity to specify more about our style guide use in general, mostly to make it easier to understand our conventions, and also call out known problem areas (notably, our over-use of `shared_ptr` and `auto` in some cases). I am happy to split those up, but it was easier to make the markdown changes at once.
I also took @cbracken and folks advice and clarified directly that explicit types in Dart are _not_ bad (with examples).
Based on the (internal) discussion around converging on using the official Dart style guide, with the exception of the code that gets published under `dart:ui`, as that is user-facing, and we'd like to evolve the code style in conjunction with the framework.
I also took the opportunity to specify more about our style guide use in general, mostly to make it easier to understand our conventions, and also call out known problem areas (notably, our over-use of `shared_ptr` and `auto` in some cases). I am happy to split those up, but it was easier to make the markdown changes at once.
I also took @cbracken and folks advice and clarified directly that explicit types in Dart are _not_ bad (with examples).
This avoids the awkward capitalisation 'E.g.' but more importantly
triggers a build with the latest recipe update, which includes Windows
UWP build products.
The engine build bots and engine development team now rely solely on
Xcode 9 to build the engine. The iOS 11 SDK is required (though we build
with deployment target of iOS 8) for several features such as safe area
inset support.
As we anyway always recommend to set the target_os to android.
Also if you happen to not set the target_os to android it messes up the licenses script output.
* Removed 'pub get' from the instructions
According to Hixie, 'pub get' is abstracted into flutter packages upgrade, and so there shouldn't be a need to run pub get directly.
* Removed pub get lines entirely.
