Remove extensions code from download code. The delegate and other users in Chrome can always figure out when a download is for an extension install, instead of storing this data in DownloadItem. I added two new methods to the delegate to allow it to override opening the download item, which allows the chrome layer to install the crx when it completes or is opened.

BUG=82782
Review URL: http://codereview.chromium.org/7685046

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

8 files changed, 119 insertions, 14 deletions

diff --git a/chrome/browser/download/chrome_download_manager_delegate.cc b/chrome/browser/download/chrome_download_manager_delegate.cc
index 0e624d9..99267e9 100644
--- a/chrome/browser/download/chrome_download_manager_delegate.cc
+++ b/chrome/browser/download/chrome_download_manager_delegate.cc
@@ -10,6 +10,7 @@
 #include "base/rand_util.h"
 #include "base/stringprintf.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/download/download_crx_util.h"
 #include "chrome/browser/download/download_extensions.h"
 #include "chrome/browser/download/download_file_picker.h"
 #include "chrome/browser/download/download_history.h"
@@ -17,6 +18,7 @@
 #include "chrome/browser/download/download_safe_browsing_client.h"
 #include "chrome/browser/download/download_util.h"
 #include "chrome/browser/download/save_package_file_picker.h"
+#include "chrome/browser/extensions/crx_installer.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/prefs/pref_member.h"
 #include "chrome/browser/prefs/pref_service.h"
@@ -31,6 +33,7 @@
 #include "content/browser/download/download_manager.h"
 #include "content/browser/download/download_status_updater.h"
 #include "content/browser/tab_contents/tab_contents.h"
+#include "content/common/notification_source.h"
 #include "grit/generated_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 
@@ -42,6 +45,15 @@ ChromeDownloadManagerDelegate::ChromeDownloadManagerDelegate(Profile* profile)
 ChromeDownloadManagerDelegate::~ChromeDownloadManagerDelegate() {
 }
 
+bool ChromeDownloadManagerDelegate::IsExtensionDownload(
+    const DownloadItem* item) {
+  if (item->prompt_user_for_save_location())
+    return false;
+
+  return (item->mime_type() == Extension::kMimeType) ||
+      UserScript::IsURLUserScript(item->GetURL(), item->mime_type());
+}
+
 void ChromeDownloadManagerDelegate::SetDownloadManager(DownloadManager* dm) {
   download_manager_ = dm;
   download_history_.reset(new DownloadHistory(profile_));
@@ -111,6 +123,36 @@ bool ChromeDownloadManagerDelegate::ShouldOpenFileBasedOnExtension(
   return download_prefs_->IsAutoOpenEnabledForExtension(extension);
 }
 
+bool ChromeDownloadManagerDelegate::ShouldOpenDownload(DownloadItem* item) {
+  if (!IsExtensionDownload(item))
+    return true;
+
+  download_crx_util::OpenChromeExtension(profile_, *item);
+  return false;
+}
+
+bool ChromeDownloadManagerDelegate::ShouldCompleteDownload(DownloadItem* item) {
+  if (!IsExtensionDownload(item))
+    return true;
+
+  scoped_refptr<CrxInstaller> crx_installer =
+      download_crx_util::OpenChromeExtension(profile_, *item);
+
+  // CRX_INSTALLER_DONE will fire when the install completes.  Observe()
+  // will call CompleteDelayedDownload() on this item.  If this DownloadItem is
+  // not around when CRX_INSTALLER_DONE fires, Complete() will not be called.
+  registrar_.Add(this,
+                 chrome::NOTIFICATION_CRX_INSTALLER_DONE,
+                 Source<CrxInstaller>(crx_installer.get()));
+
+  crx_installers_[crx_installer.get()] = item->id();
+  // The status text and percent complete indicator will change now
+  // that we are installing a CRX.  Update observers so that they pick
+  // up the change.
+  item->UpdateObservers();
+  return false;
+}
+
 bool ChromeDownloadManagerDelegate::GenerateFileHash() {
 #if defined(ENABLE_SAFE_BROWSING)
   return profile_->GetPrefs()->GetBoolean(prefs::kSafeBrowsingEnabled) &&
@@ -216,6 +258,26 @@ void ChromeDownloadManagerDelegate::CheckDownloadUrlDone(
           &ChromeDownloadManagerDelegate::CheckVisitedReferrerBeforeDone));
 }
 
