Reland "Add mock policy option to extension mixin"

The original CL had some relative FilePaths that started with a '/'.
This caused browser tests to fail when run in debug mode. This is now
fixed by deleting the leading '/' characters.

Original change's description:
> Add mock policy option to extension install mixin
>
> Add an option to initialize ExtensionForceInstallMixin with a
> MockConfigurationPolicyProvider.
> Use this option in ForceInstalledAffiliatedExtensionApiTest.
> Add mixin support to ForceInstalledAffiliatedExtensionApiTest.
>
> Bug: 1090941
> Change-Id: Ieea931c108e091679be607045f405b55f4186a57
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2416453
> Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
> Reviewed-by: David Bertoni <dbertoni@chromium.org>
> Reviewed-by: Omar Morsi <omorsi@google.com>
> Commit-Queue: Fabian Sommer <fabiansommer@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#808281}

Bug: 1090941
Change-Id: I4803c1de248b40e22a0194aa9e98182cb5ae1ed8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2540586Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: David Bertoni <dbertoni@chromium.org>
Commit-Queue: Fabian Sommer <fabiansommer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#828163}

chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc

@@ -27,8 +27,10 @@ constexpr char kAnnotatedLocation[] = "annotated_location";
 constexpr char kHostname[] = "hostname";
 
 constexpr char kTestExtensionID[] = "nbiliclbejdndfpchgkbmfoppjplbdok";
