-
Notifications
You must be signed in to change notification settings - Fork 6k
[ios] Fix memory leak in ChildClippingView #50389
Conversation
947e412 to
76217ea
Compare
5fc1bb5 to
17b19d5
Compare
5500abe to
fd3f7cd
Compare
|
@jmagman it's worth checking if this helps improving the platform view performance issue we are having. |
hellohuanlin
left a 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.
LGTM. Could you hold off landing it for now? we are having some perf issues with the platform view, and it may worth checking if this helps.
| } | ||
|
|
||
| - (void)testReleasesBackdropFilterSubviewsOnChildClippingViewDealloc { | ||
| __weak NSMutableArray<UIVisualEffectView*>* weakBackdropFilterSubviews = 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.
nit: snake case
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.
Hm, why would this Objective-C local be snake case?
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.
oh because i saw clipping_view below is snake case. maybe that should be camel case?
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.
Yeah that one should have been camel case.
| } | ||
|
|
||
| - (void)testReleasesBackdropFilterSubviewsOnChildClippingViewDealloc { | ||
| __weak NSMutableArray<UIVisualEffectView*>* weakBackdropFilterSubviews = 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.
Hm, why would this Objective-C local be snake case?
We aren't (currently) investigating memory leaks, I don't think we need to hold off on this. |
…143138) flutter/engine@45137ea...104804a 2024-02-08 [email protected] [ios] Fix memory leak in ChildClippingView (flutter/engine#50389) 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
This fixes the memory leak in
ChildClippingView.Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.