+// NotificationObserver implementation.
+void ChromeDownloadManagerDelegate::Observe(
+    int type,
+    const NotificationSource& source,
+    const NotificationDetails& details) {
+  DCHECK(type == chrome::NOTIFICATION_CRX_INSTALLER_DONE);
+
+  registrar_.Remove(this,
+                    chrome::NOTIFICATION_CRX_INSTALLER_DONE,
+                    source);
+
+  CrxInstaller* installer = Source<CrxInstaller>(source).ptr();
+  int download_id = crx_installers_[installer];
+  crx_installers_.erase(installer);
+
+  DownloadItem* item = download_manager_->GetActiveDownloadItem(download_id);
+  if (item)
+    item->CompleteDelayedDownload();
+}
+
 void ChromeDownloadManagerDelegate::CheckVisitedReferrerBeforeDone(
     int32 download_id,
     bool visited_referrer_before) {
@@ -229,13 +291,6 @@ void ChromeDownloadManagerDelegate::CheckVisitedReferrerBeforeDone(
   // Check whether this download is for an extension install or not.
   // Allow extensions to be explicitly saved.
   DownloadStateInfo state = download->state_info();
-  if (!state.prompt_user_for_save_location) {
-    if (UserScript::IsURLUserScript(download->GetURL(),
-        download->mime_type()) ||
-        (download->mime_type() == Extension::kMimeType)) {
-      state.is_extension_install = true;
-    }
-  }
 
   if (state.force_file_name.empty()) {
     FilePath generated_name;
@@ -252,7 +307,7 @@ void ChromeDownloadManagerDelegate::CheckVisitedReferrerBeforeDone(
       //    "save as...".
       // 2) Filetypes marked "always open." If the user just wants this file
       //    opened, don't bother asking where to keep it.
-      if (!state.is_extension_install &&
+      if (!IsExtensionDownload(download) &&
           !ShouldOpenFileBasedOnExtension(generated_name))
         state.prompt_user_for_save_location = true;
     }
@@ -413,7 +468,7 @@ bool ChromeDownloadManagerDelegate::IsDangerousFile(
       (!state.has_user_gesture || !visited_referrer_before))
     return true;
 
-  if (state.is_extension_install) {
+  if (IsExtensionDownload(&download)) {
     // Extensions that are not from the gallery are considered dangerous.
     ExtensionService* service = profile_->GetExtensionService();
     if (!service || !service->IsDownloadFromGallery(download.GetURL(),
diff --git a/chrome/browser/download/chrome_download_manager_delegate.h b/chrome/browser/download/chrome_download_manager_delegate.h
index 9b6faab..f9e571c 100644
--- a/chrome/browser/download/chrome_download_manager_delegate.h
+++ b/chrome/browser/download/chrome_download_manager_delegate.h
@@ -7,11 +7,15 @@
 #pragma once
 
 #include "base/compiler_specific.h"
+#include "base/hash_tables.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/task.h"
 #include "content/browser/download/download_manager_delegate.h"
+#include "content/common/notification_observer.h"
+#include "content/common/notification_registrar.h"
 
+class CrxInstaller;
 class DownloadHistory;
 class DownloadItem;
 class DownloadManager;
@@ -19,13 +23,30 @@ class DownloadPrefs;
 class Profile;
 struct DownloadStateInfo;
 
+#if defined(COMPILER_GCC)
+namespace __gnu_cxx {
+
+template<>
+struct hash<CrxInstaller*> {
+  std::size_t operator()(CrxInstaller* const& p) const {
+    return reinterpret_cast<std::size_t>(p);
+  }
+};
+
+}  // namespace __gnu_cxx
+#endif
+
 // This is the Chrome side helper for the download system.
 class ChromeDownloadManagerDelegate
     : public base::RefCountedThreadSafe<ChromeDownloadManagerDelegate>,
-      public DownloadManagerDelegate {
+      public DownloadManagerDelegate,
+      public NotificationObserver {
  public:
   explicit ChromeDownloadManagerDelegate(Profile* profile);
 
+  // Returns true if the given item is for an extension download.
+  static bool IsExtensionDownload(const DownloadItem* item);
+
   void SetDownloadManager(DownloadManager* dm);
 
   virtual void Shutdown() OVERRIDE;
@@ -35,6 +56,8 @@ class ChromeDownloadManagerDelegate
                                   void* data) OVERRIDE;
   virtual TabContents* GetAlternativeTabContentsToNotifyForDownload() OVERRIDE;
   virtual bool ShouldOpenFileBasedOnExtension(const FilePath& path) OVERRIDE;
+  virtual bool ShouldOpenDownload(DownloadItem* item) OVERRIDE;
+  virtual bool ShouldCompleteDownload(DownloadItem* item) OVERRIDE;
   virtual bool GenerateFileHash() OVERRIDE;
   virtual void AddItemToPersistentStore(DownloadItem* item) OVERRIDE;
   virtual void UpdateItemInPersistentStore(DownloadItem* item) OVERRIDE;
@@ -60,6 +83,11 @@ class ChromeDownloadManagerDelegate
   friend class base::RefCountedThreadSafe<ChromeDownloadManagerDelegate>;
   virtual ~ChromeDownloadManagerDelegate();
 
+  // NotificationObserver implementation.
+  virtual void Observe(int type,
+                       const NotificationSource& source,
+                       const NotificationDetails& details) OVERRIDE;
+
   // Callback function after url is checked with safebrowsing service.
   void CheckDownloadUrlDone(int32 download_id, bool is_dangerous_url);
 
@@ -96,6 +124,12 @@ class ChromeDownloadManagerDelegate
   scoped_ptr<DownloadPrefs> download_prefs_;
   scoped_ptr<DownloadHistory> download_history_;
 
+  // Maps from pending extension installations to DownloadItem IDs.
+  typedef base::hash_map<CrxInstaller*, int> CrxInstallerMap;
+  CrxInstallerMap crx_installers_;
+
+  NotificationRegistrar registrar_;
+
   DISALLOW_COPY_AND_ASSIGN(ChromeDownloadManagerDelegate);
 };
 
diff --git a/chrome/browser/download/download_crx_util.cc b/chrome/browser/download/download_crx_util.cc
index 9b5b45a..574c9ab9 100644
--- a/chrome/browser/download/download_crx_util.cc
+++ b/chrome/browser/download/download_crx_util.cc
@@ -50,7 +50,6 @@ scoped_refptr<CrxInstaller> OpenChromeExtension(
     Profile* profile,
     const DownloadItem& download_item) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK(download_item.is_extension_install());
 
   ExtensionService* service = profile->GetExtensionService();
   CHECK(service);
diff --git a/chrome/browser/download/download_history.cc b/chrome/browser/download/download_history.cc
index f709e74..3da198b 100644
--- a/chrome/browser/download/download_history.cc
+++ b/chrome/browser/download/download_history.cc
@@ -5,6 +5,7 @@
 #include "chrome/browser/download/download_history.h"
 
 #include "base/logging.h"
+#include "chrome/browser/download/chrome_download_manager_delegate.h"
 #include "chrome/browser/history/history_marshaling.h"
 #include "chrome/browser/profiles/profile.h"
 #include "content/browser/download/download_item.h"
@@ -73,7 +74,8 @@ void DownloadHistory::AddEntry(
   // you'd have to do enough downloading that your ISP would likely stab you in
   // the neck first. YMMV.
   HistoryService* hs = profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
-  if (download_item->is_otr() || download_item->is_extension_install() ||
+  if (download_item->is_otr() ||
+      ChromeDownloadManagerDelegate::IsExtensionDownload(download_item) ||
       download_item->is_temporary() || !hs) {
     callback->RunWithParams(
         history::DownloadCreateRequest::TupleType(download_item->id(),
diff --git a/chrome/browser/download/download_item_model.cc b/chrome/browser/download/download_item_model.cc
index 7aeba8c..891a5ed 100644
--- a/chrome/browser/download/download_item_model.cc
+++ b/chrome/browser/download/download_item_model.cc
@@ -8,6 +8,7 @@
 #include "base/i18n/rtl.h"
 #include "base/string16.h"
 #include "base/utf_string_conversions.h"
+#include "chrome/browser/download/chrome_download_manager_delegate.h"
 #include "chrome/common/time_format.h"
 #include "content/browser/download/download_item.h"
 #include "content/browser/download/save_package.h"
@@ -56,7 +57,9 @@ string16 DownloadItemModel::GetStatusText() {
   string16 status_text;
   switch (download_->state()) {
     case DownloadItem::IN_PROGRESS:
-      if (download_->IsCrxInstallRuning()) {
+      if (ChromeDownloadManagerDelegate::IsExtensionDownload(download_) &&
+          download_->all_data_saved() &&
+          download_->state() == DownloadItem::IN_PROGRESS) {
         // The download is a CRX (app, extension, theme, ...) and it is
         // being unpacked and validated.
         status_text = l10n_util::GetStringUTF16(
diff --git a/chrome/browser/download/download_shelf_context_menu.cc b/chrome/browser/download/download_shelf_context_menu.cc
index bfdbcf2..de0a4f5 100644
--- a/chrome/browser/download/download_shelf_context_menu.cc
+++ b/chrome/browser/download/download_shelf_context_menu.cc
@@ -6,6 +6,7 @@
 
 #include "chrome/browser/download/download_item_model.h"
 #include "chrome/browser/download/download_prefs.h"
+#include "chrome/common/extensions/extension.h"
 #include "content/browser/download/download_item.h"
 #include "content/browser/download/download_manager.h"
 #include "grit/generated_resources.h"
@@ -30,7 +31,8 @@ bool DownloadShelfContextMenu::IsCommandIdEnabled(int command_id) const {
     case OPEN_WHEN_COMPLETE:
       return download_item_->CanShowInFolder();
     case ALWAYS_OPEN_TYPE:
-      return download_item_->CanOpenDownload();
+      return download_item_->CanOpenDownload() &&
+          !Extension::IsExtension(download_item_->state_info().target_name);
     case CANCEL:
       return download_item_->IsPartialDownload();
     case TOGGLE_PAUSE:
diff --git a/chrome/browser/download/mock_download_manager_delegate.cc b/chrome/browser/download/mock_download_manager_delegate.cc
index dba9613b..9ffa9e1 100644
--- a/chrome/browser/download/mock_download_manager_delegate.cc
+++ b/chrome/browser/download/mock_download_manager_delegate.cc
@@ -30,6 +30,14 @@ bool MockDownloadManagerDelegate::ShouldOpenFileBasedOnExtension(
   return false;
 }
 
+bool MockDownloadManagerDelegate::ShouldOpenDownload(DownloadItem* item) {
+  return true;
+}
+
+bool MockDownloadManagerDelegate::ShouldCompleteDownload(DownloadItem* item) {
+  return true;
+}
+
 bool MockDownloadManagerDelegate::GenerateFileHash() {
   return false;
 }
diff --git a/chrome/browser/download/mock_download_manager_delegate.h b/chrome/browser/download/mock_download_manager_delegate.h
index 68389d4..1ce087c 100644
--- a/chrome/browser/download/mock_download_manager_delegate.h
+++ b/chrome/browser/download/mock_download_manager_delegate.h
@@ -19,6 +19,8 @@ class MockDownloadManagerDelegate : public DownloadManagerDelegate {
                                   void* data) OVERRIDE;
   virtual TabContents* GetAlternativeTabContentsToNotifyForDownload() OVERRIDE;
   virtual bool ShouldOpenFileBasedOnExtension(const FilePath& path) OVERRIDE;
+  virtual bool ShouldOpenDownload(DownloadItem* item) OVERRIDE;
+  virtual bool ShouldCompleteDownload(DownloadItem* item) OVERRIDE;
   virtual bool GenerateFileHash() OVERRIDE;
   virtual void AddItemToPersistentStore(DownloadItem* item) OVERRIDE;
   virtual void UpdateItemInPersistentStore(DownloadItem* item) OVERRIDE;