[BrowserSwitcher] Migrate extensions policies to the Chrome namespace

Also, migrate the BrowserSwitcherEnabled policy in the other direction,
as "browser_switcher_enabled".

This CL refactors ChromeExtensionPolicyMigrator to make it easier to
transform values (e.g. convert types) as they are being migrated.

Bug: 909880
Change-Id: I0d49bd9296a096da713a2d6cface9eab724b29d9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1500203
Commit-Queue: Nicolas Ouellet-Payeur <nicolaso@chromium.org>
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#637862}

chrome/browser/BUILD.gn

@@ -3443,6 +3443,8 @@ jumbo_split_static_library("browser") {
     sources += [
       "badging/badge_manager_delegate_win.cc",
       "badging/badge_manager_delegate_win.h",
+      "browser_switcher/browser_switcher_policy_migrator.cc",
+      "browser_switcher/browser_switcher_policy_migrator.h",
       "browser_switcher/browser_switcher_service_win.cc",
       "browser_switcher/browser_switcher_service_win.h",
       "downgrade/user_data_downgrade.cc",

chrome/browser/browser_switcher/browser_switcher_policy_migrator.cc

+// Copyright 2019 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 "chrome/browser/browser_switcher/browser_switcher_policy_migrator.h"
+
+#include <string>
+#include <utility>
+
+#include "base/bind.h"
+#include "components/policy/core/common/policy_bundle.h"
+#include "components/policy/core/common/policy_map.h"
+#include "components/policy/core/common/policy_namespace.h"
+#include "components/policy/policy_constants.h"
+#include "extensions/common/hashed_extension_id.h"
+
+namespace browser_switcher {
+
+namespace {
+
+const char kLBSExtensionId[] = "heildphpnddilhkemkielfhnkaagiabh";
+
+void SecondsToMilliseconds(base::Value* val) {
+  const int ms_per_second = 1000;
+  *val = base::Value(val->GetInt() * ms_per_second);
+}
+
+// Transforms the string policy to a list policy (containing 1 string).
+//
+// The LBS extension's command-line parameter policies are single strings,
+// because on Windows the command-line parameters are passed as a single string
+// to the program. The parameters are parsed by the program, not the shell.
+//
+// On other platforms though, parameter parsing is done by the shell, not the
+// program. So the new policies are string-lists that are given to the program
+// pre-parsed. This is why we need to convert the string to a list, when
+// migrating from the old policy.
+void StringToList(base::Value* val) {
+  std::string str = val->GetString();
+  *val = base::Value(base::Value::Type::LIST);
+  val->GetList().push_back(base::Value(std::move(str)));
+}
+
+}  // namespace
+
+BrowserSwitcherPolicyMigrator::BrowserSwitcherPolicyMigrator() = default;
+BrowserSwitcherPolicyMigrator::~BrowserSwitcherPolicyMigrator() = default;
+
+void BrowserSwitcherPolicyMigrator::Migrate(policy::PolicyBundle* bundle) {
+  policy::PolicyMap& extension_map = bundle->Get(policy::PolicyNamespace(
+      policy::POLICY_DOMAIN_EXTENSIONS, kLBSExtensionId));
+  policy::PolicyMap& chrome_map =
+      bundle->Get(policy::PolicyNamespace(policy::POLICY_DOMAIN_CHROME, ""));
+  if (extension_map.empty())
+    return;
+
+  const auto* entry = chrome_map.Get("BrowserSwitcherEnabled");
+  if (!entry || !entry->value || !entry->value->GetBool())
+    return;
+  extension_map.Set("browser_switcher_enabled", entry->DeepCopy());
+
+  using Migration = policy::ExtensionPolicyMigrator::Migration;
+  const Migration migrations[] = {
+      Migration("alternative_browser_path",
+                policy::key::kAlternativeBrowserPath),
+      Migration("chrome_path", policy::key::kBrowserSwitcherChromePath),
+      Migration("url_list", policy::key::kBrowserSwitcherUrlList),
+      Migration("url_greylist", policy::key::kBrowserSwitcherUrlGreylist),
+      Migration("keep_last_chrome_tab",
+                policy::key::kBrowserSwitcherKeepLastChromeTab),
+      Migration("use_ie_site_list", policy::key::kBrowserSwitcherUseIeSitelist),
+      Migration("show_transition_screen", policy::key::kBrowserSwitcherDelay,
+                base::BindRepeating(&SecondsToMilliseconds)),
+      Migration("chrome_arguments",
+                policy::key::kBrowserSwitcherChromeParameters,
+                base::BindRepeating(&StringToList)),
+      Migration("alternative_browser_arguments",
+                policy::key::kAlternativeBrowserParameters,
+                base::BindRepeating(&StringToList)),
+  };
+
+  CopyPoliciesIfUnset(bundle,
+                      extensions::HashedExtensionId(kLBSExtensionId).value(),
+                      migrations);
+}
+
+}  // namespace browser_switcher

chrome/browser/browser_switcher/browser_switcher_policy_migrator.h

+// Copyright 2019 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.
+
+#ifndef CHROME_BROWSER_BROWSER_SWITCHER_BROWSER_SWITCHER_POLICY_MIGRATOR_H_
+#define CHROME_BROWSER_BROWSER_SWITCHER_BROWSER_SWITCHER_POLICY_MIGRATOR_H_
+
+#include "chrome/browser/policy/chrome_extension_policy_migrator.h"
+
+namespace browser_switcher {
+
+class BrowserSwitcherPolicyMigrator
+    : public policy::ChromeExtensionPolicyMigrator {
+ public:
+  BrowserSwitcherPolicyMigrator();
+  ~BrowserSwitcherPolicyMigrator() override;
+
+  void Migrate(policy::PolicyBundle* bundle) override;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(BrowserSwitcherPolicyMigrator);
+};
+
+}  // namespace browser_switcher
+
+#endif  // CHROME_BROWSER_BROWSER_SWITCHER_BROWSER_SWITCHER_POLICY_MIGRATOR_H_

chrome/browser/policy/chrome_browser_policy_connector.cc

@@ -51,11 +51,20 @@
 #include "components/policy/core/common/cloud/machine_level_user_cloud_policy_manager.h"
 #endif
 
+#if defined(OS_WIN)
+#include "chrome/browser/browser_switcher/browser_switcher_policy_migrator.h"
+#endif
+
 namespace policy {
 
 namespace {
 
-void AddMigrators(ConfigurationPolicyProvider* provider) {}
+void AddMigrators(ConfigurationPolicyProvider* provider) {
+#if defined(OS_WIN)
+  provider->AddMigrator(
+      std::make_unique<browser_switcher::BrowserSwitcherPolicyMigrator>());
+#endif
+}
 
 bool ProviderHasPolicies(const ConfigurationPolicyProvider* provider) {
   if (!provider)

chrome/browser/policy/chrome_extension_policy_migrator.cc

@@ -41,7 +41,9 @@ void ChromeExtensionPolicyMigrator::CopyPoliciesIfUnset(
     PolicyMap::Entry* entry = extension_map->GetMutable(migration.old_name);
     if (entry) {
       if (!chrome_map.Get(migration.new_name)) {
-        chrome_map.Set(migration.new_name, entry->DeepCopy());
+        auto new_entry = entry->DeepCopy();
+        migration.transform.Run(new_entry.value.get());
+        chrome_map.Set(migration.new_name, std::move(new_entry));
       }
       // TODO(crbug/869958): Mark the old policy as deprecated for
       // chrome://policy.

chrome/browser/policy/chrome_extension_policy_migrator_unittest.cc

@@ -21,21 +21,23 @@ const char kOldPolicy1[] = "OldPolicyOne";
 const char kOldPolicy2[] = "OldPolicyTwo";
 const char kOldPolicy3[] = "OldPolicyThree";
 const char kOldPolicy4[] = "OldPolicyFour";
+const char kOldPolicy5[] = "OldPolicyFive";
 const char kNewPolicy1[] = "NewPolicyOne";
 const char kNewPolicy2[] = "NewPolicyTwo";
 const char kNewPolicy3[] = "NewPolicyThree";
+const char kNewPolicy4[] = "NewPolicyFour";
 
 const int kOldValue1 = 111;
 const int kOldValue2 = 222;
 const int kOldValue3 = 333;
 const int kOldValue4 = 444;
+const int kOldValue5 = 555;
 const int kNewValue3 = 999;
+const int kNewValue4 = 888;
 
-const ExtensionPolicyMigrator::Migration kMigrations[] = {
-    {kOldPolicy1, kNewPolicy1},
-    {kOldPolicy2, kNewPolicy2},
-    {kOldPolicy3, kNewPolicy3},
-};
+void MultiplyByTwo(base::Value* val) {
+  *val = base::Value(val->GetInt() * 2);
+}
 
 void SetPolicy(PolicyMap* policy,
                const char* policy_name,
@@ -47,9 +49,17 @@ void SetPolicy(PolicyMap* policy,
 class TestingPolicyMigrator : public ChromeExtensionPolicyMigrator {
  public:
   void Migrate(PolicyBundle* bundle) override {
+    using Migration = ExtensionPolicyMigrator::Migration;
+    const Migration migrations[] = {
+        Migration(kOldPolicy1, kNewPolicy1),
+        Migration(kOldPolicy2, kNewPolicy2),
+        Migration(kOldPolicy3, kNewPolicy3),
+        Migration(kOldPolicy4, kNewPolicy4,
+                  base::BindRepeating(&MultiplyByTwo)),
+    };
     CopyPoliciesIfUnset(bundle,
                         extensions::HashedExtensionId(kExtensionId).value(),
-                        kMigrations);
+                        migrations);
   }
 };
 
@@ -73,11 +83,13 @@ TEST(ChromeExtensionPolicyMigratorTest, CopyPoliciesIfUnset) {
             std::make_unique<base::Value>(kOldValue3));
   SetPolicy(&extension_map, kOldPolicy4,
             std::make_unique<base::Value>(kOldValue4));
+  SetPolicy(&extension_map, kOldPolicy5,
+            std::make_unique<base::Value>(kOldValue5));
 
   TestingPolicyMigrator().Migrate(&bundle);
 
   // Policies in kMigrations should be renamed + copied into the Chrome domain.
-  EXPECT_EQ(3u, chrome_map.size());
+  EXPECT_EQ(4u, chrome_map.size());
   ASSERT_TRUE(chrome_map.GetValue(kNewPolicy1));
   EXPECT_EQ(base::Value(kOldValue1), *chrome_map.GetValue(kNewPolicy1));
   ASSERT_TRUE(chrome_map.GetValue(kNewPolicy2));
@@ -85,6 +97,9 @@ TEST(ChromeExtensionPolicyMigratorTest, CopyPoliciesIfUnset) {
   // kNewPolicy3 is already set, and should not be overwritten.
   ASSERT_TRUE(chrome_map.GetValue(kNewPolicy3));
   EXPECT_EQ(base::Value(kNewValue3), *chrome_map.GetValue(kNewPolicy3));
+  // This policy was transformed by MultiplyByTwo.
+  ASSERT_TRUE(chrome_map.GetValue(kNewPolicy4));
+  EXPECT_EQ(base::Value(kNewValue4), *chrome_map.GetValue(kNewPolicy4));
 }
 
 }  // namespace policy

components/policy/core/common/BUILD.gn

@@ -90,6 +90,7 @@ jumbo_source_set("internal") {
     "config_dir_policy_loader.h",
     "configuration_policy_provider.cc",
     "configuration_policy_provider.h",
+    "extension_policy_migrator.cc",
     "extension_policy_migrator.h",
     "external_data_fetcher.cc",
     "external_data_fetcher.h",

components/policy/core/common/extension_policy_migrator.cc

+// Copyright 2019 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 "components/policy/core/common/extension_policy_migrator.h"
+
+#include <algorithm>
+#include <utility>
+
+#include "base/bind.h"
+
+namespace policy {
+
+namespace {
+
+void DoNothing(base::Value* val) {}
+
+}  // namespace
+
+ExtensionPolicyMigrator::Migration::Migration(Migration&&) = default;
+
+ExtensionPolicyMigrator::Migration::Migration(const char* old_name_,
+                                              const char* new_name_)
+    : Migration(old_name_, new_name_, base::BindRepeating(&DoNothing)) {}
+
+ExtensionPolicyMigrator::Migration::Migration(const char* old_name_,
+                                              const char* new_name_,
+                                              ValueTransform transform_)
+    : old_name(old_name_),
+      new_name(new_name_),
+      transform(std::move(transform_)) {}
+
+ExtensionPolicyMigrator::Migration::~Migration() = default;
+
+}  // namespace policy

components/policy/core/common/extension_policy_migrator.h

@@ -5,7 +5,11 @@
 #ifndef COMPONENTS_POLICY_CORE_COMMON_EXTENSION_POLICY_MIGRATOR_H_
 #define COMPONENTS_POLICY_CORE_COMMON_EXTENSION_POLICY_MIGRATOR_H_
 
+#include <memory>
+
+#include "base/callback.h"
 #include "base/containers/span.h"
+#include "base/values.h"
 #include "components/policy/core/common/policy_bundle.h"
 #include "components/policy/policy_export.h"
 
@@ -27,11 +31,24 @@ class POLICY_EXPORT ExtensionPolicyMigrator {
 
   // Indicates how to rename a policy when migrating from the extension domain
   // to the Chrome domain.
-  struct Migration {
+  struct POLICY_EXPORT Migration {
+    using ValueTransform = base::RepeatingCallback<void(base::Value*)>;
+
+    Migration(Migration&&);
+    Migration(const char* old_name_, const char* new_name_);
+    Migration(const char* old_name_,
+              const char* new_name_,
+              ValueTransform transform_);
+    ~Migration();
+
     // Old name for the policy, in the extension domain.
     const char* old_name;
     // New name for the policy, in the Chrome domain.
     const char* new_name;
+    // Function to use to convert values from the old namespace to the new
+    // namespace (e.g. convert value types). It should mutate the Value in
+    // place. By default, it does no transform.
+    ValueTransform transform;
   };
 };