Nasty short-term hack to special case display of dialog

on theme installation. It should not be displayed if the
theme was installed from our gallery URL.

I intend to fix this very soon, but I wanted it to make
this coming build, so I did this quick thing for now.

Also, I found another bug in ExtensionsService install
logic. There was a test, it was ensuring the wrong
behavior :(.
Review URL: http://codereview.chromium.org/155936

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

5 files changed, 65 insertions, 31 deletions

diff --git a/chrome/browser/download/download_manager.cc b/chrome/browser/download/download_manager.cc
index 2a097f0..ccd986c 100644
--- a/chrome/browser/download/download_manager.cc
+++ b/chrome/browser/download/download_manager.cc
@@ -839,7 +839,7 @@ void DownloadManager::ContinueDownloadFinished(DownloadItem* download) {
 
   // Handle chrome extensions explicitly and skip the shell execute.
   if (Extension::IsExtension(download->full_path())) {
-    OpenChromeExtension(download->full_path());
+    OpenChromeExtension(download->full_path(), download->url());
     download->set_auto_opened(true);
   } else if (download->open_when_complete() ||
              ShouldOpenFileExtension(extension)) {
@@ -1208,15 +1208,16 @@ void DownloadManager::OpenDownload(const DownloadItem* download,
   // Open Chrome extensions with ExtensionsService. For everything else do shell
   // execute.
   if (Extension::IsExtension(download->full_path())) {
-    OpenChromeExtension(download->full_path());
+    OpenChromeExtension(download->full_path(), download->url());
   } else {
     OpenDownloadInShell(download, parent_window);
   }
 }
 
-void DownloadManager::OpenChromeExtension(const FilePath& full_path) {
+void DownloadManager::OpenChromeExtension(const FilePath& full_path,
+                                          const GURL& download_url) {
   profile_->GetOriginalProfile()->GetExtensionsService()->
-      InstallExtension(full_path);
+      InstallExtension(full_path, download_url);
 }
 
 void DownloadManager::OpenDownloadInShell(const DownloadItem* download,
diff --git a/chrome/browser/download/download_manager.h b/chrome/browser/download/download_manager.h
index 4fdc8dd..dad535e 100644
--- a/chrome/browser/download/download_manager.h
+++ b/chrome/browser/download/download_manager.h
@@ -448,7 +448,7 @@ class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>,
                            gfx::NativeView parent_window);
 
   // Opens downloaded Chrome extension file (*.crx).
-  void OpenChromeExtension(const FilePath& full_path);
+  void OpenChromeExtension(const FilePath& full_path, const GURL& download_url);
 
   // Shutdown the download manager.  This call is needed only after Init.
   void Shutdown();
diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index f1981f4..7d417bb 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -79,6 +79,9 @@ const char* kSignatureVerificationFailed = "Signature verification failed";
 const char* kSignatureVerificationInitFailed =
     "Signature verification initialization failed. This is most likely "
     "caused by a public key in the wrong format (should encode algorithm).";
+
+const char* kGalleryHost = "www.google.com";
+const char* kGalleryPath = "/chrome/";
 }
 
 // This class coordinates an extension unpack task which is run in a separate
@@ -91,10 +94,10 @@ class ExtensionsServiceBackend::UnpackerClient
                  const FilePath& extension_path,
                  const std::string& public_key,
                  const std::string& expected_id,
-                 bool silent)
+                 bool silent, bool from_gallery)
     : backend_(backend), extension_path_(extension_path),
       public_key_(public_key), expected_id_(expected_id), got_response_(false),
-      silent_(silent) {
+      silent_(silent), from_gallery_(from_gallery) {
   }
 
   // Starts the unpack task.  We call back to the backend when the task is done,
@@ -171,7 +174,8 @@ class ExtensionsServiceBackend::UnpackerClient
     FilePath extension_dir = temp_extension_path_.DirName().AppendASCII(
         ExtensionsServiceBackend::kTempExtensionName);
     backend_->OnExtensionUnpacked(extension_path_, extension_dir,
-                                  expected_id_, manifest, images, silent_);
+                                  expected_id_, manifest, images, silent_,
+                                  from_gallery_);
     Cleanup();
   }
 
@@ -218,6 +222,10 @@ class ExtensionsServiceBackend::UnpackerClient
 
   // True if the install should be done with no confirmation dialog.
   bool silent_;