-constexpr char kUpdateManifestPath[] =
-    "/extensions/api_test/enterprise_device_attributes/update_manifest.xml";
+constexpr char kExtensionPath[] =
+    "extensions/api_test/enterprise_device_attributes/";
+constexpr char kExtensionPemPath[] =
+    "extensions/api_test/enterprise_device_attributes.pem";
 
 base::Value BuildCustomArg(const std::string& expected_directory_device_id,
                            const std::string& expected_serial_number,
@@ -98,7 +100,7 @@ IN_PROC_BROWSER_TEST_P(EnterpriseDeviceAttributesTest, Success) {
                                ->IsAffiliated());
 
   const Extension* extension =
-      ForceInstallExtension(kTestExtensionID, kUpdateManifestPath);
+      ForceInstallExtension(kExtensionPath, kExtensionPemPath);
   const GURL test_url = extension->GetResourceURL("basic.html");
 
   // Device attributes are available only for affiliated user.

chrome/browser/extensions/api/enterprise_networking_attributes/enterprise_networking_attributes_apitest.cc

@@ -24,8 +24,10 @@ const char kErrorNetworkNotConnected[] =
     "Device is not connected to a network.";
 
 constexpr char kTestExtensionID[] = "pkhdjpcjgonhlomdjmnddhbfpgkdhgle";
-constexpr char kUpdateManifestPath[] =
-    "/extensions/api_test/enterprise_networking_attributes/update_manifest.xml";
+constexpr char kExtensionPath[] =
+    "extensions/api_test/enterprise_networking_attributes/";
+constexpr char kExtensionPemPath[] =
+    "extensions/api_test/enterprise_networking_attributes.pem";
 
 constexpr char kMacAddress[] = "0123456789AB";
 constexpr char kFormattedMacAddress[] = "01:23:45:67:89:AB";
@@ -157,7 +159,7 @@ IN_PROC_BROWSER_TEST_P(EnterpriseNetworkingAttributesTest, GetNetworkDetails) {
                                ->IsAffiliated());
 
   const Extension* extension =
-      ForceInstallExtension(kTestExtensionID, kUpdateManifestPath);
+      ForceInstallExtension(kExtensionPath, kExtensionPemPath);
   SetupDisconnectedNetwork();
   const GURL test_url = extension->GetResourceURL("test.html");
 

chrome/browser/extensions/api/force_installed_affiliated_extension_apitest.cc

@@ -4,11 +4,16 @@
 
 #include "chrome/browser/extensions/api/force_installed_affiliated_extension_apitest.h"
 
+#include "base/files/file_path.h"
 #include "base/json/json_writer.h"
+#include "base/optional.h"
+#include "base/path_service.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/policy/affiliation_test_helper.h"
 #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
-#include "chrome/browser/extensions/policy_test_utils.h"
+#include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/common/chrome_paths.h"
+#include "chrome/test/base/mixin_based_in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "chromeos/dbus/session_manager/fake_session_manager_client.h"
 #include "components/prefs/pref_service.h"
@@ -29,6 +34,10 @@ constexpr char kAnotherAffiliationID[] = "another-affiliation-id";
 constexpr char kAffiliatedUserEmail[] = "user@example.com";
 constexpr char kAffiliatedUserGaiaId[] = "1029384756";
 
+base::FilePath GetTestDataDir() {
+  return base::PathService::CheckedGet(chrome::DIR_TEST_DATA);
+}
+
 }  // namespace
 
 namespace extensions {
@@ -49,16 +58,33 @@ ForceInstalledAffiliatedExtensionApiTest::
 ForceInstalledAffiliatedExtensionApiTest::
     ~ForceInstalledAffiliatedExtensionApiTest() = default;
 
+void ForceInstalledAffiliatedExtensionApiTest::SetUp() {
+  mixin_host_.SetUp();
+  ExtensionApiTest::SetUp();
+}
+
 void ForceInstalledAffiliatedExtensionApiTest::SetUpCommandLine(
     base::CommandLine* command_line) {
-  ExtensionApiTest::SetUpCommandLine(command_line);
+  mixin_host_.SetUpCommandLine(command_line);
   policy::AffiliationTestHelper::AppendCommandLineSwitchesForLoginManager(
       command_line);
+  ExtensionApiTest::SetUpCommandLine(command_line);
+}
+
+void ForceInstalledAffiliatedExtensionApiTest::SetUpDefaultCommandLine(
+    base::CommandLine* command_line) {
+  mixin_host_.SetUpDefaultCommandLine(command_line);
+  ExtensionApiTest::SetUpDefaultCommandLine(command_line);
+}
+
+bool ForceInstalledAffiliatedExtensionApiTest::SetUpUserDataDirectory() {
+  return mixin_host_.SetUpUserDataDirectory() &&
+         ExtensionApiTest::SetUpUserDataDirectory();
 }
 
 void ForceInstalledAffiliatedExtensionApiTest::
     SetUpInProcessBrowserTestFixture() {
-  ExtensionApiTest::SetUpInProcessBrowserTestFixture();
+  mixin_host_.SetUpInProcessBrowserTestFixture();
 
   // Initialize clients here so they are available during setup. They will be
   // shutdown in ChromeBrowserMain.
@@ -93,9 +119,18 @@ void ForceInstalledAffiliatedExtensionApiTest::
 
   // Set retry delay to prevent timeouts.
   policy::DeviceManagementService::SetRetryDelayForTesting(0);
+
+  ExtensionApiTest::SetUpInProcessBrowserTestFixture();
+}
+
+void ForceInstalledAffiliatedExtensionApiTest::CreatedBrowserMainParts(
+    content::BrowserMainParts* browser_main_parts) {
+  mixin_host_.CreatedBrowserMainParts(browser_main_parts);
+  ExtensionApiTest::CreatedBrowserMainParts(browser_main_parts);
 }
 
 void ForceInstalledAffiliatedExtensionApiTest::SetUpOnMainThread() {
+  mixin_host_.SetUpOnMainThread();
   // Log in user that was created with
   // policy::AffiliationTestHelper::PreLoginUser() in the PRE_ test.
   const base::ListValue* users =
@@ -104,24 +139,36 @@ void ForceInstalledAffiliatedExtensionApiTest::SetUpOnMainThread() {
     policy::AffiliationTestHelper::LoginUser(affiliated_account_id_);
   }
 
-  policy_test_utils::SetUpEmbeddedTestServer(embedded_test_server());
-  ASSERT_TRUE(embedded_test_server()->Start());
-
+  force_install_mixin_.InitWithMockPolicyProvider(profile(), &policy_provider_);
   ExtensionApiTest::SetUpOnMainThread();
 }
 
+void ForceInstalledAffiliatedExtensionApiTest::TearDownOnMainThread() {
+  mixin_host_.TearDownOnMainThread();
+  ExtensionApiTest::TearDownOnMainThread();
+}
+
+void ForceInstalledAffiliatedExtensionApiTest::
+    TearDownInProcessBrowserTestFixture() {
+  mixin_host_.TearDownInProcessBrowserTestFixture();
+  ExtensionApiTest::TearDownInProcessBrowserTestFixture();
+}
+
+void ForceInstalledAffiliatedExtensionApiTest::TearDown() {
+  mixin_host_.TearDown();
+  ExtensionApiTest::TearDown();
+}
+
 const extensions::Extension*
 ForceInstalledAffiliatedExtensionApiTest::ForceInstallExtension(
-    const extensions::ExtensionId& extension_id,
-    const std::string& update_manifest_path) {
-  policy_test_utils::SetExtensionInstallForcelistPolicy(
-      extension_id, embedded_test_server()->GetURL(update_manifest_path),
-      profile(), &policy_provider_);
-  const extensions::Extension* extension =
-      ExtensionRegistry::Get(profile())->enabled_extensions().GetByID(
-          extension_id);
-  DCHECK(extension);
-  return extension;
+    const std::string& extension_path,
+    const std::string& pem_path) {
+  extensions::ExtensionId extension_id;
+  EXPECT_TRUE(force_install_mixin_.ForceInstallFromSourceDir(
+      GetTestDataDir().AppendASCII(extension_path),
+      GetTestDataDir().AppendASCII(pem_path),
+      ExtensionForceInstallMixin::WaitMode::kLoad, &extension_id));
+  return force_install_mixin_.GetInstalledExtension(extension_id);
 }
 
 void ForceInstalledAffiliatedExtensionApiTest::TestExtension(

chrome/browser/extensions/api/force_installed_affiliated_extension_apitest.h

@@ -10,6 +10,8 @@
 #include "base/values.h"
 #include "chrome/browser/chromeos/policy/device_policy_cros_browser_test.h"
 #include "chrome/browser/extensions/extension_apitest.h"
+#include "chrome/browser/policy/extension_force_install_mixin.h"
+#include "chrome/test/base/mixin_based_in_process_browser_test.h"
 #include "chromeos/tpm/stub_install_attributes.h"
 #include "components/account_id/account_id.h"
 #include "components/policy/core/common/mock_configuration_policy_provider.h"
@@ -26,8 +28,12 @@ namespace extensions {
 
 class Extension;
 
-// TODO(https://crbug.com/1082195) Create force-installed extension and user
-// affiliation test mixins to replace this class.
+// TODO(https://crbug.com/1082195) Create user affiliation test mixin to use in
+// this class.
+
+// TODO(https://crbug.com/1129486) This class is duplicating mixin functionality
+// from MixinBasedInProcessBrowserTest. Move this into its own class and inherit
+// from it instead.
 
 // Helper class to test force-installed extensions in a
 // affiliated/non-affiliated user profile.
@@ -38,13 +44,21 @@ class ForceInstalledAffiliatedExtensionApiTest : public ExtensionApiTest {
 
  protected:
   // ExtensionApiTest
+  void SetUp() override;
   void SetUpCommandLine(base::CommandLine* command_line) override;
+  void SetUpDefaultCommandLine(base::CommandLine* command_line) override;
+  bool SetUpUserDataDirectory() override;
   void SetUpInProcessBrowserTestFixture() override;
+  void CreatedBrowserMainParts(
+      content::BrowserMainParts* browser_main_parts) override;
   void SetUpOnMainThread() override;
+  void TearDownOnMainThread() override;
+  void TearDownInProcessBrowserTestFixture() override;
+  void TearDown() override;
 
   const extensions::Extension* ForceInstallExtension(
-      const extensions::ExtensionId& extension_id,
-      const std::string& update_manifest_path);
+      const std::string& extension_path,
+      const std::string& pem_path);
 
   // Sets |custom_arg_value|, loads |page_url| and waits for an extension API
   // test pass/fail notification.
@@ -60,6 +74,8 @@ class ForceInstalledAffiliatedExtensionApiTest : public ExtensionApiTest {
   policy::MockConfigurationPolicyProvider policy_provider_;
   chromeos::ScopedStubInstallAttributes test_install_attributes_;
   policy::DevicePolicyCrosTestHelper test_helper_;
+  InProcessBrowserTestMixinHost mixin_host_;
+  ExtensionForceInstallMixin force_install_mixin_{&mixin_host_};
 };
 
 }  //  namespace extensions

chrome/browser/policy/extension_force_install_mixin.cc

@@ -29,6 +29,11 @@
 #include "chrome/browser/profiles/profile.h"
 #include "components/crx_file/crx_verifier.h"
 #include "components/crx_file/id_util.h"
+#include "components/policy/core/common/mock_configuration_policy_provider.h"
+#include "components/policy/core/common/policy_map.h"
+#include "components/policy/core/common/policy_namespace.h"
+#include "components/policy/core/common/policy_types.h"
+#include "components/policy/policy_constants.h"
 #include "components/prefs/pref_change_registrar.h"
 #include "components/prefs/pref_service.h"
 #include "extensions/browser/extension_creator.h"
@@ -325,8 +330,6 @@ bool ParseCrxInnerData(const base::FilePath& crx_path,
   return ParseExtensionManifestData(temp_dir.GetPath(), extension_version);
 }
 
-#if defined(OS_CHROMEOS)
-
 std::string MakeForceInstallPolicyItemValue(
     const extensions::ExtensionId& extension_id,
     const GURL& update_manifest_url) {
@@ -336,6 +339,35 @@ std::string MakeForceInstallPolicyItemValue(
                             update_manifest_url.spec().c_str());
 }
 
+void UpdatePolicyViaMockPolicyProvider(
+    const extensions::ExtensionId& extension_id,
+    const GURL& update_manifest_url,
+    policy::MockConfigurationPolicyProvider* mock_policy_provider) {
+  const std::string policy_item_value =
+      MakeForceInstallPolicyItemValue(extension_id, update_manifest_url);
+  policy::PolicyMap policy_map;
+  policy_map.CopyFrom(
+      mock_policy_provider->policies().Get(policy::PolicyNamespace(
+          policy::POLICY_DOMAIN_CHROME, /*component_id=*/std::string())));
+  policy::PolicyMap::Entry* const existing_entry =
+      policy_map.GetMutable(policy::key::kExtensionInstallForcelist);
+  if (existing_entry) {
+    // Append to the existing policy.
+    existing_entry->value()->Append(policy_item_value);
+  } else {
+    // Set the new policy value.
+    base::Value policy_value(base::Value::Type::LIST);
+    policy_value.Append(policy_item_value);
+    policy_map.Set(policy::key::kExtensionInstallForcelist,
+                   policy::POLICY_LEVEL_MANDATORY, policy::POLICY_SCOPE_USER,
+                   policy::POLICY_SOURCE_CLOUD, std::move(policy_value),
+                   /*external_data_fetcher=*/nullptr);
+  }
+  mock_policy_provider->UpdateChromePolicy(policy_map);
+}
+
+#if defined(OS_CHROMEOS)
+
 void UpdatePolicyViaDeviceStateMixin(
     const extensions::ExtensionId& extension_id,
     const GURL& update_manifest_url,
@@ -369,6 +401,17 @@ ExtensionForceInstallMixin::ExtensionForceInstallMixin(
 
 ExtensionForceInstallMixin::~ExtensionForceInstallMixin() = default;
 
+void ExtensionForceInstallMixin::InitWithMockPolicyProvider(
+    Profile* profile,
+    policy::MockConfigurationPolicyProvider* mock_policy_provider) {
+  DCHECK(profile);
+  DCHECK(mock_policy_provider);
+  DCHECK(!profile_) << "Init already called";
+  DCHECK(!mock_policy_provider_);
+  profile_ = profile;
+  mock_policy_provider_ = mock_policy_provider;
+}
+
 #if defined(OS_CHROMEOS)
 
 void ExtensionForceInstallMixin::InitWithDeviceStateMixin(
@@ -605,6 +648,11 @@ bool ExtensionForceInstallMixin::UpdatePolicy(
     const GURL& update_manifest_url) {
   DCHECK(profile_) << "Init not called";
 
+  if (mock_policy_provider_) {
+    UpdatePolicyViaMockPolicyProvider(extension_id, update_manifest_url,
+                                      mock_policy_provider_);
+    return true;
+  }
 #if defined(OS_CHROMEOS)
   if (device_state_mixin_) {
     UpdatePolicyViaDeviceStateMixin(extension_id, update_manifest_url,

chrome/browser/policy/extension_force_install_mixin.h

@@ -25,6 +25,10 @@ namespace extensions {
 class Extension;
 }  // namespace extensions
 
+namespace policy {
+class MockConfigurationPolicyProvider;
+}  // namespace policy
+
 #if defined(OS_CHROMEOS)
 
 namespace chromeos {
@@ -92,6 +96,10 @@ class ExtensionForceInstallMixin final : public InProcessBrowserTestMixin {
   // Use one of the Init*() methods to initialize the object before calling any
   // other method:
 
+  void InitWithMockPolicyProvider(
+      Profile* profile,
+      policy::MockConfigurationPolicyProvider* mock_policy_provider);
+
 #if defined(OS_CHROMEOS)
   void InitWithDeviceStateMixin(Profile* profile,
                                 chromeos::DeviceStateMixin* device_state_mixin);
@@ -177,6 +185,7 @@ class ExtensionForceInstallMixin final : public InProcessBrowserTestMixin {
   base::ScopedTempDir temp_dir_;
   net::EmbeddedTestServer embedded_test_server_;
   Profile* profile_ = nullptr;
+  policy::MockConfigurationPolicyProvider* mock_policy_provider_ = nullptr;
 #if defined(OS_CHROMEOS)
   chromeos::DeviceStateMixin* device_state_mixin_ = nullptr;
   policy::DevicePolicyCrosTestHelper* device_policy_cros_test_helper_ = nullptr;

chrome/test/data/extensions/api_test/enterprise_device_attributes/update_manifest.xml

-<?xml version='1.0' encoding='UTF-8'?>
-<!--
-  This update manifest points to the enterprise_device_attributes.crx file.
-  "mock.http" is a placeholder that gets substituted with the test server
-  address in runtime.
--->
-<gupdate xmlns='http://www.google.com/update2/response' protocol='2.0'>
-  <app appid='nbiliclbejdndfpchgkbmfoppjplbdok'>
-    <updatecheck
-        codebase='http://mock.http/extensions/api_test/enterprise_device_attributes.crx'
-        version='0.1' />
-   </app>
-</gupdate>

chrome/test/data/extensions/api_test/enterprise_networking_attributes/update_manifest.xml

-<?xml version='1.0' encoding='UTF-8'?>
-<!--
-  This update manifest points to the enterprise_networking_attributes.crx file.
-  "mock.http" is a placeholder that gets substituted with the test server
-  address in runtime.
--->
-<gupdate xmlns='http://www.google.com/update2/response' protocol='2.0'>
-  <app appid='pkhdjpcjgonhlomdjmnddhbfpgkdhgle'>
-    <updatecheck
-        codebase='http://mock.http/extensions/api_test/enterprise_networking_attributes.crx'
-        version='1.0' />
-   </app>
-</gupdate>