[Impeller] Respect enable-impeller command line setting over info.plist setting (#40902)

[Impeller] Respect enable-impeller command line setting over info.plist setting

Author: dnfield

shell/common/switches_unittests.cc

@@ -93,5 +93,24 @@ TEST(SwitchesTest, EnableEmbedderAPI) {
   }
 }
 
+TEST(SwitchesTest, NoEnableImpeller) {
+  {
+    // enable
+    fml::CommandLine command_line =
+        fml::CommandLineFromInitializerList({"command", "--enable-impeller"});
+    EXPECT_TRUE(command_line.HasOption("enable-impeller"));
+    Settings settings = SettingsFromCommandLine(command_line);
+    EXPECT_EQ(settings.enable_impeller, true);
+  }
+  {
+    // disable
+    fml::CommandLine command_line = fml::CommandLineFromInitializerList(
+        {"command", "--enable-impeller=false"});
+    EXPECT_TRUE(command_line.HasOption("enable-impeller"));
+    Settings settings = SettingsFromCommandLine(command_line);
+    EXPECT_EQ(settings.enable_impeller, false);
+  }
+}
+
 }  // namespace testing
 }  // namespace flutter

shell/platform/darwin/common/command_line.h

@@ -5,13 +5,16 @@
 #ifndef FLUTTER_SHELL_PLATFORM_DARWIN_COMMON_COMMAND_LINE_H_
 #define FLUTTER_SHELL_PLATFORM_DARWIN_COMMON_COMMAND_LINE_H_
 
+#import <Foundation/Foundation.h>
+
 #include "flutter/fml/command_line.h"
 
 #include "flutter/fml/macros.h"
 
 namespace flutter {
 
-fml::CommandLine CommandLineFromNSProcessInfo();
+fml::CommandLine CommandLineFromNSProcessInfo(
+    NSProcessInfo* processInfoOrNil = nil);
 
 }  // namespace flutter
 

shell/platform/darwin/common/command_line.mm

@@ -8,10 +8,11 @@
 
 namespace flutter {
 
-fml::CommandLine CommandLineFromNSProcessInfo() {
+fml::CommandLine CommandLineFromNSProcessInfo(NSProcessInfo* processInfoOrNil) {
   std::vector<std::string> args_vector;
+  auto processInfo = processInfoOrNil ? processInfoOrNil : [NSProcessInfo processInfo];
 
-  for (NSString* arg in [NSProcessInfo processInfo].arguments) {
+  for (NSString* arg in processInfo.arguments) {
     args_vector.emplace_back(arg.UTF8String);
   }
 

shell/platform/darwin/ios/framework/Source/FlutterDartProject.mm

@@ -76,11 +76,11 @@
   return [NSBundle bundleWithIdentifier:bundleID];
 }
 
-flutter::Settings FLTDefaultSettingsForBundle(NSBundle* bundle) {
-  auto command_line = flutter::CommandLineFromNSProcessInfo();
+flutter::Settings FLTDefaultSettingsForBundle(NSBundle* bundle, NSProcessInfo* processInfoOrNil) {
+  auto command_line = flutter::CommandLineFromNSProcessInfo(processInfoOrNil);
 
   // Precedence:
-  // 1. Settings from the specified NSBundle.
+  // 1. Settings from the specified NSBundle (except for enable-impeller).
   // 2. Settings passed explicitly via command-line arguments.
   // 3. Settings from the NSBundle with the default bundle ID.
   // 4. Settings from the main NSBundle and default values.
@@ -206,15 +206,21 @@
   BOOL enableWideGamut = nsEnableWideGamut ? nsEnableWideGamut.boolValue : NO;
   settings.enable_wide_gamut = enableWideGamut;
 
-  // Whether to enable Impeller. First, look in the app bundle.
-  NSNumber* enableImpeller = [bundle objectForInfoDictionaryKey:@"FLTEnableImpeller"];
-  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;
+  // TODO(dnfield): We should reverse the order for all these settings so that command line options
+  // are preferred to plist settings. https://github.com/flutter/flutter/issues/124049
+  // Whether to enable Impeller. If the command line explicitly
+  // specified an option for this, ignore what's in the plist.
+  if (!command_line.HasOption("enable-impeller")) {
+    // Next, look in the app bundle.
+    NSNumber* enableImpeller = [bundle objectForInfoDictionaryKey:@"FLTEnableImpeller"];
+    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;
+    }
   }
 
   NSNumber* enableTraceSystrace = [mainBundle objectForInfoDictionaryKey:@"FLTTraceSystrace"];

shell/platform/darwin/ios/framework/Source/FlutterDartProjectTest.mm

@@ -93,6 +93,32 @@ - (void)testEnableImpellerSettingIsCorrectlyParsed {
   [mockMainBundle stopMocking];
 }
 
+- (void)testEnableImpellerSettingIsCorrectlyOverriddenByCommandLine {
+  id mockMainBundle = OCMPartialMock([NSBundle mainBundle]);
+  OCMStub([mockMainBundle objectForInfoDictionaryKey:@"FLTEnableImpeller"]).andReturn(@"NO");
+  id mockProcessInfo = OCMPartialMock([NSProcessInfo processInfo]);
+  NSArray* arguments = @[ @"process_name", @"--enable-impeller" ];
+  OCMStub([mockProcessInfo arguments]).andReturn(arguments);
+
+  auto settings = FLTDefaultSettingsForBundle(nil, mockProcessInfo);
+  // Check settings.enable_impeller value is same as the value on command line.
+  XCTAssertEqual(settings.enable_impeller, YES);
+  [mockMainBundle stopMocking];
+}
+
+- (void)testDisableImpellerSettingIsCorrectlyOverriddenByCommandLine {
+  id mockMainBundle = OCMPartialMock([NSBundle mainBundle]);
+  OCMStub([mockMainBundle objectForInfoDictionaryKey:@"FLTEnableImpeller"]).andReturn(@"YES");
+  id mockProcessInfo = OCMPartialMock([NSProcessInfo processInfo]);
+  NSArray* arguments = @[ @"process_name", @"--enable-impeller=false" ];
+  OCMStub([mockProcessInfo arguments]).andReturn(arguments);
+
+  auto settings = FLTDefaultSettingsForBundle(nil, mockProcessInfo);
+  // Check settings.enable_impeller value is same as the value on command line.
+  XCTAssertEqual(settings.enable_impeller, NO);
+  [mockMainBundle stopMocking];
+}
+
 - (void)testDisableImpellerAppBundleSettingIsCorrectlyParsed {
   NSString* bundleId = [FlutterDartProject defaultBundleIdentifier];
   id mockAppBundle = OCMClassMock([NSBundle class]);

shell/platform/darwin/ios/framework/Source/FlutterDartProject_Internal.h

@@ -14,7 +14,8 @@ NS_ASSUME_NONNULL_BEGIN
 
 NSBundle* FLTFrameworkBundleInternal(NSString* bundleID, NSURL* searchURL);
 
-flutter::Settings FLTDefaultSettingsForBundle(NSBundle* bundle = nil);
+flutter::Settings FLTDefaultSettingsForBundle(NSBundle* _Nullable bundle = nil,
+                                              NSProcessInfo* _Nullable processInfoOrNil = nil);
 
 @interface FlutterDartProject ()