-
Notifications
You must be signed in to change notification settings - Fork 6k
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.
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
iOS: Migrate FlutterEngineGroup to ARC #55503
iOS: Migrate FlutterEngineGroup to ARC #55503
Conversation
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
shell/platform/darwin/ios/framework/Headers/FlutterEngineGroup.h
Outdated
Show resolved
Hide resolved
shell/platform/darwin/ios/framework/Source/FlutterEngineGroup.mm
Outdated
Show resolved
Hide resolved
shell/platform/darwin/ios/framework/Headers/FlutterEngineGroup.h
Outdated
Show resolved
Hide resolved
test-exempt: code refactor with no semantic change |
|
This migrates FlutterEngineGroup to automatic reference counting. It also migrates `enginesCreatedCount` from an ivar to property syntax. In a followup cleanup, I'll look at migrating engine from `NSMutableArray<NSValue*>` to a weak pointer array. Issue: flutter/flutter#137801
9e32a9c
to
675c2b2
Compare
Rebased -- turns out FlutterEngineGroup is alphabetically right next to FlutterHeadlessEngine in the build file. |
Internally to the engine maybe, but it's a public field of the class, and unless there's some very compelling reason, non-private collections should be |
Ah sorry -- GitHub comments never cease to confuse me. I didn't look at the filename, and was mixing it up with my recollection of this field which is non-public:
Alright, yes -- fixing the other one. Thanks for spotting. |
…155952) flutter/engine@b466a0d...e61bc85 2024-09-30 [email protected] fix use_of_uninitialized_value in message_loop_task_queues.cc (flutter/engine#55520) 2024-09-30 [email protected] iOS: Migrate FlutterEngineGroup to ARC (flutter/engine#55503) 2024-09-30 [email protected] Reland [skwasm] Scene builder optimizations for platform view placement (flutter/engine#55468) 2024-09-30 [email protected] Roll Dart SDK from c55ae50d619a to 79863e31de87 (1 revision) (flutter/engine#55525) 2024-09-30 [email protected] Roll Skia from 06721a724831 to dfeeb199b226 (2 revisions) (flutter/engine#55524) 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 [email protected],[email protected] 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
…lutter#155952) flutter/engine@b466a0d...e61bc85 2024-09-30 [email protected] fix use_of_uninitialized_value in message_loop_task_queues.cc (flutter/engine#55520) 2024-09-30 [email protected] iOS: Migrate FlutterEngineGroup to ARC (flutter/engine#55503) 2024-09-30 [email protected] Reland [skwasm] Scene builder optimizations for platform view placement (flutter/engine#55468) 2024-09-30 [email protected] Roll Dart SDK from c55ae50d619a to 79863e31de87 (1 revision) (flutter/engine#55525) 2024-09-30 [email protected] Roll Skia from 06721a724831 to dfeeb199b226 (2 revisions) (flutter/engine#55524) 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 [email protected],[email protected] 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
This migrates FlutterEngineGroup to automatic reference counting. It also migrates
enginesCreatedCount
from an ivar to property syntax.I'll look at migrating engine from
NSMutableArray<NSValue*>
to a weak pointer array in a followup patch since it's orthogonal to ARC migration.Issue: flutter/flutter#137801
Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.