Add support to chrome://flags for command line flags that need to be …

…treated as a list of origins

This CL adds support for command line flags that are lists of url::Origins. E.g. The value in --flag=value will now be modifyable from the chrome://flags page using a free form textbox, and will be treated as a comma separated list of origins (e.g. http://example1.test,http://example2.test)

The string from the textbox is transformed as follows:
- Continuous whitespace characters are collapsed into single a space
- String is tokenized using space and comma as delimiters
- Each token is parsed as a GURL. Invalid URLs or URLs with a scheme other than http and https are discarded.
- Remaining URLs are converted to url::Origins, then joined into a single, comma separated string.

The CL also adds --unsafely-treat-insecure-origin-as-secure as the first such flag to chrome://flags. Developers will now be able to modify the list of insecure origins treated as secure from the chrome://flags page on all platforms.

Bug: 834381
Change-Id: Iad44b5b2724687c7bea1ae45c23ccc910eb5cc9f
Reviewed-on: https://chromium-review.googlesource.com/1038152
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Mattias Nissler <mnissler@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Edward Jung <edwardjung@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Mustafa Emre Acer <meacer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560791}

chrome/browser/about_flags.cc

@@ -3885,6 +3885,11 @@ const FeatureEntry kFeatureEntries[] = {
      flag_descriptions::kWebSocketHandshakeReuseConnectionDescription, kOsAll,
      FEATURE_VALUE_TYPE(net::WebSocketBasicHandshakeStream::
                             kWebSocketHandshakeReuseConnection)},
+    {"unsafely-treat-insecure-origin-as-secure",
+     flag_descriptions::kTreatInsecureOriginAsSecureName,
+     flag_descriptions::kTreatInsecureOriginAsSecureDescription, kOsAll,
+     ORIGIN_LIST_VALUE_TYPE(switches::kUnsafelyTreatInsecureOriginAsSecure,
+                            "")},
 
     // NOTE: Adding a new flag requires adding a corresponding entry to enum
     // "LoginCustomFlags" in tools/metrics/histograms/enums.xml. See "Flag
@@ -4052,6 +4057,13 @@ void SetFeatureEntryEnabled(flags_ui::FlagsStorage* flags_storage,
       flags_storage, internal_name, enable);
 }
 
+void SetOriginListFlag(const std::string& internal_name,
+                       const std::string& value,
+                       flags_ui::FlagsStorage* flags_storage) {
+  FlagsStateSingleton::GetFlagsState()->SetOriginListFlag(internal_name, value,
+                                                          flags_storage);
+}
+
 void RemoveFlagsSwitches(base::CommandLine::SwitchMap* switch_list) {
   FlagsStateSingleton::GetFlagsState()->RemoveFlagsSwitches(switch_list);
 }

chrome/browser/about_flags.h

@@ -72,6 +72,17 @@ void SetFeatureEntryEnabled(flags_ui::FlagsStorage* flags_storage,
                             const std::string& internal_name,
                             bool enable);
 
+// Sets a flag value with a list of origins given by |value|. Origins in |value|
+// can be separated by a comma or whitespace. Invalid URLs will be dropped when
+// setting the command line flag.
+// E.g. SetOriginListFlag("test-flag",
+//                        "http://example.test1 http://example.test2",
+//                        flags_storage);
+// will add --test-flag=http://example.test to the command line.
+void SetOriginListFlag(const std::string& internal_name,
+                       const std::string& value,
+                       flags_ui::FlagsStorage* flags_storage);
+
 // Removes all switches that were added to a command line by a previous call to
 // |ConvertFlagsToSwitches()|.
 void RemoveFlagsSwitches(base::CommandLine::SwitchMap* switch_list);

chrome/browser/about_flags_browsertest.cc

