Improve settings override bubble to indicate policy installed extensions

chrome/browser/extensions/extension_service_test_with_install.cc

Index: chrome/browser/extensions/extension_service_test_with_install.cc
diff --git a/chrome/browser/extensions/extension_service_test_with_install.cc b/chrome/browser/extensions/extension_service_test_with_install.cc
index 9ff4cf07e22e1857a8d6baa20e5c2f03e42d08cc..c00b4c6f57e5d3f76823b5f3e41542ed706e5afd 100644
--- a/chrome/browser/extensions/extension_service_test_with_install.cc
+++ b/chrome/browser/extensions/extension_service_test_with_install.cc
@@ -101,14 +101,24 @@ const Extension* ExtensionServiceTestWithInstall::PackAndInstallCRX(
     const base::FilePath& dir_path,
     const base::FilePath& pem_path,
     InstallState install_state,
-    int creation_flags) {
+    int creation_flags,
+    Manifest::Location install_location) {
   base::FilePath crx_path;
   base::ScopedTempDir temp_dir;
   EXPECT_TRUE(temp_dir.CreateUniqueTempDir());
   crx_path = temp_dir.GetPath().AppendASCII("temp.crx");
-
   PackCRX(dir_path, pem_path, crx_path);
-  return InstallCRX(crx_path, install_state, creation_flags);
+
+  scoped_refptr<CrxInstaller> installer(CrxInstaller::CreateSilent(service()));
+  installer->set_install_source(install_location);
+
+  content::WindowedNotificationObserver observer(
+      extensions::NOTIFICATION_CRX_INSTALLER_DONE,
+      content::NotificationService::AllSources());
+  installer->InstallCrx(crx_path);

[catmullings, 2016/10/12 18:58:58]

This line replaces:

 return InstallCRX(crx_path, install_state, creation_flags);

However, the new line doesn't have an option to input creation_flags. I didn't
see any method in CrxInstaller that does. How should I accomodate for the
creation_flags?

[Devlin, 2016/10/12 20:16:53]

Instead, we should just be adding a new parameter to InstallCRX to allow for the
location.  Remember, the goal here is to basically have all these methods funnel
down into one central version.  A good litmus test would be that we should
ideally only have one place be creating a CrxInstaller.

[catmullings, 2016/10/14 23:16:46]

Done.

+  observer.Wait();
+
+  return VerifyCrxInstall(crx_path, install_state);
 }
 
 const Extension* ExtensionServiceTestWithInstall::PackAndInstallCRX(
@@ -116,14 +126,22 @@ const Extension* ExtensionServiceTestWithInstall::PackAndInstallCRX(
     const base::FilePath& pem_path,
     InstallState install_state) {
   return PackAndInstallCRX(dir_path, pem_path, install_state,
-                           Extension::NO_FLAGS);
+                           Extension::NO_FLAGS, Manifest::Location::INTERNAL);

[catmullings, 2016/10/12 18:58:58]

What is the default for Manifest::Location among the enum Locations
(https://cs.chromium.org/chromium/src/extensions/common/manifest.h?q=manifest....)
?

I tried Manifest::Location::INTERNAL, but that doesn't work (i.e. breaks my
unit_test), so it's simply serving as a placeholder for now.

[Devlin, 2016/10/12 20:16:53]

INTERNAL is the default value (you can see it in CrxInstaller's ctor here:
https://cs.chromium.org/chromium/src/chrome/browser/extensions/crx_installer....).
 What type of failure are you getting?

[catmullings, 2016/10/14 23:16:46]

Done.

[catmullings, 2016/10/14 23:16:46]

In addressing your other CL comment above, refactoring the InstallCRX functions
ended fixing this.
My unit tests pass.

+}
+
+const Extension* ExtensionServiceTestWithInstall::PackAndInstallCRX(
+    const base::FilePath& dir_path,
+    InstallState install_state) {
+  return PackAndInstallCRX(dir_path, base::FilePath(), install_state,
+                           Extension::NO_FLAGS, Manifest::Location::INTERNAL);
 }
 
 const Extension* ExtensionServiceTestWithInstall::PackAndInstallCRX(
     const base::FilePath& dir_path,
+    Manifest::Location install_location,
     InstallState install_state) {
   return PackAndInstallCRX(dir_path, base::FilePath(), install_state,
-                           Extension::NO_FLAGS);
+                           Extension::NO_FLAGS, install_location);
 }
 
 // Attempts to install an extension. Use INSTALL_FAILED if the installation
@@ -330,6 +348,9 @@ void ExtensionServiceTestWithInstall::UninstallExtension(
   EXPECT_GT(pref_key_count, 0u);
   ValidateIntegerPref(id, "state", expected_state);
 
+  // We make a copy of the extension's id since the extension can be deleted
+  // once it's uninstalled.
+  std::string extension_id = id;
   // Uninstall it.
   if (use_helper) {
     EXPECT_TRUE(ExtensionService::UninstallExtensionHelper(
@@ -343,7 +364,7 @@ void ExtensionServiceTestWithInstall::UninstallExtension(
 
   // We should get an unload notification.
   EXPECT_FALSE(unloaded_id_.empty());
-  EXPECT_EQ(id, unloaded_id_);
+  EXPECT_EQ(extension_id, unloaded_id_);
 
   // Verify uninstalled state.
   size_t new_pref_key_count = GetPrefKeyCount();
@@ -355,7 +376,7 @@ void ExtensionServiceTestWithInstall::UninstallExtension(
   }
 
   // The extension should not be in the service anymore.
-  EXPECT_FALSE(service()->GetInstalledExtension(id));
+  EXPECT_FALSE(service()->GetInstalledExtension(extension_id));
   base::RunLoop().RunUntilIdle();
 
   // The directory should be gone.