flutter
Delete engine v1 embedding
Fixes https://github.com/flutter/flutter/issues/143531
Also fixes a random typo I found
TODO to test this: -test the framework against this as well, probably with a dummy PR changing the engine commit to my branch if this is possible -figure out if the old embedding is used in g3 at all -~figure out exactly what the ShimPluginRegistry/ShimRegistrar are doing, and if fully deleting them was right~ (see https://github.com/flutter/engine/pull/51229#issuecomment-1981757743)
Pre-launch Checklist
- [x] I read the Contributor Guide and followed the process outlined there for submitting PRs.
- [x] I read the Tree Hygiene wiki page, which explains my responsibilities.
- [x] I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
- [x] 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
///). - [x] I signed the CLA.
- [x] All existing and new tests are passing.
If you need help, consider asking for advice on the #hackers-new channel on Discord.
It seems like the ShimPluginRegistry is used when an app w/ embedding 2 is using a plugin w/ embedding 1, therefore making it probably also fair game to delete? https://github.com/flutter/flutter/blob/f38c5ad44141d3b7711dddd5bbf9c8862a5ed68a/packages/flutter_tools/lib/src/flutter_plugins.dart#L401
There are tests that this will make fail in the framework, so I'll need to address those before landing this.
This will be blocked until I finish and land https://github.com/flutter/flutter/pull/144726, but should be ready for review now. Please inform me if you believe I've deleted something that shouldn't be deleted.
Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).
If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.
Changes reported for pull request #51229 at sha 2d3f5d68d708bf2985bb11b039341d9bf72643d8
![flutter-dashboard[bot] avatar](https://avatars.githubusercontent.com/in/64368?v=4 "Published by a pub.dev verified publisher"){kind=small}
It seems like the ShimPluginRegistry is used when an app w/ embedding 2 is using a plugin w/ embedding 1, therefore making it probably also fair game to delete? https://github.com/flutter/flutter/blob/f38c5ad44141d3b7711dddd5bbf9c8862a5ed68a/packages/flutter_tools/lib/src/flutter_plugins.dart#L401
But it also looks like plugins are always version 2, so I'm not sure this is ever even used regardless
Looks like there was a doc here, and another at go/flutter-delete-android-v1-embedding (I've requested ownership)
Golden file changes are available for triage from new commit, Click here to view.
Changes reported for pull request #51229 at sha 19297dd354ca36ab64509344a18a3265480aea75
Last internal CL has landed, so this is unblocked. I've removed framework code referencing the v1 embedding, and removed internal uses, so at this point the path should be cleared for this removal.
That said, there is still a decent chance landing this will uncover more places it is being referenced, so I'll be watching closely to see if a revert->fix->reland is needed.
Reason for revert: blocking engine->framework roll (I missed some framework code referencing the v1 embedding).