@@ -0,0 +1,141 @@
+// Copyright 2018 The Chromium 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 "base/command_line.h"
+#include "base/strings/stringprintf.h"
+#include "base/strings/utf_string_conversions.h"
+#include "build/build_config.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/interactive_test_utils.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/test/browser_test_utils.h"
+#include "ui/base/window_open_disposition.h"
+
+namespace {
+
+const char kSwitchName[] = "unsafely-treat-insecure-origin-as-secure";
+
+void SimulateTextType(content::WebContents* contents,
+                      const char* experiment_id,
+                      const char* text) {
+  EXPECT_TRUE(content::ExecuteScript(
+      contents, base::StringPrintf(
+                    "var parent = document.getElementById('%s');"
+                    "var textarea = parent.getElementsByTagName('textarea')[0];"
+                    "textarea.focus();"
+                    "textarea.value = `%s`;"
+                    "textarea.onchange();",
+                    experiment_id, text)));
+}
+
+void ToggleEnableDropdown(content::WebContents* contents,
+                          const char* experiment_id,
+                          bool enable) {
+  EXPECT_TRUE(content::ExecuteScript(
+      contents,
+      base::StringPrintf(
+          "var k = "
+          "document.getElementById('%s');"
+          "var s = k.getElementsByClassName('experiment-enable-disable')[0];"
+          "s.focus();"
+          "s.selectedIndex = %d;"
+          "s.onchange();",
+          experiment_id, enable ? 1 : 0)));
+}
+
+void SetSwitch(base::CommandLine::SwitchMap* switch_map,
+               const std::string& switch_name,
+               const std::string& switch_value) {
+#if defined(OS_WIN)
+  (*switch_map)[switch_name] = base::ASCIIToUTF16(switch_value.c_str());
+#else
+  (*switch_map)[switch_name] = switch_value;
+#endif
+}
+
+class AboutFlagsBrowserTest : public InProcessBrowserTest {};
+
+// Tests experiments with origin values in chrome://flags page.
+IN_PROC_BROWSER_TEST_F(AboutFlagsBrowserTest, OriginFlag) {
+  ui_test_utils::NavigateToURL(browser(), GURL("chrome://flags"));
+
+  const base::CommandLine::SwitchMap switches =
+      base::CommandLine::ForCurrentProcess()->GetSwitches();
+
+  content::WebContents* contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+
+  // Type a value in the experiment's textarea. Since the flag state is
+  // "Disabled" by default, command line shouldn't change.
+  SimulateTextType(contents, kSwitchName, "http://example.test");
+  EXPECT_EQ(switches, base::CommandLine::ForCurrentProcess()->GetSwitches());
+
+  // Enable the experiment. Command line should change.
+  ToggleEnableDropdown(contents, kSwitchName, true);
+  base::CommandLine::SwitchMap expected_switches = switches;
+  SetSwitch(&expected_switches, kSwitchName, "http://example.test");
+  EXPECT_EQ(expected_switches,
+            base::CommandLine::ForCurrentProcess()->GetSwitches());
+
+  // Typing while enabled should immediately change the flag.
+  SimulateTextType(contents, kSwitchName, "http://example.test.com");
+  SetSwitch(&expected_switches, kSwitchName, "http://example.test.com");
+  EXPECT_EQ(expected_switches,
+            base::CommandLine::ForCurrentProcess()->GetSwitches());
+
+  // Disable the experiment. Command line switch should be cleared.
+  ToggleEnableDropdown(contents, kSwitchName, false);
+  expected_switches.erase(kSwitchName);
+  EXPECT_EQ(expected_switches,
+            base::CommandLine::ForCurrentProcess()->GetSwitches());
+
+  // Enable again. Command line switch should be added back.
+  ToggleEnableDropdown(contents, kSwitchName, true);
+  SetSwitch(&expected_switches, kSwitchName, "http://example.test.com");
+  EXPECT_EQ(expected_switches,
+            base::CommandLine::ForCurrentProcess()->GetSwitches());
+
+  // Disable again and type. Command line switch should stay cleared.
+  ToggleEnableDropdown(contents, kSwitchName, false);
+  SimulateTextType(contents, kSwitchName, "http://example.test2.com");
+  expected_switches.erase(kSwitchName);
+  EXPECT_EQ(expected_switches,
+            base::CommandLine::ForCurrentProcess()->GetSwitches());
+
+  // Enable one last time. Command line should pick up the last typed value.
+  ToggleEnableDropdown(contents, kSwitchName, true);
+  SetSwitch(&expected_switches, kSwitchName, "http://example.test2.com");
+  EXPECT_EQ(expected_switches,
+            base::CommandLine::ForCurrentProcess()->GetSwitches());
+}
+
+// Tests that only valid http and https origins should be added to the command
+// line when modified from chrome://flags.
+IN_PROC_BROWSER_TEST_F(AboutFlagsBrowserTest, StringFlag) {
+  ui_test_utils::NavigateToURL(browser(), GURL("chrome://flags"));
+
+  const base::CommandLine::SwitchMap switches =
+      base::CommandLine::ForCurrentProcess()->GetSwitches();
+
+  content::WebContents* contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+
+  const char kValue[] =
+      "http://example.test/path    http://example2.test/?query\n"
+      "invalid-value, filesystem:http://example.test.file, "
+      "ws://example3.test http://&^.com";
+
+  ToggleEnableDropdown(contents, kSwitchName, true);
+  SimulateTextType(contents, kSwitchName, kValue);
+  base::CommandLine::SwitchMap expected_switches = switches;
+  SetSwitch(&expected_switches, kSwitchName,
+            "http://example.test,http://example2.test,ws://example3.test");
+  EXPECT_EQ(expected_switches,
+            base::CommandLine::ForCurrentProcess()->GetSwitches());
+}
+
+}  // namespace

