Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Fix FlutterMetalLayer testDealloc flakiness on iOS 18 #54403

Merged
auto-submit merged 1 commit into from
Aug 8, 2024
Merged

Fix FlutterMetalLayer testDealloc flakiness on iOS 18 #54403

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion shell/platform/darwin/ios/framework/Source/FlutterMetalLayerTest.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,8 +283,11 @@ - (void)testDealloc {
[compositor commitTransaction];

[self removeMetalLayer:layer];
}
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);
Comment on lines +287 to +290
Copy link
Member

@jmagman jmagman Aug 7, 2024 •
edited
Loading

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];

Copy link
Member

@jmagman jmagman Aug 7, 2024

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.

Copy link
Member Author

@knopp knopp Aug 8, 2024

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.

Copy link
Member

@jmagman jmagman Aug 8, 2024

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.

}

XCTAssertNil(weakLayer);
Expand Down