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

Read the FLTEnableImpeller flag from the right bundle #40535

Merged
merged 1 commit into from
Mar 22, 2023
Merged

Read the FLTEnableImpeller flag from the right bundle #40535

Show file tree
Hide file tree
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
8 changes: 6 additions & 2 deletions shell/platform/darwin/ios/framework/Source/FlutterDartProject.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,8 +206,12 @@
BOOL enableWideGamut = nsEnableWideGamut ? nsEnableWideGamut.boolValue : NO;
settings.enable_wide_gamut = enableWideGamut;

// Whether to enable Impeller.
NSNumber* enableImpeller = [mainBundle objectForInfoDictionaryKey:@"FLTEnableImpeller"];
// Whether to enable Impeller. First, look in the app bundle.
NSNumber* enableImpeller = [bundle objectForInfoDictionaryKey:@"FLTEnableImpeller"];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to check both, right? Otherwise we will break customers who followed our instructions to add the flag to their app's Info.plist.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohh. Good point. I need to take another look at this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like this:

Suggested change
NSNumber* enableImpeller = [bundle objectForInfoDictionaryKey:@"FLTEnableImpeller"];
NSNumber* enableImpellerMainBundle = [mainBundle objectForInfoDictionaryKey:@"FLTEnableImpeller"];
NSNumber* enableImpeller =
enableImpellerMainBundle != nil
? enableImpellerMainBundle
: [bundle objectForInfoDictionaryKey:@"FLTEnableImpeller"];

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Based on the comment above, I look in the app bundle first.

if (enableImpeller == nil) {
// If it isn't in the app bundle, look in the main bundle.
enableImpeller = [mainBundle objectForInfoDictionaryKey:@"FLTEnableImpeller"];
}
// Change the default only if the option is present.
if (enableImpeller != nil) {
settings.enable_impeller = enableImpeller.boolValue;
Expand Down
47 changes: 43 additions & 4 deletions shell/platform/darwin/ios/framework/Source/FlutterDartProjectTest.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,54 @@ - (void)testFLTFrameworkBundleInternalWhenBundleIsPresent {
XCTAssertNotNil(found);
}

- (void)testDisableImpellerSettingIsCorrectlyParsed {
id mockMainBundle = OCMPartialMock([NSBundle mainBundle]);
OCMStub([mockMainBundle objectForInfoDictionaryKey:@"FLTEnableImpeller"]).andReturn(@"NO");

auto settings = FLTDefaultSettingsForBundle();
// Check settings.enable_impeller value is same as the value defined in Info.plist.
XCTAssertEqual(settings.enable_impeller, NO);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

XCTAssertTrue and XCTAssertFalse are a little more idiomatic, but it's a minor nit.

Suggested change
XCTAssertEqual(settings.enable_impeller, NO);
XCTAssertFalse(settings.enable_impeller);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect I'll be touching this file again soon, and I'll keep this in mind for a cleanup then.

[mockMainBundle stopMocking];
}

- (void)testEnableImpellerSettingIsCorrectlyParsed {
// The FLTEnableImpeller's value is defined in Info.plist
NSBundle* mainBundle = [NSBundle mainBundle];
NSNumber* enableImpeller = [mainBundle objectForInfoDictionaryKey:@"FLTEnableImpeller"];
XCTAssertEqual(enableImpeller.boolValue, NO);
id mockMainBundle = OCMPartialMock([NSBundle mainBundle]);
OCMStub([mockMainBundle objectForInfoDictionaryKey:@"FLTEnableImpeller"]).andReturn(@"YES");

auto settings = FLTDefaultSettingsForBundle();
// Check settings.enable_impeller value is same as the value defined in Info.plist.
XCTAssertEqual(settings.enable_impeller, YES);
[mockMainBundle stopMocking];
}

- (void)testDisableImpellerAppBundleSettingIsCorrectlyParsed {
NSString* bundleId = [FlutterDartProject defaultBundleIdentifier];
id mockAppBundle = OCMClassMock([NSBundle class]);
OCMStub([mockAppBundle objectForInfoDictionaryKey:@"FLTEnableImpeller"]).andReturn(@"NO");
OCMStub([mockAppBundle bundleWithIdentifier:bundleId]).andReturn(mockAppBundle);

auto settings = FLTDefaultSettingsForBundle();
// Check settings.enable_impeller value is same as the value defined in Info.plist.
XCTAssertEqual(settings.enable_impeller, NO);

[mockAppBundle stopMocking];
}

- (void)testEnableImpellerAppBundleSettingIsCorrectlyParsed {
NSString* bundleId = [FlutterDartProject defaultBundleIdentifier];
id mockAppBundle = OCMClassMock([NSBundle class]);
OCMStub([mockAppBundle objectForInfoDictionaryKey:@"FLTEnableImpeller"]).andReturn(@"YES");
OCMStub([mockAppBundle bundleWithIdentifier:bundleId]).andReturn(mockAppBundle);

// Since FLTEnableImpeller is set to false in the main bundle, this is also
// testing that setting FLTEnableImpeller in the app bundle takes
// precedence over setting it in the root bundle.

auto settings = FLTDefaultSettingsForBundle();
// Check settings.enable_impeller value is same as the value defined in Info.plist.
XCTAssertEqual(settings.enable_impeller, YES);

[mockAppBundle stopMocking];
}

- (void)testEnableTraceSystraceSettingIsCorrectlyParsed {
Expand Down