Improve settings override bubble to indicate policy installed extensions

chrome/browser/extensions/extension_service_test_with_install.cc

 // Copyright 2015 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/extensions/extension_service_test_with_install.h"
 
 #include "base/files/file_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/extensions/crx_installer.h"
 #include "chrome/browser/extensions/extension_creator.h"
@@ skipping 83 matching lines @@
                            pem_output_path,
                            ExtensionCreator::kOverwriteCRX));
 
   ASSERT_TRUE(base::PathExists(crx_path));
 }
 
 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);
 
-  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(
     const base::FilePath& dir_path,
     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);
+                           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, install_location);
 }
 
 // Attempts to install an extension. Use INSTALL_FAILED if the installation
 // is expected to fail.
 // If |install_state| is INSTALL_UPDATED, and |expected_old_name| is
 // non-empty, expects that the existing extension's title was
 // |expected_old_name|.
 const Extension* ExtensionServiceTestWithInstall::InstallCRX(
     const base::FilePath& path,
     InstallState install_state,
@@ skipping 186 matching lines @@
     const std::string& id,
     bool use_helper,
     Extension::State expected_state) {
   // Verify that the extension is installed.
   base::FilePath extension_path = extensions_install_dir().AppendASCII(id);
   EXPECT_TRUE(base::PathExists(extension_path));
   size_t pref_key_count = GetPrefKeyCount();
   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(
         service(), id, extensions::UNINSTALL_REASON_FOR_TESTING));
   } else {
     EXPECT_TRUE(service()->UninstallExtension(
         id, extensions::UNINSTALL_REASON_FOR_TESTING,
         base::Bind(&base::DoNothing), nullptr));
   }
   --expected_extensions_count_;
 
   // 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();
   if (new_pref_key_count == pref_key_count) {
     ValidateIntegerPref(id, "state",
                         Extension::EXTERNAL_EXTENSION_UNINSTALLED);
   } else {
     EXPECT_EQ(new_pref_key_count, pref_key_count - 1);
   }
 
   // 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.
   EXPECT_FALSE(base::PathExists(extension_path));
 }
 
 void ExtensionServiceTestWithInstall::TerminateExtension(
     const std::string& id) {
   const Extension* extension = service()->GetInstalledExtension(id);
   if (!extension) {
@@ skipping 54 matching lines @@
   content::WindowedNotificationObserver observer(
       extensions::NOTIFICATION_CRX_INSTALLER_DONE,
       content::Source<extensions::CrxInstaller>(installer.get()));
 
   installer->InstallCrx(crx_path);
 
   observer.Wait();
 }
 
 }  // namespace extensions