-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
Migrate SemanticsObject and FlutterSemanticsScrollView to ARC #52824
Conversation
@@ -296,14 +289,9 @@ - (void)dealloc { | |||
for (SemanticsObject* child in _children) { | |||
child.parent = nil; | |||
} | |||
[_children removeAllObjects]; | |||
[_children release]; | |||
[_childrenInHitTestOrder release]; | |||
|
|||
_parent = nil; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can remove this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per discussion in the previous PR it's not clear we can. Instead maybe we can add a comment for these two lines explaining why they are necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per discussion in the previous PR
To link it back: #52729 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a comment.
[[[SemanticsObjectContainer alloc] initWithSemanticsObject:self | ||
bridge:self.bridge] autorelease]; | ||
self.container = [[SemanticsObjectContainer alloc] initWithSemanticsObject:self | ||
bridge:[self bridge]]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: self.bridge
@@ -794,7 +779,6 @@ @implementation FlutterSemanticsObject { | |||
|
|||
// Method declared as unavailable in the interface |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
likely not related to this pr - can we just remove this init
since it's already marked as unavailable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -296,14 +289,9 @@ - (void)dealloc { | |||
for (SemanticsObject* child in _children) { | |||
child.parent = nil; | |||
} | |||
[_children removeAllObjects]; | |||
[_children release]; | |||
[_childrenInHitTestOrder release]; | |||
|
|||
_parent = nil; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per discussion in the previous PR it's not clear we can. Instead maybe we can add a comment for these two lines explaining why they are necessary.
1. Migrate TextInputSemanticsObject to ARC 2. Rename the Objective-C-only `accessibility_text_entry.*` files to `TextInputSemanticsObject.*` 3. Move `FlutterInactiveTextInput` interface from the header file into the implementation, the only place it's used. Blocked on `SemanticsObject` ARC migration #52824 Part of flutter/flutter#137801.
…148428) flutter/engine@bf1c6da...41b86b5 2024-05-15 [email protected] Roll Skia from 475559da107d to b9a304eb05d2 (1 revision) (flutter/engine#52857) 2024-05-15 [email protected] Migrate TextInputSemanticsObject to ARC (flutter/engine#52785) 2024-05-15 [email protected] Roll Fuchsia Linux SDK from NbCi2ETfLHPLLB-JV... to Ftw7GopnudHydGS1y... (flutter/engine#52853) 2024-05-15 [email protected] Roll Dart SDK from 7d9de0c0b231 to f773d45634ed (2 revisions) (flutter/engine#52854) 2024-05-15 [email protected] Roll buildroot (flutter/engine#52826) 2024-05-15 [email protected] Roll Skia from 4a7419d662d5 to 475559da107d (5 revisions) (flutter/engine#52848) 2024-05-15 [email protected] Roll Skia from a9ba7411d4b2 to 4a7419d662d5 (1 revision) (flutter/engine#52844) 2024-05-15 [email protected] Roll Dart SDK from 1eec9de2f9c6 to 7d9de0c0b231 (1 revision) (flutter/engine#52841) 2024-05-15 [email protected] Roll Skia from b3aadd56d187 to a9ba7411d4b2 (1 revision) (flutter/engine#52839) 2024-05-15 [email protected] Roll Skia from c0caf10486ce to b3aadd56d187 (3 revisions) (flutter/engine#52837) 2024-05-15 [email protected] Roll Skia from fad584324d85 to c0caf10486ce (1 revision) (flutter/engine#52835) 2024-05-15 [email protected] Migrate SemanticsObject and FlutterSemanticsScrollView to ARC (flutter/engine#52824) 2024-05-15 [email protected] Roll Dart SDK from 0b77fbab8cf5 to 1eec9de2f9c6 (1 revision) (flutter/engine#52833) 2024-05-15 [email protected] Roll Skia from d0d87c26b489 to fad584324d85 (1 revision) (flutter/engine#52831) 2024-05-15 [email protected] Re-add `MipFilter::kBase`, but keep `kNearest` as the default. (flutter/engine#52779) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from NbCi2ETfLHPL to Ftw7GopnudHy 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],[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
On top of the
SemanticsObject
refactor #52729 migrateSemanticsObject
andFlutterSemanticsScrollView
to ARC.Part of flutter/flutter#137801.