chrome/browser/about_flags_unittest.cc

@@ -182,6 +182,9 @@ std::set<std::string> GetAllSwitchesAndFeaturesForTesting() {
       case flags_ui::FeatureEntry::SINGLE_DISABLE_VALUE:
         result.insert(entry.command_line_switch);
         break;
+      case flags_ui::FeatureEntry::ORIGIN_LIST_VALUE:
+        // Do nothing, origin list values are not added as feature flags.
+        break;
       case flags_ui::FeatureEntry::MULTI_VALUE:
         for (int j = 0; j < entry.num_options; ++j) {
           result.insert(entry.ChoiceForOption(j).command_line_switch);

chrome/browser/flag_descriptions.cc

@@ -1708,6 +1708,14 @@ const char kTranslateRankerEnforcementDescription[] =
     "Improved Translate UI triggering logic. TranslateRanker decides whether "
     "or not Translate UI should be triggered in a given context.";
 
+const char kTreatInsecureOriginAsSecureName[] =
+    "Insecure origins treated as secure";
+const char kTreatInsecureOriginAsSecureDescription[] =
+    "Treat given (insecure) origins as secure origins. Multiple origins can be "
+    "supplied as a comma-separated list. For the definition of secure "
+    "contexts, "
+    "see https://w3c.github.io/webappsec-secure-contexts/";
+
 const char kTrySupportedChannelLayoutsName[] =
     "Causes audio output streams to check if channel layouts other than the "
     "default hardware layout are available.";

chrome/browser/flag_descriptions.h

@@ -1043,6 +1043,9 @@ extern const char kTraceUploadUrlChoiceTesting[];
 extern const char kTranslateRankerEnforcementName[];
 extern const char kTranslateRankerEnforcementDescription[];
 
+extern const char kTreatInsecureOriginAsSecureName[];
+extern const char kTreatInsecureOriginAsSecureDescription[];
+
 extern const char kTrySupportedChannelLayoutsName[];
 extern const char kTrySupportedChannelLayoutsDescription[];
 

chrome/browser/ui/webui/flags_ui.cc

@@ -113,6 +113,9 @@ class FlagsDOMHandler : public WebUIMessageHandler {
   // Callback for the "enableExperimentalFeature" message.
   void HandleEnableExperimentalFeatureMessage(const base::ListValue* args);
 
+  // Callback for the "setOriginListFlag" message.
+  void HandleSetOriginListFlagMessage(const base::ListValue* args);
+
   // Callback for the "restartBrowser" message. Restores all tabs on restart.
   void HandleRestartBrowser(const base::ListValue* args);
 
@@ -137,6 +140,10 @@ void FlagsDOMHandler::RegisterMessages() {
       base::BindRepeating(
           &FlagsDOMHandler::HandleEnableExperimentalFeatureMessage,
           base::Unretained(this)));
+  web_ui()->RegisterMessageCallback(
+      flags_ui::kSetOriginListFlag,
+      base::BindRepeating(&FlagsDOMHandler::HandleSetOriginListFlagMessage,
+                          base::Unretained(this)));
   web_ui()->RegisterMessageCallback(
       flags_ui::kRestartBrowser,
       base::BindRepeating(&FlagsDOMHandler::HandleRestartBrowser,
@@ -210,6 +217,26 @@ void FlagsDOMHandler::HandleEnableExperimentalFeatureMessage(
                                       enable_str == "true");
 }
 
+void FlagsDOMHandler::HandleSetOriginListFlagMessage(
+    const base::ListValue* args) {
+  DCHECK(flags_storage_);
+  if (args->GetSize() != 2) {
+    NOTREACHED();
+    return;
+  }
+
+  std::string entry_internal_name;
+  std::string value_str;
+  if (!args->GetString(0, &entry_internal_name) ||
+      !args->GetString(1, &value_str) || entry_internal_name.empty()) {
+    NOTREACHED();
+    return;
+  }
+
+  about_flags::SetOriginListFlag(entry_internal_name, value_str,
+                                 flags_storage_.get());
+}
+
 void FlagsDOMHandler::HandleRestartBrowser(const base::ListValue* args) {
   DCHECK(flags_storage_);
 #if defined(OS_CHROMEOS)

chrome/test/BUILD.gn

@@ -4583,6 +4583,7 @@ if (!is_android) {
 
   test("interactive_ui_tests") {
     sources = [
+      "../browser/about_flags_browsertest.cc",
       "../browser/apps/app_browsertest_util.cc",
       "../browser/apps/app_browsertest_util.h",
       "../browser/apps/app_pointer_lock_interactive_uitest.cc",

components/flags_ui/BUILD.gn

@@ -26,6 +26,7 @@ static_library("flags_ui") {
     "//components/strings",
     "//components/variations",
     "//ui/base",
+    "//url",
   ]
 }
 

components/flags_ui/DEPS

@@ -4,4 +4,5 @@ include_rules = [
   "+components/strings/grit/components_strings.h",
   "+components/variations",
   "+ui/base",
+  "+url",
 ]

components/flags_ui/feature_entry.h

@@ -73,6 +73,10 @@ struct FeatureEntry {
     // feature is overriden to be enabled and empty set of parameters is used
     // boiling down to the default behavior in the code.
     FEATURE_WITH_PARAMS_VALUE,
+
+    // Corresponds to a command line switch where the value is treatead as a
+    // list of url::Origins. Default state is disabled like SINGLE_VALUE.
+    ORIGIN_LIST_VALUE
   };
 
   // Describes state of a feature.

components/flags_ui/feature_entry_macros.h

@@ -15,6 +15,9 @@
       nullptr, nullptr, nullptr, 0, nullptr, nullptr, nullptr
 #define SINGLE_VALUE_TYPE(command_line_switch) \
   SINGLE_VALUE_TYPE_AND_VALUE(command_line_switch, "")
+#define ORIGIN_LIST_VALUE_TYPE(command_line_switch, switch_value) \
+  flags_ui::FeatureEntry::ORIGIN_LIST_VALUE, command_line_switch, \
+      switch_value, nullptr, nullptr, nullptr, 0, nullptr, nullptr, nullptr
 #define SINGLE_DISABLE_VALUE_TYPE_AND_VALUE(command_line_switch, switch_value) \
   flags_ui::FeatureEntry::SINGLE_DISABLE_VALUE, command_line_switch,           \
       switch_value, nullptr, nullptr, nullptr, 0, nullptr, nullptr, nullptr