-
Notifications
You must be signed in to change notification settings - Fork 6k
Fix FlutterMetalLayer testDealloc flakiness on iOS 18 #54403
Conversation
| CFTimeInterval start = CACurrentMediaTime(); | ||
| while (weakLayer != nil && CACurrentMediaTime() - start < 1) { | ||
| // Deallocating the layer after removing is not synchronous. | ||
| CFRunLoopRunInMode(kCFRunLoopDefaultMode, 1, YES); | ||
| CFRunLoopRunInMode(kCFRunLoopDefaultMode, 0.01, YES); |
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.
Would something like this work (I didn't try it):
@interface FlutterMetalLayerTest : XCTestCase
@property(weak) FlutterMetalLayer *weakLayer;
@end
- (void)testDealloc {
@autoreleasepool {
...
self.weakLayer = layer;
...
}
XCTestExpectation *weakNil = [self keyValueObservingExpectationForObject:self keyPath:@"weakLayer" expectedValue:nil];
[self waitForExpectationsWithTimeout:1.0 handler: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.
(I'm not sure about the KVO part, but it would be nice to let -waitForExpectations handle the runloop spinning.
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.
@jmagman, I don't think KVO will work since zeroing weak reference doesn't trigger KVO notifications. I tried this
- (void)testDealloc {
__weak FlutterMetalLayer* weakLayer;
@autoreleasepool {
FlutterMetalLayer* layer = [self addMetalLayer];
weakLayer = layer;
TestCompositor* compositor = [[TestCompositor alloc] initWithLayer:layer];
id<CAMetalDrawable> drawable = [layer nextDrawable];
BAIL_IF_NO_DRAWABLE(drawable);
[drawable present];
[compositor commitTransaction];
[self removeMetalLayer:layer];
}
NSPredicate* predicate =
[NSPredicate predicateWithBlock:^(id evaluatedObject, NSDictionary* bindings) {
return weakLayer == nil;
}];
[self expectationForPredicate:predicate evaluatedWithObject:self handler:nil];
[self waitForExpectationsWithTimeout:1 handler:nil];
}Unfortunately it didn't work either. I can confirm that weakLayer==nil is YES in the predicate and yet the expection fails. Maybe I'm holding it wrong but I'm not sure how.
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.
since zeroing weak reference doesn't trigger KVO notifications
That's what I suspected, but I couldn't think other another way. Thanks for checking.
…153111) flutter/engine@3978ddd...387f6f3 2024-08-08 [email protected] Fix FlutterMetalLayer testDealloc flakiness on iOS 18 (flutter/engine#54403) 2024-08-08 [email protected] [iOS] Bundle dSYM packages in Flutter.xcframework (flutter/engine#54414) 2024-08-08 [email protected] Roll Dart SDK from 0131aabd8631 to 067c7cfcbc8c (1 revision) (flutter/engine#54437) 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
|
Failed to create CP due to merge conflicts. |
|
The fix that this test is for has not been cherry-pick, so I don't think this test should be either. FlutterMetalLayer is not enabled by default in stable. |
Ha yeah, I remembered after the bot failed. Thanks! |
…lutter#153111) flutter/engine@3978ddd...387f6f3 2024-08-08 [email protected] Fix FlutterMetalLayer testDealloc flakiness on iOS 18 (flutter/engine#54403) 2024-08-08 [email protected] [iOS] Bundle dSYM packages in Flutter.xcframework (flutter/engine#54414) 2024-08-08 [email protected] Roll Dart SDK from 0131aabd8631 to 067c7cfcbc8c (1 revision) (flutter/engine#54437) 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
…lutter#153111) flutter/engine@3978ddd...387f6f3 2024-08-08 [email protected] Fix FlutterMetalLayer testDealloc flakiness on iOS 18 (flutter/engine#54403) 2024-08-08 [email protected] [iOS] Bundle dSYM packages in Flutter.xcframework (flutter/engine#54414) 2024-08-08 [email protected] Roll Dart SDK from 0131aabd8631 to 067c7cfcbc8c (1 revision) (flutter/engine#54437) 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
Fixes flutter/flutter#152990
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.