+
+  // True if the install is from the gallery (and therefore should not get an
+  // alert UI if it turns out to also be a theme).
+  bool from_gallery_;
 };
 
 ExtensionsService::ExtensionsService(Profile* profile,
@@ -285,9 +293,16 @@ void ExtensionsService::Init() {
 }
 
 void ExtensionsService::InstallExtension(const FilePath& extension_path) {
+  InstallExtension(extension_path, GURL());
+}
+
+void ExtensionsService::InstallExtension(const FilePath& extension_path,
+                                         const GURL& url) {
+  bool from_gallery = url.host() == kGalleryHost &&
+                      StartsWithASCII(url.path(), kGalleryPath, false);
+
   backend_loop_->PostTask(FROM_HERE, NewRunnableMethod(backend_.get(),
-      &ExtensionsServiceBackend::InstallExtension,
-      extension_path,
+      &ExtensionsServiceBackend::InstallExtension, extension_path, from_gallery,
       scoped_refptr<ExtensionsService>(this)));
 }
 
@@ -940,13 +955,14 @@ bool ExtensionsServiceBackend::SetCurrentVersion(const FilePath& dest_dir,
 }
 
 void ExtensionsServiceBackend::InstallExtension(
-    const FilePath& extension_path, scoped_refptr<ExtensionsService> frontend) {
+    const FilePath& extension_path, bool from_gallery,
+    scoped_refptr<ExtensionsService> frontend) {
   LOG(INFO) << "Installing extension " << extension_path.value();
 
   frontend_ = frontend;
   alert_on_error_ = true;
 
-  InstallOrUpdateExtension(extension_path, std::string(), false);
+  InstallOrUpdateExtension(extension_path, from_gallery, std::string(), false);
 }
 
 void ExtensionsServiceBackend::UpdateExtension(const std::string& id,
@@ -957,18 +973,19 @@ void ExtensionsServiceBackend::UpdateExtension(const std::string& id,
   frontend_ = frontend;
   alert_on_error_ = alert_on_error;
 
-  InstallOrUpdateExtension(extension_path, id, true);
+  InstallOrUpdateExtension(extension_path, false, id, true);
 }
 
 void ExtensionsServiceBackend::InstallOrUpdateExtension(
-    const FilePath& extension_path, const std::string& expected_id,
-    bool silent) {
+    const FilePath& extension_path, bool from_gallery,
+    const std::string& expected_id, bool silent) {
   std::string actual_public_key;
   if (!ValidateSignature(extension_path, &actual_public_key))
     return;  // Failures reported within ValidateSignature().
 
   UnpackerClient* client = new UnpackerClient(
-      this, extension_path, actual_public_key, expected_id, silent);
+      this, extension_path, actual_public_key, expected_id, silent,
+      from_gallery);
   client->Start();
 }
 
@@ -1067,7 +1084,7 @@ void ExtensionsServiceBackend::OnExtensionUnpacked(
     const std::string expected_id,
     const DictionaryValue& manifest,
     const std::vector< Tuple2<SkBitmap, FilePath> >& images,
-    bool silent) {
+    bool silent, bool from_gallery) {
   Extension extension;
   std::string error;
   if (!extension.InitFromValue(manifest,
@@ -1084,7 +1101,7 @@ void ExtensionsServiceBackend::OnExtensionUnpacked(
   // installed.
   if (!extensions_enabled_ &&
       !extension.IsTheme() &&
-      location != Extension::EXTERNAL_REGISTRY) {
+      !Extension::IsExternalLocation(location)) {
     ReportExtensionInstallError(extension_path,
         "Extensions are not enabled. Add --enable-extensions to the "
         "command-line to enable extensions.\n\n"
@@ -1095,12 +1112,23 @@ void ExtensionsServiceBackend::OnExtensionUnpacked(
 
   // TODO(extensions): Make better extensions UI. http://crbug.com/12116
 
-  // We don't show the install dialog for themes, updates, or external
-  // extensions.
-  if (!extension.IsTheme() && !silent && frontend_->show_extensions_prompts()) {
+  // We don't show the dialog for a few special cases:
+  // - themes from the gallery
+  // - externally registered extensions
+  // - during tests (!frontend->show_extension_prompts()) and updates (silent).
+  bool show_dialog = true;
+  if (extension.IsTheme() && from_gallery)
+    show_dialog = false;
+
+  if (Extension::IsExternalLocation(location))
+    show_dialog = false;
+
+  if (silent || !frontend_->show_extensions_prompts())
+    show_dialog = false;
+
+  if (show_dialog) {
 #if defined(OS_WIN)
-    if (!Extension::IsExternalLocation(location) &&
-        win_util::MessageBox(GetForegroundWindow(),
+    if (!win_util::MessageBox(GetForegroundWindow(),
             L"Are you sure you want to install this extension?\n\n"
             L"This is a temporary message and it will be removed when "
             L"extensions UI is finalized.",
@@ -1300,7 +1328,7 @@ void ExtensionsServiceBackend::CheckVersionAndInstallExtension(
     const std::string& id, const Version* extension_version,
     const FilePath& extension_path) {
   if (ShouldInstall(id, extension_version))
-    InstallOrUpdateExtension(FilePath(extension_path), id, false);
+    InstallOrUpdateExtension(FilePath(extension_path), false, id, false);
 }
 
 bool ExtensionsServiceBackend::LookupExternalExtension(
diff --git a/chrome/browser/extensions/extensions_service.h b/chrome/browser/extensions/extensions_service.h
index 87c0dc7..d63073c 100644
--- a/chrome/browser/extensions/extensions_service.h
+++ b/chrome/browser/extensions/extensions_service.h
@@ -121,6 +121,10 @@ class ExtensionsService
   // immediately loaded.
   void InstallExtension(const FilePath& extension_path);
 
+  // XXX Hack: This is a temporary nasty hack to get theme installation working
+  // without a dialog. Will be fixed by making ExtensionsService more modular.
+  void InstallExtension(const FilePath& extension_path, const GURL& url);
+
   // Updates a currently-installed extension with the contents from
   // |extension_path|. The |alert_on_error| parameter controls whether the
   // user will be notified in the event of failure. If |callback| is non-null,
@@ -296,7 +300,7 @@ class ExtensionsServiceBackend
   // Install the extension file at |extension_path|. Errors are reported through
   // ExtensionErrorReporter. OnExtensionInstalled is called in the frontend on
   // success.
-  void InstallExtension(const FilePath& extension_path,
+  void InstallExtension(const FilePath& extension_path, bool from_gallery,
                         scoped_refptr<ExtensionsService> frontend);
 
   // Similar to InstallExtension, but |extension_path| must be an updated
@@ -358,6 +362,7 @@ class ExtensionsServiceBackend
   // number is greater than the current installed version. If |silent| is true,
   // the confirmation dialog will not pop up.
   void InstallOrUpdateExtension(const FilePath& extension_path,
+                                bool from_gallery,
                                 const std::string& expected_id, bool silent);
 
   // Validates the signature of the extension in |extension_path|. Returns true
@@ -376,7 +381,8 @@ class ExtensionsServiceBackend
       const std::string expected_id,
       const DictionaryValue& manifest,
       const std::vector< Tuple2<SkBitmap, FilePath> >& images,
-      bool silent);
+      bool silent,
+      bool from_gallery);
 
   // Notify the frontend that there was an error loading an extension.
   void ReportExtensionLoadError(const FilePath& extension_path,
diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc
index dd77d47..2b1492d 100644
--- a/chrome/browser/extensions/extensions_service_unittest.cc
+++ b/chrome/browser/extensions/extensions_service_unittest.cc
@@ -1329,17 +1329,16 @@ TEST_F(ExtensionsServiceTest, ExternalInstallPref) {
   // The extension should also be gone from the install directory.
   ASSERT_FALSE(file_util::PathExists(install_path));
 
-  // This shouldn't work if extensions are disabled.
+  // It should still work if extensions are disabled (disableness doesn't
+  // apply to externally registered extensions).
   SetExtensionsEnabled(false);
 
   pref_provider->UpdateOrAddExtension(good_crx, "1.0", source_path);
   service_->CheckForExternalUpdates();
   loop_.RunAllPending();
 
-  ASSERT_EQ(0u, loaded_.size());
-  ASSERT_EQ(1u, GetErrors().size());
-  ASSERT_TRUE(GetErrors()[0].find("Extensions are not enabled") !=
-              std::string::npos);
+  ASSERT_EQ(1u, loaded_.size());
+  ASSERT_EQ(0u, GetErrors().size());
 }
 
 TEST_F(ExtensionsServiceTest, ExternalPrefProvider) {