[macos] Allow engine flags via environment vars (flutter#21468)

Replaces the (temporary) compile-time option to pass engine switches
with the ability to pass them temporarily at runtime via environment
variables. This moves the recently-added code for doing this on Windows
to a shared location for use by all desktop embeddings.

This is enabled only for debug/profile to avoid potential issues with
tampering with released applications, but if there is a need for that in
the future it could be added (potentially with a whitelist, as is
currently used for Dart VM flags).

Temporarily adds a way to enable mirrors as a compile time option,
as is already provided in the Linux embedding, to provide a migration
path for the one remaining known need for compile-time options
that has been raised in flutter#38569.

Author: stuartmorgan-g

ci/licenses_golden/licenses_flutter

@@ -878,6 +878,9 @@ FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/plugin_registrar
 FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/standard_codec.cc
 FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/standard_message_codec_unittests.cc
 FILE: ../../../flutter/shell/platform/common/cpp/client_wrapper/standard_method_codec_unittests.cc
+FILE: ../../../flutter/shell/platform/common/cpp/engine_switches.cc
+FILE: ../../../flutter/shell/platform/common/cpp/engine_switches.h
+FILE: ../../../flutter/shell/platform/common/cpp/engine_switches_unittests.cc
 FILE: ../../../flutter/shell/platform/common/cpp/incoming_message_dispatcher.cc
 FILE: ../../../flutter/shell/platform/common/cpp/incoming_message_dispatcher.h
 FILE: ../../../flutter/shell/platform/common/cpp/json_message_codec.cc

shell/platform/common/cpp/BUILD.gn

@@ -2,6 +2,7 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+import("//flutter/common/config.gni")
 import("//flutter/testing/testing.gni")
 
 config("desktop_library_implementation") {
@@ -42,12 +43,25 @@ source_set("common_cpp_input") {
 
   configs += [ ":desktop_library_implementation" ]
 
+  public_configs = [ "//flutter:config" ]
+
   if (is_win) {
     # For wstring_conversion. See issue #50053.
     defines = [ "_SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING" ]
   }
 }
 
+source_set("common_cpp_switches") {
+  public = [ "engine_switches.h" ]
+
+  sources = [ "engine_switches.cc" ]
+
+  public_configs = [
+    "//flutter:config",
+    "//flutter/common:flutter_config",
+  ]
+}
+
 source_set("common_cpp") {
   public = [
     "incoming_message_dispatcher.h",
@@ -65,6 +79,8 @@ source_set("common_cpp") {
 
   configs += [ ":desktop_library_implementation" ]
 
+  public_configs = [ "//flutter:config" ]
+
   deps = [
     ":common_cpp_library_headers",
     "//flutter/shell/platform/common/cpp/client_wrapper:client_wrapper",
@@ -85,6 +101,8 @@ source_set("common_cpp_core") {
   public = [ "path_utils.h" ]
 
   sources = [ "path_utils.cc" ]
+
+  public_configs = [ "//flutter:config" ]
 }
 
 if (enable_unittests) {
@@ -115,6 +133,7 @@ if (enable_unittests) {
     testonly = true
 
     sources = [
+      "engine_switches_unittests.cc",
       "json_message_codec_unittests.cc",
       "json_method_codec_unittests.cc",
       "text_input_model_unittests.cc",
@@ -124,6 +143,7 @@ if (enable_unittests) {
       ":common_cpp",
       ":common_cpp_fixtures",
       ":common_cpp_input",
+      ":common_cpp_switches",
       "//flutter/shell/platform/common/cpp/client_wrapper:client_wrapper",
       "//flutter/shell/platform/common/cpp/client_wrapper:client_wrapper_library_stubs",
       "//flutter/testing",

shell/platform/common/cpp/engine_switches.cc

@@ -0,0 +1,41 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "flutter/shell/platform/common/cpp/engine_switches.h"
+
+#include <algorithm>
+#include <cstdlib>
+#include <iostream>
+#include <sstream>
+
+namespace flutter {
+
+std::vector<std::string> GetSwitchesFromEnvironment() {
+  std::vector<std::string> switches;
+  // Read engine switches from the environment in debug/profile. If release mode
+  // support is needed in the future, it should likely use a whitelist.
+#ifndef FLUTTER_RELEASE
+  const char* switch_count_key = "FLUTTER_ENGINE_SWITCHES";
+  const int kMaxSwitchCount = 50;
+  const char* switch_count_string = std::getenv(switch_count_key);
+  int switch_count = std::min(
+      kMaxSwitchCount, switch_count_string ? atoi(switch_count_string) : 0);
+  for (int i = 1; i <= switch_count; ++i) {
+    std::ostringstream switch_key;
+    switch_key << "FLUTTER_ENGINE_SWITCH_" << i;
+    const char* switch_value = std::getenv(switch_key.str().c_str());
+    if (switch_value) {
+      std::ostringstream switch_value_as_flag;
+      switch_value_as_flag << "--" << switch_value;
+      switches.push_back(switch_value_as_flag.str());
+    } else {
+      std::cerr << switch_count << " keys expected from " << switch_count_key
+                << ", but " << switch_key.str() << " is missing." << std::endl;
+    }
+  }
+#endif  // !FLUTTER_RELEASE
+  return switches;
+}
+
+}  // namespace flutter

shell/platform/common/cpp/engine_switches.h

@@ -0,0 +1,31 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef FLUTTER_SHELL_PLATFORM_COMMON_CPP_ENGINE_SWITCHES_H_
+#define FLUTTER_SHELL_PLATFORM_COMMON_CPP_ENGINE_SWITCHES_H_
+
+#include <string>
+#include <vector>
+
+namespace flutter {
+
+// Returns an array of engine switches suitable to pass to the embedder API
+// in FlutterProjectArgs, based on parsing variables from the environment in
+// the form:
+//   FLUTTER_ENGINE_SWITCHES=<count>
+//   FLUTTER_ENGINE_SWITCH_1=...
+//   FLUTTER_ENGINE_SWITCH_2=...
+//   ...
+// Values should match those in shell/common/switches.h
+//
+// The returned array does not include the initial dummy argument expected by
+// the embedder API, so command_line_argv should not be set directly from it.
+//
+// In release mode, not all switches from the environment will necessarily be
+// returned. See the implementation for details.
+std::vector<std::string> GetSwitchesFromEnvironment();
+
+}  // namespace flutter
+
+#endif  // FLUTTER_SHELL_PLATFORM_COMMON_CPP_ENGINE_SWITCHES_H_

shell/platform/common/cpp/engine_switches_unittests.cc

@@ -0,0 +1,88 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "flutter/shell/platform/common/cpp/engine_switches.h"
+
+#include "gtest/gtest.h"
+
+namespace flutter {
+
+namespace {
+// Sets |key=value| in the environment of this process.
+void SetEnvironmentVariable(const char* key, const char* value) {
+#ifdef _WIN32
+  _putenv_s(key, value);
+#else
+  setenv(key, value, 1);
+#endif
+}
+
+// Removes |key| from the environment of this process, if present.
+void ClearEnvironmentVariable(const char* key) {
+#ifdef _WIN32
+  _putenv_s(key, "");
+#else
+  unsetenv(key);
+#endif
+}
+}  // namespace
+
+TEST(FlutterProjectBundle, SwitchesEmpty) {
+  // Clear the main environment variable, since test order is not guaranteed.
+  ClearEnvironmentVariable("FLUTTER_ENGINE_SWITCHES");
+
+  EXPECT_EQ(GetSwitchesFromEnvironment().size(), 0U);
+}
+
+#ifdef FLUTTER_RELEASE
+TEST(FlutterProjectBundle, SwitchesIgnoredInRelease) {
+  SetEnvironmentVariable("FLUTTER_ENGINE_SWITCHES", "2");
+  SetEnvironmentVariable("FLUTTER_ENGINE_SWITCH_1", "abc");
+  SetEnvironmentVariable("FLUTTER_ENGINE_SWITCH_2", "foo=\"bar, baz\"");
+
+  std::vector<std::string> switches = GetSwitchesFromEnvironment();
+  EXPECT_EQ(switches.size(), 0U);
+}
+#endif  // FLUTTER_RELEASE
+
+#ifndef FLUTTER_RELEASE
+TEST(FlutterProjectBundle, Switches) {
+  SetEnvironmentVariable("FLUTTER_ENGINE_SWITCHES", "2");
+  SetEnvironmentVariable("FLUTTER_ENGINE_SWITCH_1", "abc");
+  SetEnvironmentVariable("FLUTTER_ENGINE_SWITCH_2", "foo=\"bar, baz\"");
+
+  std::vector<std::string> switches = GetSwitchesFromEnvironment();
+  EXPECT_EQ(switches.size(), 2U);
+  EXPECT_EQ(switches[0], "--abc");
+  EXPECT_EQ(switches[1], "--foo=\"bar, baz\"");
+}
+
+TEST(FlutterProjectBundle, SwitchesExtraValues) {
+  SetEnvironmentVariable("FLUTTER_ENGINE_SWITCHES", "1");
+  SetEnvironmentVariable("FLUTTER_ENGINE_SWITCH_1", "abc");
+  SetEnvironmentVariable("FLUTTER_ENGINE_SWITCH_2", "foo=\"bar, baz\"");
+
+  std::vector<std::string> switches = GetSwitchesFromEnvironment();
+  EXPECT_EQ(switches.size(), 1U);
+  EXPECT_EQ(switches[0], "--abc");
+}
+
+TEST(FlutterProjectBundle, SwitchesMissingValues) {
+  SetEnvironmentVariable("FLUTTER_ENGINE_SWITCHES", "4");
+  SetEnvironmentVariable("FLUTTER_ENGINE_SWITCH_1", "abc");
+  SetEnvironmentVariable("FLUTTER_ENGINE_SWITCH_2", "foo=\"bar, baz\"");
+  ClearEnvironmentVariable("FLUTTER_ENGINE_SWITCH_3");
+  SetEnvironmentVariable("FLUTTER_ENGINE_SWITCH_4", "oops");
+
+  std::vector<std::string> switches = GetSwitchesFromEnvironment();
+  EXPECT_EQ(switches.size(), 3U);
+  EXPECT_EQ(switches[0], "--abc");
+  EXPECT_EQ(switches[1], "--foo=\"bar, baz\"");
+  // The missing switch should be skipped, leaving SWITCH_4 as the third
+  // switch in the array.
+  EXPECT_EQ(switches[2], "--oops");
+}
+#endif  // !FLUTTER_RELEASE
+
+}  // namespace flutter

shell/platform/darwin/macos/BUILD.gn

@@ -67,6 +67,7 @@ source_set("flutter_framework_source") {
   sources += _flutter_framework_headers
 
   deps = [
+    "//flutter/shell/platform/common/cpp:common_cpp_switches",
     "//flutter/shell/platform/darwin/common:framework_shared",
     "//flutter/shell/platform/embedder:embedder_as_internal_library",
   ]

shell/platform/darwin/macos/framework/Headers/FlutterDartProject.h

@@ -30,15 +30,11 @@ FLUTTER_EXPORT
     NS_DESIGNATED_INITIALIZER;
 
 /**
- * Switches to pass to the Flutter engine. See
- * https://github.com/flutter/engine/blob/master/shell/common/switches.h
- * for details. Not all switches will apply to embedding mode. Switches have not stability
- * guarantee, and are subject to change without notice.
+ * If set, allows the Flutter project to use the dart:mirrors library.
  *
- * Note: This property WILL BE REMOVED in the future. If you use this property, please see
- * https://github.com/flutter/flutter/issues/38569.
+ * Deprecated: This function is temporary and will be removed in a future release.
  */
-@property(nullable) NSArray<NSString*>* engineSwitches;
+@property(nonatomic) bool enableMirrors;
 
 @end
 

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

@@ -7,6 +7,8 @@
 
 #include <vector>
 
+#include "flutter/shell/platform/common/cpp/engine_switches.h"
+
 static NSString* const kICUBundlePath = @"icudtl.dat";
 static NSString* const kAppBundleIdentifier = @"io.flutter.flutter.app";
 
@@ -67,13 +69,10 @@ - (NSString*)ICUDataPath {
   return path;
 }
 
-- (std::vector<const char*>)argv {
-  // FlutterProjectArgs expects a full argv, so when processing it for flags the first item is
-  // treated as the executable and ignored. Add a dummy value so that all provided arguments
-  // are used.
-  std::vector<const char*> arguments = {"placeholder"};
-  for (NSUInteger i = 0; i < _engineSwitches.count; ++i) {
-    arguments.push_back([_engineSwitches[i] UTF8String]);
+- (std::vector<std::string>)switches {
+  std::vector<std::string> arguments = flutter::GetSwitchesFromEnvironment();
+  if (self.enableMirrors) {
+    arguments.push_back("--dart-flags=--enable_mirrors=true");
   }
   return arguments;
 }

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

@@ -7,6 +7,7 @@
 
 #import "flutter/shell/platform/darwin/macos/framework/Headers/FlutterDartProject.h"
 
+#include <string>
 #include <vector>
 
 /**
@@ -26,12 +27,8 @@
 
 /**
  * The command line arguments array for the engine.
- *
- * WARNING: The pointers in this array are valid only until the next call to set `engineSwitches`.
- * The returned vector should be used immediately, then discarded. It is returned this way for
- * ease of use with FlutterProjectArgs.
  */
-@property(nonatomic, readonly) std::vector<const char*> argv;
+@property(nonatomic, readonly) std::vector<std::string> switches;
 
 /**
  * Instead of looking up the assets and ICU data path in the application bundle, this initializer

shell/platform/darwin/macos/framework/Source/FlutterEngine.mm

@@ -6,6 +6,7 @@
 #import "flutter/shell/platform/darwin/macos/framework/Headers/FlutterEngine.h"
 #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h"
 
+#include <algorithm>
 #include <vector>
 
 #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterDartProject_Internal.h"
@@ -236,13 +237,20 @@ - (BOOL)runWithEntrypoint:(NSString*)entrypoint {
 
   // TODO(stuartmorgan): Move internal channel registration from FlutterViewController to here.
 
+  // FlutterProjectArgs is expecting a full argv, so when processing it for
+  // flags the first item is treated as the executable and ignored. Add a dummy
+  // value so that all provided arguments are used.
+  std::vector<std::string> switches = _project.switches;
+  std::vector<const char*> argv = {"placeholder"};
+  std::transform(switches.begin(), switches.end(), std::back_inserter(argv),
+                 [](const std::string& arg) -> const char* { return arg.c_str(); });
+
   FlutterProjectArgs flutterArguments = {};
   flutterArguments.struct_size = sizeof(FlutterProjectArgs);
   flutterArguments.assets_path = _project.assetsPath.UTF8String;
   flutterArguments.icu_data_path = _project.ICUDataPath.UTF8String;
-  std::vector<const char*> arguments = _project.argv;
-  flutterArguments.command_line_argc = static_cast<int>(arguments.size());
-  flutterArguments.command_line_argv = &arguments[0];
+  flutterArguments.command_line_argc = static_cast<int>(argv.size());
+  flutterArguments.command_line_argv = argv.size() > 0 ? argv.data() : nullptr;
   flutterArguments.platform_message_callback = (FlutterPlatformMessageCallback)OnPlatformMessage;
   flutterArguments.custom_dart_entrypoint = entrypoint.UTF8String;
   static size_t sTaskRunnerIdentifiers = 0;