Revert "Add mock policy option to extension install mixin"

This reverts commit 34905419.

Reason for revert: Causes crash in tests on linux-chromeos-dbg.
First failing build:
https://ci.chromium.org/p/chromium/builders/ci/linux-chromeos-dbg/20491

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}

TBR=emaxx@chromium.org,dbertoni@chromium.org,omorsi@google.com,fabiansommer@chromium.org

Change-Id: I1b888be0d90a6ef99ac4f9e6c431296062bdb5cc
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1090941
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2419237Reviewed-by: Fabrice de Gans-Riberi <fdegans@chromium.org>
Commit-Queue: Fabrice de Gans-Riberi <fdegans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#808391}

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

@@ -27,10 +27,8 @@ constexpr char kAnnotatedLocation[] = "annotated_location";
 constexpr char kHostname[] = "hostname";
 
 constexpr char kTestExtensionID[] = "nbiliclbejdndfpchgkbmfoppjplbdok";
-constexpr char kExtensionPath[] =
-    "/extensions/api_test/enterprise_device_attributes/";
-constexpr char kExtensionPemPath[] =
-    "/extensions/api_test/enterprise_device_attributes.pem";
+constexpr char kUpdateManifestPath[] =
+    "/extensions/api_test/enterprise_device_attributes/update_manifest.xml";
 
 base::Value BuildCustomArg(const std::string& expected_directory_device_id,
                            const std::string& expected_serial_number,
@@ -100,7 +98,7 @@ IN_PROC_BROWSER_TEST_P(EnterpriseDeviceAttributesTest, Success) {
                                ->IsAffiliated());
 
   const Extension* extension =
-      ForceInstallExtension(kExtensionPath, kExtensionPemPath);
+      ForceInstallExtension(kTestExtensionID, kUpdateManifestPath);
   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,10 +24,8 @@ const char kErrorNetworkNotConnected[] =
     "Device is not connected to a network.";
 
 constexpr char kTestExtensionID[] = "pkhdjpcjgonhlomdjmnddhbfpgkdhgle";
-constexpr char kExtensionPath[] =
-    "/extensions/api_test/enterprise_networking_attributes/";
-constexpr char kExtensionPemPath[] =
-    "/extensions/api_test/enterprise_networking_attributes.pem";
+constexpr char kUpdateManifestPath[] =
+    "/extensions/api_test/enterprise_networking_attributes/update_manifest.xml";
 
 constexpr char kMacAddress[] = "0123456789AB";
 constexpr char kFormattedMacAddress[] = "01:23:45:67:89:AB";
@@ -159,7 +157,7 @@ IN_PROC_BROWSER_TEST_P(EnterpriseNetworkingAttributesTest, GetNetworkDetails) {
                                ->IsAffiliated());
 
   const Extension* extension =
-      ForceInstallExtension(kExtensionPath, kExtensionPemPath);
+      ForceInstallExtension(kTestExtensionID, kUpdateManifestPath);
   SetupDisconnectedNetwork();
   const GURL test_url = extension->GetResourceURL("test.html");
 

chrome/browser/extensions/api/force_installed_affiliated_extension_apitest.cc

@@ -4,16 +4,11 @@
 
 #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/profiles/profile_manager.h"
-#include "chrome/common/chrome_paths.h"
-#include "chrome/test/base/mixin_based_in_process_browser_test.h"
+#include "chrome/browser/extensions/policy_test_utils.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"
@@ -34,10 +29,6 @@ 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 {
@@ -58,33 +49,16 @@ ForceInstalledAffiliatedExtensionApiTest::
 ForceInstalledAffiliatedExtensionApiTest::
     ~ForceInstalledAffiliatedExtensionApiTest() = default;
 
-void ForceInstalledAffiliatedExtensionApiTest::SetUp() {
-  mixin_host_.SetUp();
-  ExtensionApiTest::SetUp();
-}
-
 void ForceInstalledAffiliatedExtensionApiTest::SetUpCommandLine(
     base::CommandLine* command_line) {
-  mixin_host_.SetUpCommandLine(command_line);
+  ExtensionApiTest::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() {
-  mixin_host_.SetUpInProcessBrowserTestFixture();
+  ExtensionApiTest::SetUpInProcessBrowserTestFixture();
 
   // Initialize clients here so they are available during setup. They will be
   // shutdown in ChromeBrowserMain.
@@ -119,18 +93,9 @@ 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 =
@@ -139,36 +104,24 @@ void ForceInstalledAffiliatedExtensionApiTest::SetUpOnMainThread() {
     policy::AffiliationTestHelper::LoginUser(affiliated_account_id_);
   }
 
-  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();
-}
+  policy_test_utils::SetUpEmbeddedTestServer(embedded_test_server());
+  ASSERT_TRUE(embedded_test_server()->Start());
 
-void ForceInstalledAffiliatedExtensionApiTest::TearDown() {
-  mixin_host_.TearDown();
-  ExtensionApiTest::TearDown();
+  ExtensionApiTest::SetUpOnMainThread();
 }
 
 const extensions::Extension*
 ForceInstalledAffiliatedExtensionApiTest::ForceInstallExtension(
-    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);
+    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;
 }
 
 void ForceInstalledAffiliatedExtensionApiTest::TestExtension(

chrome/browser/extensions/api/force_installed_affiliated_extension_apitest.h

@@ -10,8 +10,6 @@
 #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"
@@ -28,12 +26,8 @@ namespace extensions {
 
 class Extension;
 
-// 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.
+// TODO(https://crbug.com/1082195) Create force-installed extension and user
+// affiliation test mixins to replace this class.
 
 // Helper class to test force-installed extensions in a
 // affiliated/non-affiliated user profile.
@@ -44,21 +38,13 @@ 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 std::string& extension_path,
-      const std::string& pem_path);
+      const extensions::ExtensionId& extension_id,
+      const std::string& update_manifest_path);
 
   // Sets |custom_arg_value|, loads |page_url| and waits for an extension API
   // test pass/fail notification.
@@ -74,8 +60,6 @@ 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,11 +29,6 @@
 #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"
@@ -330,6 +325,8 @@ 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) {
@@ -339,35 +336,6 @@ 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,
@@ -401,17 +369,6 @@ 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(
@@ -648,11 +605,6 @@ 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,10 +25,6 @@ namespace extensions {
 class Extension;
 }  // namespace extensions
 
-namespace policy {
-class MockConfigurationPolicyProvider;
-}  // namespace policy
-
 #if defined(OS_CHROMEOS)
 
 namespace chromeos {
@@ -96,10 +92,6 @@ 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);
@@ -185,7 +177,6 @@ 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>