Don't show extension disabled infobar when manually updating

extensions that increase privilges.

BUG=20461

Review URL: http://codereview.chromium.org/174637

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@24775 0039d316-1c4b-4281-b951-d872f2087c98

11 files changed, 97 insertions, 50 deletions

diff --git a/chrome/browser/download/download_manager.cc b/chrome/browser/download/download_manager.cc
index 111a3e0..9477925 100644
--- a/chrome/browser/download/download_manager.cc
+++ b/chrome/browser/download/download_manager.cc
@@ -1255,6 +1255,7 @@ void DownloadManager::OpenChromeExtension(const FilePath& full_path,
                         Extension::INTERNAL,
                         "",  // no expected id
                         true,  // please delete crx on completion
+                        true,  // privilege increase allowed
                         g_browser_process->file_thread()->message_loop(),
                         service,
                         new ExtensionInstallUI(profile_));
diff --git a/chrome/browser/extensions/crx_installer.cc b/chrome/browser/extensions/crx_installer.cc
index 677aada..7d30cef 100644
--- a/chrome/browser/extensions/crx_installer.cc
+++ b/chrome/browser/extensions/crx_installer.cc
@@ -29,13 +29,15 @@ void CrxInstaller::Start(const FilePath& crx_path,
                          Extension::Location install_source,
                          const std::string& expected_id,
                          bool delete_crx,
+                         bool allow_privilege_increase,
                          MessageLoop* file_loop,
                          ExtensionsService* frontend,
                          ExtensionInstallUI* client) {
   // Note: We don't keep a reference because this object manages its own
   // lifetime.
   new CrxInstaller(crx_path, install_directory, install_source, expected_id,
-                   delete_crx, file_loop, frontend, client);
+                   delete_crx, allow_privilege_increase, file_loop, frontend,
+                   client);
 }
 
 CrxInstaller::CrxInstaller(const FilePath& crx_path,
@@ -43,6 +45,7 @@ CrxInstaller::CrxInstaller(const FilePath& crx_path,
                            Extension::Location install_source,
                            const std::string& expected_id,
                            bool delete_crx,
+                           bool allow_privilege_increase,
                            MessageLoop* file_loop,
                            ExtensionsService* frontend,
                            ExtensionInstallUI* client)
@@ -51,6 +54,7 @@ CrxInstaller::CrxInstaller(const FilePath& crx_path,
       install_source_(install_source),
       expected_id_(expected_id),
       delete_crx_(delete_crx),
+      allow_privilege_increase_(allow_privilege_increase),
       file_loop_(file_loop),
       ui_loop_(MessageLoop::current()),
       frontend_(frontend),
@@ -289,7 +293,8 @@ void CrxInstaller::ReportSuccessFromUIThread() {
 
   // Tell the frontend about the installation and hand off ownership of
   // extension_ to it.
-  frontend_->OnExtensionInstalled(extension_.release());
+  frontend_->OnExtensionInstalled(extension_.release(),
+                                  allow_privilege_increase_);
 
   // We're done. We don't post any more tasks to ourselves so we are deleted
   // soon.
diff --git a/chrome/browser/extensions/crx_installer.h b/chrome/browser/extensions/crx_installer.h
index c1a4d55..23f5cda 100644
--- a/chrome/browser/extensions/crx_installer.h
+++ b/chrome/browser/extensions/crx_installer.h
@@ -59,6 +59,7 @@ class CrxInstaller :
                     Extension::Location install_source,
                     const std::string& expected_id,
                     bool delete_crx,
+                    bool allow_privilege_increase,
                     MessageLoop* file_loop,
                     ExtensionsService* frontend,
                     ExtensionInstallUI* client);
@@ -78,6 +79,7 @@ class CrxInstaller :
                Extension::Location install_source,
                const std::string& expected_id,
                bool delete_crx,
+               bool allow_privilege_increase,
                MessageLoop* file_loop,
                ExtensionsService* frontend,
                ExtensionInstallUI* client);
@@ -128,6 +130,10 @@ class CrxInstaller :
   // Whether we're supposed to delete the source crx file on destruction.
   bool delete_crx_;
 
+  // Whether privileges should be allowed to silently increaes from any
+  // previously installed version of the extension.
+  bool allow_privilege_increase_;
+
   // The message loop to use for file IO.
   MessageLoop* file_loop_;
 
diff --git a/chrome/browser/extensions/extension_browsertest.cc b/chrome/browser/extensions/extension_browsertest.cc
index 4fead1c..27e6311 100644
--- a/chrome/browser/extensions/extension_browsertest.cc
+++ b/chrome/browser/extensions/extension_browsertest.cc
@@ -64,8 +64,8 @@ bool ExtensionBrowserTest::LoadExtension(const FilePath& path) {
   return WaitForExtensionHostsToLoad();
 }
 
-bool ExtensionBrowserTest::InstallExtension(const FilePath& path,
-                                            int expected_change) {
+bool ExtensionBrowserTest::InstallOrUpdateExtension(
+    const std::string& id, const FilePath& path, int expected_change) {
   ExtensionsService* service = browser()->profile()->GetExtensionsService();
   service->set_show_extensions_prompts(false);
   size_t num_before = service->extensions()->size();
@@ -76,7 +76,19 @@ bool ExtensionBrowserTest::InstallExtension(const FilePath& path,
                   NotificationService::AllSources());
     registrar.Add(this, NotificationType::EXTENSION_UPDATE_DISABLED,
                   NotificationService::AllSources());
-    service->InstallExtension(path);
+
+    if (!id.empty()) {
+      // We need to copy this to a temporary location because Update() will
+      // delete it.
+      FilePath temp_dir;
+      PathService::Get(base::DIR_TEMP, &temp_dir);
+      FilePath copy = temp_dir.Append(path.BaseName());
+      file_util::CopyFile(path, copy);
+      service->UpdateExtension(id, copy);
+    } else {
+      service->InstallExtension(path);
+    }
+
     MessageLoop::current()->PostDelayedTask(
         FROM_HERE, new MessageLoop::QuitTask, kTimeoutMs);
     ui_test_utils::RunMessageLoop();
diff --git a/chrome/browser/extensions/extension_browsertest.h b/chrome/browser/extensions/extension_browsertest.h
index 98fb948..1910229 100644
--- a/chrome/browser/extensions/extension_browsertest.h
+++ b/chrome/browser/extensions/extension_browsertest.h
@@ -22,11 +22,22 @@ class ExtensionBrowserTest
  protected:
   virtual void SetUpCommandLine(CommandLine* command_line);
   bool LoadExtension(const FilePath& path);
+
   // |expected_change| indicates how many extensions should be installed (or
   // disabled, if negative).
   // 1 means you expect a new install, 0 means you expect an upgrade, -1 means
   // you expect a failed upgrade.
-  bool InstallExtension(const FilePath& path, int expected_change);
+  bool InstallExtension(const FilePath& path, int expected_change) {
+    return InstallOrUpdateExtension("", path, expected_change);
+  }
+
+  // Same as above but calls ExtensionsService::UpdateExtension instead of
+  // InstallExtension().
+  bool UpdateExtension(const std::string& id, const FilePath& path,
+                       int expected_change) {
+    return InstallOrUpdateExtension(id, path, expected_change);
+  }
+
   void UninstallExtension(const std::string& extension_id);
 
   // Wait for the number of visible page actions to change to |count|.
@@ -42,6 +53,9 @@ class ExtensionBrowserTest
   FilePath test_data_dir_;
 
  private:
+  bool InstallOrUpdateExtension(const std::string& id, const FilePath& path,
+                                int expected_change);
+
   bool WaitForExtensionHostsToLoad();
 };
 
diff --git a/chrome/browser/extensions/extension_browsertests_misc.cc b/chrome/browser/extensions/extension_browsertests_misc.cc
index 3165c59..03e2bc3 100644
--- a/chrome/browser/extensions/extension_browsertests_misc.cc
+++ b/chrome/browser/extensions/extension_browsertests_misc.cc
@@ -486,7 +486,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, UninstallDisabled) {
   // Install and upgrade, so that we have a disabled extension.
   ASSERT_TRUE(InstallExtension(
       test_data_dir_.AppendASCII("permissions-low-v1.crx"), 1));
-  ASSERT_TRUE(InstallExtension(
+  ASSERT_TRUE(UpdateExtension("pgdpcfcocojkjfbgpiianjngphoopgmo",
       test_data_dir_.AppendASCII("permissions-high-v2.crx"), -1));
 
   ExtensionsService* service = browser()->profile()->GetExtensionsService();
diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index d389aa6..acb74cd 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -146,6 +146,7 @@ void ExtensionsService::InstallExtension(const FilePath& extension_path) {
   CrxInstaller::Start(extension_path, install_directory_, Extension::INTERNAL,
                       "",   // no expected id
                       false,  // don't delete crx when complete
+                      true,  // allow privilege increase
                       backend_loop_,
                       this,
                       NULL);  // no client (silent install)
@@ -162,6 +163,7 @@ void ExtensionsService::UpdateExtension(const std::string& id,
   CrxInstaller::Start(extension_path, install_directory_, Extension::INTERNAL,
                       id,
                       true,  // delete crx when complete
+                      false,  // do not allow upgrade of privileges
                       backend_loop_,
                       this,
                       NULL);  // no client (silent install)
@@ -256,7 +258,7 @@ void ExtensionsService::LoadInstalledExtension(
   }
 
   extension->set_location(location);
-  OnExtensionLoaded(extension);
+  OnExtensionLoaded(extension, true);
 
   if (location == Extension::EXTERNAL_PREF ||
       location == Extension::EXTERNAL_REGISTRY) {
@@ -370,7 +372,8 @@ void ExtensionsService::OnLoadedInstalledExtensions() {
       NotificationService::NoDetails());
 }
 
-void ExtensionsService::OnExtensionLoaded(Extension* extension) {
+void ExtensionsService::OnExtensionLoaded(Extension* extension,
+                                          bool allow_privilege_increase) {
   // Ensure extension is deleted unless we transfer ownership.
   scoped_ptr<Extension> scoped_extension(extension);
 
@@ -381,8 +384,9 @@ void ExtensionsService::OnExtensionLoaded(Extension* extension) {
     Extension* old = GetExtensionByIdInternal(extension->id(), true, true);
     if (old) {
       if (extension->version()->CompareTo(*(old->version())) > 0) {
-        bool allow_silent_upgrade = Extension::AllowSilentUpgrade(
-            old, extension);
+        bool allow_silent_upgrade =
+            allow_privilege_increase || !Extension::IsPrivilegeIncrease(
+                old, extension);
 
         // To upgrade an extension in place, unload the old one and
         // then load the new one.
@@ -433,7 +437,8 @@ void ExtensionsService::OnExtensionLoaded(Extension* extension) {
   }
 }
 
-void ExtensionsService::OnExtensionInstalled(Extension* extension) {
+void ExtensionsService::OnExtensionInstalled(Extension* extension,
+                                             bool allow_privilege_increase) {
   extension_prefs_->OnExtensionInstalled(extension);
 
   // If the extension is a theme, tell the profile (and therefore ThemeProvider)
@@ -451,7 +456,7 @@ void ExtensionsService::OnExtensionInstalled(Extension* extension) {
   }
 
   // Also load the extension.
-  OnExtensionLoaded(extension);
+  OnExtensionLoaded(extension, allow_privilege_increase);
 }
 
 void ExtensionsService::OnExtensionOverinstallAttempted(const std::string& id) {
@@ -528,6 +533,7 @@ void ExtensionsService::OnExternalExtensionFound(const std::string& id,
 
   CrxInstaller::Start(path, install_directory_, location, id,
                       false,  // don't delete crx when complete
+                      true,  // allow privilege increase
                       backend_loop_,
                       this,
                       NULL);  // no client (silent install)
@@ -592,7 +598,7 @@ void ExtensionsServiceBackend::ReportExtensionLoadError(
 
 void ExtensionsServiceBackend::ReportExtensionLoaded(Extension* extension) {
   frontend_loop_->PostTask(FROM_HERE, NewRunnableMethod(
-      frontend_, &ExtensionsService::OnExtensionLoaded, extension));
+      frontend_, &ExtensionsService::OnExtensionLoaded, extension, true));
 }
 
 bool ExtensionsServiceBackend::LookupExternalExtension(
diff --git a/chrome/browser/extensions/extensions_service.h b/chrome/browser/extensions/extensions_service.h
index d745058..75303b4 100644
--- a/chrome/browser/extensions/extensions_service.h
+++ b/chrome/browser/extensions/extensions_service.h
@@ -170,10 +170,12 @@ class ExtensionsService
   void OnLoadedInstalledExtensions();
 
   // Called by the backend when an extension has been loaded.
-  void OnExtensionLoaded(Extension* extension);
+  void OnExtensionLoaded(Extension* extension,
+                         bool allow_privilege_increase);
 
   // Called by the backend when an extension has been installed.
-  void OnExtensionInstalled(Extension* extension);
+  void OnExtensionInstalled(Extension* extension,
+                            bool allow_privilege_increase);
 
   // Called by the backend when an attempt was made to reinstall the same
   // version of an existing extension.
diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index 134ee3f..c826c07 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -531,21 +531,22 @@ bool Extension::FormatPEMForFileOutput(const std::string input,
 // extensions that require less permissions than the current version, but then
 // we don't silently allow them to go back. In order to fix this, we would need
 // to remember the max set of permissions we ever granted a single extension.
-bool Extension::AllowSilentUpgrade(Extension* old_extension,
-                                   Extension* new_extension) {
+bool Extension::IsPrivilegeIncrease(Extension* old_extension,
+                                    Extension* new_extension) {
   // If the old extension had native code access, we don't need to go any
   // further. Things can't get any worse.
   if (old_extension->plugins().size() > 0)
-    return true;
+    return false;
 
-  // Otherwise, if the new extension has a plugin, no silent upgrade.
+  // Otherwise, if the new extension has a plugin, it's a privilege increase.
   if (new_extension->plugins().size() > 0)
-    return false;
+    return true;
 
-  // If we are increasing the set of hosts we have access to, no silent upgrade.
+  // If we are increasing the set of hosts we have access to, it's a privilege
+  // increase.
   if (!old_extension->HasAccessToAllHosts()) {
     if (new_extension->HasAccessToAllHosts())
-      return false;
+      return true;
 
     std::set<std::string> old_hosts =
         old_extension->GetEffectiveHostPermissions();
@@ -558,17 +559,17 @@ bool Extension::AllowSilentUpgrade(Extension* old_extension,
                         std::insert_iterator<std::set<std::string> >(
                             difference, difference.end()));
     if (difference.size() > 0)
-      return false;
+      return true;
   }
 
-  // If we're going from not having api permissions to having them, no silent
-  // upgrade.
+  // If we're going from not having api permissions to having them, it's a
+  // privilege increase.
   if (old_extension->api_permissions().size() == 0 &&
       new_extension->api_permissions().size() > 0)
-    return false;
+    return true;
 
-  // Nothing much has changed. Allow the silent upgrade.
-  return true;
+  // Nothing much has changed.
+  return false;
 }
 
 bool Extension::InitFromValue(const DictionaryValue& source, bool require_id,
diff --git a/chrome/common/extensions/extension.h b/chrome/common/extensions/extension.h
index ab68d60..6e7215f 100644
--- a/chrome/common/extensions/extension.h
+++ b/chrome/common/extensions/extension.h
@@ -168,10 +168,10 @@ class Extension {
   static bool FormatPEMForFileOutput(const std::string input,
       std::string* output, bool is_public);
 
-  // Determine whether we should allow a silent upgrade from |old_extension| to
-  // |new_extension|. If not, the user will have to approve the upgrade.
-  static bool AllowSilentUpgrade(Extension* old_extension,
-                                 Extension* new_extension);
+  // Determine whether |new_extension| has increased privileges compared to
+  // |old_extension|.
+  static bool IsPrivilegeIncrease(Extension* old_extension,
+                                  Extension* new_extension);
 
   // Initialize the extension from a parsed manifest.
   // If |require_id| is true, will return an error if the "id" key is missing
diff --git a/chrome/common/extensions/extension_unittest.cc b/chrome/common/extensions/extension_unittest.cc
index 8f1b69b..fb40330 100644
--- a/chrome/common/extensions/extension_unittest.cc
+++ b/chrome/common/extensions/extension_unittest.cc
@@ -582,25 +582,25 @@ TEST(ExtensionTest, EffectiveHostPermissions) {
   EXPECT_TRUE(extension->HasAccessToAllHosts());
 }
 
-TEST(ExtensionTest, AllowSilentUpgrade) {
+TEST(ExtensionTest, IsPrivilegeIncrease) {
   const struct {
     const char* base_name;
     bool expect_success;
   } kTests[] = {
-    { "allhosts1", true },  // all -> all
-    { "allhosts2", true },  // all -> one
-    { "allhosts3", false },  // one -> all
-    { "hosts1", true },  // http://a,http://b -> http://a,http://b
-    { "hosts2", true },  // http://a,http://b -> https://a,http://*.b
-    { "hosts3", true },  // http://a,http://b -> http://a
-    { "hosts4", false },  // http://a -> http://a,http://b
-    { "permissions1", true},  // tabs -> tabs
-    { "permissions2", true},  // tabs -> tabs,bookmarks
-    { "permissions3", false},  // http://a -> http://a,tabs
-    { "permissions4", true},  // plugin -> plugin,tabs
-    { "plugin1", true},  // plugin -> plugin
-    { "plugin2", true},  // plugin -> none
-    { "plugin3", false}  // none -> plugin
+    { "allhosts1", false },  // all -> all
+    { "allhosts2", false },  // all -> one
+    { "allhosts3", true },  // one -> all
+    { "hosts1", false },  // http://a,http://b -> http://a,http://b
+    { "hosts2", false },  // http://a,http://b -> https://a,http://*.b
+    { "hosts3", false },  // http://a,http://b -> http://a
+    { "hosts4", true },  // http://a -> http://a,http://b
+    { "permissions1", false },  // tabs -> tabs
+    { "permissions2", false },  // tabs -> tabs,bookmarks
+    { "permissions3", true },  // http://a -> http://a,tabs
+    { "permissions4", false },  // plugin -> plugin,tabs
+    { "plugin1", false },  // plugin -> plugin
+    { "plugin2", false },  // plugin -> none
+    { "plugin3", true }  // none -> plugin
   };
 
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kTests); ++i) {
@@ -612,8 +612,8 @@ TEST(ExtensionTest, AllowSilentUpgrade) {
                      std::string(kTests[i].base_name) + "_new.json"));
 
     EXPECT_EQ(kTests[i].expect_success,
-              Extension::AllowSilentUpgrade(old_extension.get(),
-                                            new_extension.get()))
+              Extension::IsPrivilegeIncrease(old_extension.get(),
+                                             new_extension.get()))
         << kTests[i].base_name;
   }
 }