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

11 files changed, 61 insertions, 141 deletions

diff --git a/content/browser/download/download_create_info.cc b/content/browser/download/download_create_info.cc
index 4b556ab..af87041 100644
--- a/content/browser/download/download_create_info.cc
+++ b/content/browser/download/download_create_info.cc
@@ -27,8 +27,7 @@ DownloadCreateInfo::DownloadCreateInfo(const FilePath& path,
       download_id(download_id),
       has_user_gesture(has_user_gesture),
       db_handle(0),
-      prompt_user_for_save_location(false),
-      is_extension_install(false) {
+      prompt_user_for_save_location(false) {
 }
 
 DownloadCreateInfo::DownloadCreateInfo()
@@ -39,8 +38,7 @@ DownloadCreateInfo::DownloadCreateInfo()
       download_id(-1),
       has_user_gesture(false),
       db_handle(0),
-      prompt_user_for_save_location(false),
-      is_extension_install(false) {
+      prompt_user_for_save_location(false) {
 }
 
 DownloadCreateInfo::~DownloadCreateInfo() {
diff --git a/content/browser/download/download_create_info.h b/content/browser/download/download_create_info.h
index d2e5cd6..770ac264 100644
--- a/content/browser/download/download_create_info.h
+++ b/content/browser/download/download_create_info.h
@@ -95,9 +95,6 @@ struct DownloadCreateInfo {
   // The original name for a dangerous download.
   FilePath original_name;
 
-  // Whether this download is for extension install or not.
-  bool is_extension_install;
-
   // The charset of the referring page where the download request comes from.
   // It's used to construct a suggested filename.
   std::string referrer_charset;
diff --git a/content/browser/download/download_item.cc b/content/browser/download/download_item.cc
index 9126d7a..4374c04 100644
--- a/content/browser/download/download_item.cc
+++ b/content/browser/download/download_item.cc
@@ -14,13 +14,7 @@
 #include "base/timer.h"
 #include "base/utf_string_conversions.h"
 #include "net/base/net_util.h"
-#include "chrome/browser/download/download_crx_util.h"
-#include "chrome/browser/download/download_extensions.h"
 #include "chrome/browser/download/download_util.h"
-#include "chrome/browser/extensions/crx_installer.h"
-#include "chrome/browser/profiles/profile.h"
-#include "chrome/common/chrome_notification_types.h"
-#include "chrome/common/extensions/extension.h"
 #include "content/browser/browser_thread.h"
 #include "content/browser/content_browser_client.h"
 #include "content/browser/download/download_create_info.h"
@@ -30,7 +24,6 @@
 #include "content/browser/download/download_persistent_store_info.h"
 #include "content/browser/download/download_request_handle.h"
 #include "content/browser/download/download_stats.h"
-#include "content/common/notification_source.h"
 
 // A DownloadItem normally goes through the following states:
 //      * Created (when download starts)
@@ -145,7 +138,8 @@ DownloadItem::DownloadItem(DownloadManager* download_manager,
       is_temporary_(false),
       all_data_saved_(false),
       opened_(false),
-      open_enabled_(true) {
+      open_enabled_(true),
+      delegate_delayed_complete_(false) {
   if (IsInProgress())
     state_ = CANCELLED;
   if (IsComplete())
@@ -159,8 +153,7 @@ DownloadItem::DownloadItem(DownloadManager* download_manager,
                            bool is_otr)
     : state_info_(info.original_name, info.save_info.file_path,
                   info.has_user_gesture, info.prompt_user_for_save_location,
-                  info.path_uniquifier, false, false,
-                  info.is_extension_install),
+                  info.path_uniquifier, false, false),
       request_handle_(info.request_handle),
       download_id_(info.download_id),
       full_path_(info.path),
@@ -257,8 +250,7 @@ bool DownloadItem::CanShowInFolder() {
 }
 
 bool DownloadItem::CanOpenDownload() {
-  return !Extension::IsExtension(state_info_.target_name) &&
-    !file_externally_removed_;
+  return !file_externally_removed_;
 }
 
 bool DownloadItem::ShouldOpenFileBasedOnExtension() {
@@ -291,14 +283,8 @@ void DownloadItem::OpenDownload() {
   if (!open_enabled_)
     return;
 
-  if (is_extension_install()) {
-    download_crx_util::OpenChromeExtension(
-        Profile::FromBrowserContext(download_manager_->browser_context()),
-        *this);
-    return;
-  }
-
-  content::GetContentClient()->browser()->OpenItem(full_path());
+  if (download_manager_->delegate()->ShouldOpenDownload(this))
+    content::GetContentClient()->browser()->OpenItem(full_path());
 }
 
 void DownloadItem::ShowDownloadInShell() {
@@ -392,6 +378,11 @@ void DownloadItem::MarkAsComplete() {
   TransitionTo(COMPLETE);
 }
 
+void DownloadItem::CompleteDelayedDownload() {
+  auto_opened_ = true;
+  Completed();
+}
+
 void DownloadItem::OnAllDataSaved(int64 size) {
   // TODO(rdsmith): Change to DCHECK after http://crbug.com/85408 resolved.
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ -418,9 +409,8 @@ void DownloadItem::Completed() {
   download_manager_->DownloadCompleted(id());
   download_stats::RecordDownloadCompleted(start_tick_);
 
-  if (is_extension_install()) {
-    // Extensions should already have been unpacked and opened.
-    DCHECK(auto_opened_);
+  if (auto_opened_) {
+    // If it was already handled by the delegate, do nothing.
   } else if (open_when_complete() ||
              ShouldOpenFileBasedOnExtension() ||
              is_temporary()) {
@@ -435,31 +425,6 @@ void DownloadItem::Completed() {
   }
 }
 
-void DownloadItem::StartCrxInstall() {
-  // TODO(rdsmith): Change to DCHECK after http://crbug.com/85408 resolved.
-  CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
-  DCHECK(is_extension_install());
-  DCHECK(all_data_saved_);
-
-  scoped_refptr<CrxInstaller> crx_installer =
-      download_crx_util::OpenChromeExtension(
-          Profile::FromBrowserContext(download_manager_->browser_context()),
-          *this);
-
-  // CRX_INSTALLER_DONE will fire when the install completes.  Observe()
-  // will call Completed() 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()));
-
-  // 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.
-  UpdateObservers();
-}
-
 void DownloadItem::TransitionTo(DownloadState new_state) {
   if (state_ == new_state)
     return;
@@ -486,26 +451,6 @@ void DownloadItem::UpdateTarget() {
     state_info_.target_name = full_path_.BaseName();
 }
 
-// NotificationObserver implementation.
-void DownloadItem::Observe(int type,
-                           const NotificationSource& source,
-                           const NotificationDetails& details) {
-  // TODO(rdsmith): Change to DCHECK after http://crbug.com/85408 resolved.
-  CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
-  DCHECK(type == chrome::NOTIFICATION_CRX_INSTALLER_DONE);
-
-  // No need to listen for CRX_INSTALLER_DONE anymore.
-  registrar_.Remove(this,
-                    chrome::NOTIFICATION_CRX_INSTALLER_DONE,
-                    source);
-
-  auto_opened_ = true;
-  DCHECK(all_data_saved_);
-
-  Completed();
-}
-
 void DownloadItem::Interrupted(int64 size, int os_error) {
   // TODO(rdsmith): Change to DCHECK after http://crbug.com/85408 resolved.
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ -580,11 +525,9 @@ int64 DownloadItem::CurrentSpeed() const {
 }
 
 int DownloadItem::PercentComplete() const {
-  // We don't have an accurate way to estimate the time to unpack a CRX.
-  // The slowest part is re-encoding images, and time to do this depends on
-  // the contents of the image.  If a CRX is being unpacked, indicate that
-  // we do not know how close to completion we are.
-  if (IsCrxInstallRuning() || total_bytes_ <= 0)
+  // If the delegate is delaying completion of the download, then we have no
+  // idea how long it will take.
+  if (delegate_delayed_complete_ || total_bytes_ <= 0)
     return -1;
 
   return static_cast<int>(received_bytes_ * 100.0 / total_bytes_);
@@ -637,7 +580,6 @@ void DownloadItem::OnDownloadCompleting(DownloadFileManager* file_manager) {
     return;
   }
 
-  DCHECK(!is_extension_install());
   Completed();
 
   BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
@@ -657,13 +599,11 @@ void DownloadItem::OnDownloadRenamedToFinalName(const FilePath& full_path) {
 
   Rename(full_path);
 
-  if (is_extension_install()) {
-    StartCrxInstall();
-    // Completed() will be called when the installer finishes.
-    return;
+  if (download_manager_->delegate()->ShouldCompleteDownload(this)) {
+    Completed();
+  } else {
+    delegate_delayed_complete_ = true;
   }
-
-  Completed();
 }
 
 bool DownloadItem::MatchesQuery(const string16& query) const {
@@ -830,7 +770,6 @@ std::string DownloadItem::DebugString(bool verbose) const {
         " total_bytes = %" PRId64
         " received_bytes = %" PRId64
         " is_paused = %c"
-        " is_extension_install = %c"
         " is_otr = %c"
         " safety_state = %s"
         " url_chain = \n\t\"%s\"\n\t"
@@ -840,7 +779,6 @@ std::string DownloadItem::DebugString(bool verbose) const {
         total_bytes(),
         received_bytes(),
         is_paused() ? 'T' : 'F',
-        is_extension_install() ? 'T' : 'F',
         is_otr() ? 'T' : 'F',
         DebugSafetyStateString(safety_state()),
         url_list.c_str(),
diff --git a/content/browser/download/download_item.h b/content/browser/download/download_item.h
index 5b2ae0a..40fdfb2 100644
--- a/content/browser/download/download_item.h
+++ b/content/browser/download/download_item.h
@@ -27,11 +27,8 @@
 #include "base/timer.h"
 #include "content/browser/download/download_request_handle.h"
 #include "content/browser/download/download_state_info.h"
-#include "content/common/notification_observer.h"
-#include "content/common/notification_registrar.h"
 #include "googleurl/src/gurl.h"
 
-class CrxInstaller;
 class DownloadFileManager;
 class DownloadManager;
 struct DownloadCreateInfo;
@@ -42,7 +39,7 @@ struct DownloadPersistentStoreInfo;
 // Destination tab's download view, may refer to a given DownloadItem.
 //
 // This is intended to be used only on the UI thread.
-class DownloadItem : public NotificationObserver {
+class DownloadItem {
  public:
   enum DownloadState {
     // Download is actively progressing.
@@ -135,11 +132,6 @@ class DownloadItem : public NotificationObserver {
   // Notifies our observers periodically.
   void UpdateObservers();
 
-  // NotificationObserver implementation.
-  virtual void Observe(int type,
-                       const NotificationSource& source,
-                       const NotificationDetails& details);
-
   // Returns true if it is OK to open a folder which this file is inside.
   bool CanShowInFolder();
 
@@ -178,6 +170,10 @@ class DownloadItem : public NotificationObserver {
   // to display progress when the DownloadItem should be considered complete.
   void MarkAsComplete();
 
+  // Called by the delegate after it delayed completing the download in 
+  // DownloadManagerDelegate::ShouldCompleteDownload.
+  void CompleteDelayedDownload();
+
   // Called when all data has been saved. Only has display effects.
   void OnAllDataSaved(int64 size);
 
@@ -298,9 +294,6 @@ class DownloadItem : public NotificationObserver {
     return state_info_.prompt_user_for_save_location;
   }
   bool is_otr() const { return is_otr_; }
-  bool is_extension_install() const {
-    return state_info_.is_extension_install;
-  }
   const FilePath& suggested_path() const { return state_info_.suggested_path; }
   bool is_temporary() const { return is_temporary_; }
   void set_opened(bool opened) { opened_ = opened; }
@@ -329,13 +322,6 @@ class DownloadItem : public NotificationObserver {
     return state_info_.target_name != full_path_.BaseName();
   }
 
-  // Is a CRX installer running on this download?
-  bool IsCrxInstallRuning() const {
-    return (is_extension_install() &&
-            all_data_saved() &&
-            state_ == IN_PROGRESS);
-  }
-
   std::string DebugString(bool verbose) const;
 
 #ifdef UNIT_TEST
@@ -359,10 +345,6 @@ class DownloadItem : public NotificationObserver {
   void StartProgressTimer();
   void StopProgressTimer();
 
-  // Call to install this item as a CRX. Should only be called on
-  // items which are CRXes. Use is_extension_install() to check.
-  void StartCrxInstall();
-
   // Call to transition state; all state transitions should go through this.
   void TransitionTo(DownloadState new_state);
 
@@ -485,8 +467,8 @@ class DownloadItem : public NotificationObserver {
   // only.
   bool open_enabled_;
 
-  // DownloadItem observes CRX installs it initiates.
-  NotificationRegistrar registrar_;
+  // Did the delegate delay calling Complete on this download?
+  bool delegate_delayed_complete_;
 
   DISALLOW_COPY_AND_ASSIGN(DownloadItem);
 };
diff --git a/content/browser/download/download_manager.cc b/content/browser/download/download_manager.cc
index e211c96..5367038 100644
--- a/content/browser/download/download_manager.cc
+++ b/content/browser/download/download_manager.cc
@@ -163,7 +163,7 @@ void DownloadManager::SearchDownloads(const string16& query,
        it != history_downloads_.end(); ++it) {
     DownloadItem* download_item = it->second;
 
-    if (download_item->is_temporary() || download_item->is_extension_install())
+    if (download_item->is_temporary())
       continue;
 
     // Display Incognito downloads only in Incognito window, and vice versa.
diff --git a/content/browser/download/download_manager_delegate.h b/content/browser/download/download_manager_delegate.h
index fd2e2d6..13de586 100644
--- a/content/browser/download/download_manager_delegate.h
+++ b/content/browser/download/download_manager_delegate.h
@@ -35,36 +35,45 @@ class DownloadManagerDelegate {
 
   // Called when the download system wants to alert a TabContents that a
   // download has started, but the TabConetnts has gone away. This lets an
-  // embedder return an alternative TabContents. The embedder can return NULL.
+  // delegate return an alternative TabContents. The delegate can return NULL.
   virtual TabContents* GetAlternativeTabContentsToNotifyForDownload() = 0;
 
   // Tests if a file type should be opened automatically.
   virtual bool ShouldOpenFileBasedOnExtension(const FilePath& path) = 0;
 
+  // Allows the delegate to override the opening of a download. If it returns
+  // true then it's reponsible for opening the item.
+  virtual bool ShouldOpenDownload(DownloadItem* item) = 0;
+
+  // Allows the delegate to override completing the download. The delegate needs
+  // to call DownloadItem::CompleteDelayedDownload when it's done with the item,
+  // and is responsible for opening it.
+  virtual bool ShouldCompleteDownload(DownloadItem* item) = 0;
+
   // Returns true if we need to generate a binary hash for downloads.
   virtual bool GenerateFileHash() = 0;
 
-  // Notifies the embedder that a new download item is created. The
-  // DownloadManager waits for the embedder to add information about this
-  // download to its persistent store. When the embedder is done, it calls
+  // Notifies the delegate that a new download item is created. The
+  // DownloadManager waits for the delegate to add information about this
+  // download to its persistent store. When the delegate is done, it calls
   // DownloadManager::OnDownloadItemAddedToPersistentStore.
   virtual void AddItemToPersistentStore(DownloadItem* item) = 0;
 
-  // Notifies the embedder that information about the given download has change,
+  // Notifies the delegate that information about the given download has change,
   // so that it can update its persistent store.
   virtual void UpdateItemInPersistentStore(DownloadItem* item) = 0;
 
-  // Notifies the embedder that path for the download item has changed, so that
+  // Notifies the delegate that path for the download item has changed, so that
   // it can update its persistent store.
   virtual void UpdatePathForItemInPersistentStore(
       DownloadItem* item,
       const FilePath& new_path) = 0;
 
-  // Notifies the embedder that it should remove the download item from its
+  // Notifies the delegate that it should remove the download item from its
   // persistent store.
   virtual void RemoveItemFromPersistentStore(DownloadItem* item) = 0;
 
-  // Notifies the embedder to remove downloads from the given time range.
+  // Notifies the delegate to remove downloads from the given time range.
   virtual void RemoveItemsFromPersistentStoreBetween(
       const base::Time remove_begin,
       const base::Time remove_end) = 0;
diff --git a/content/browser/download/download_request_handle.cc b/content/browser/download/download_request_handle.cc
index 83b0df60..e5d9b60 100644
--- a/content/browser/download/download_request_handle.cc
+++ b/content/browser/download/download_request_handle.cc
@@ -5,9 +5,9 @@
 #include "content/browser/download/download_request_handle.h"
 
 #include "base/stringprintf.h"
-#include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/tab_contents/tab_util.h"
+#include "content/browser/browser_context.h"
 #include "content/browser/browser_thread.h"
+#include "content/browser/renderer_host/render_view_host.h"
 #include "content/browser/renderer_host/resource_dispatcher_host.h"
 #include "content/browser/tab_contents/tab_contents.h"
 
@@ -52,8 +52,12 @@ DownloadRequestHandle::DownloadRequestHandle(ResourceDispatcherHost* rdh,
 }
 
 TabContents* DownloadRequestHandle::GetTabContents() const {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  return tab_util::GetTabContentsByID(child_id_, render_view_id_);
+  RenderViewHost* render_view_host =
+      RenderViewHost::FromID(child_id_, render_view_id_);
+  if (!render_view_host)
+    return NULL;
+
+  return render_view_host->delegate()->GetAsTabContents();
 }
 
 DownloadManager* DownloadRequestHandle::GetDownloadManager() const {
diff --git a/content/browser/download/download_state_info.cc b/content/browser/download/download_state_info.cc
index 0631129..9e8c3a2 100644
--- a/content/browser/download/download_state_info.cc
+++ b/content/browser/download/download_state_info.cc
@@ -11,8 +11,7 @@ DownloadStateInfo::DownloadStateInfo()
       has_user_gesture(false),
       prompt_user_for_save_location(false),
       is_dangerous_file(false),
-      is_dangerous_url(false),
-      is_extension_install(false) {
+      is_dangerous_url(false) {
 }
 
 DownloadStateInfo::DownloadStateInfo(
@@ -22,8 +21,7 @@ DownloadStateInfo::DownloadStateInfo(
       has_user_gesture(has_user_gesture),
       prompt_user_for_save_location(prompt_user_for_save_location),
       is_dangerous_file(false),
-      is_dangerous_url(false),
-      is_extension_install(false) {
+      is_dangerous_url(false) {
 }
 
 DownloadStateInfo::DownloadStateInfo(
@@ -33,15 +31,13 @@ DownloadStateInfo::DownloadStateInfo(
     bool prompt_user_for_save_location,
     int uniquifier,
     bool dangerous_file,
-    bool dangerous_url,
-    bool extension_install)
+    bool dangerous_url)
     : target_name(target),
       path_uniquifier(uniquifier),
       has_user_gesture(has_user_gesture),
       prompt_user_for_save_location(prompt_user_for_save_location),
       is_dangerous_file(dangerous_file),
       is_dangerous_url(dangerous_url),
-      is_extension_install(extension_install),
       force_file_name(forced_name) {
 }
 
diff --git a/content/browser/download/download_state_info.h b/content/browser/download/download_state_info.h
index 5ca4c40..dca01c3 100644
--- a/content/browser/download/download_state_info.h
+++ b/content/browser/download/download_state_info.h
@@ -20,8 +20,7 @@ struct DownloadStateInfo {
                     bool prompt_user_for_save_location,
                     int uniquifier,
                     bool dangerous_file,
-                    bool dangerous_url,
-                    bool extension_install);
+                    bool dangerous_url);
 
   // Indicates if the download is dangerous.
   bool IsDangerous() const;
@@ -52,9 +51,6 @@ struct DownloadStateInfo {
   // If safebrowsing believes this URL leads to malware.
   bool is_dangerous_url;
 
-  // True if this download is for extension install.
-  bool is_extension_install;
-
   // True if this download's file name was specified initially.
   FilePath force_file_name;
 };
diff --git a/content/browser/zygote_host_linux.cc b/content/browser/zygote_host_linux.cc
index dc2fc4e..9d1d02d 100644
--- a/content/browser/zygote_host_linux.cc
+++ b/content/browser/zygote_host_linux.cc
@@ -172,7 +172,7 @@ void ZygoteHost::Init(const std::string& sandbox_cmd) {
     // In the SUID sandbox, the real zygote is forked from the sandbox.
     // We need to look for it.
     // But first, wait for the zygote to tell us it's running.
-    // The sending code is in chrome/browser/zygote_main_linux.cc.
+    // The sending code is in content/browser/zygote_main_linux.cc.
     std::vector<int> fds_vec;
     const int kExpectedLength = sizeof(kZygoteMagic);
     char buf[kExpectedLength];
diff --git a/content/browser/zygote_main_linux.cc b/content/browser/zygote_main_linux.cc
index aa2d9f9..aa363b6 100644
--- a/content/browser/zygote_main_linux.cc
+++ b/content/browser/zygote_main_linux.cc
@@ -111,7 +111,7 @@ class Zygote {
 
     if (g_suid_sandbox_active) {
       // Let the ZygoteHost know we are ready to go.
-      // The receiving code is in chrome/browser/zygote_host_linux.cc.
+      // The receiving code is in content/browser/zygote_host_linux.cc.
       std::vector<int> empty;
       bool r = UnixDomainSocket::SendMsg(kBrowserDescriptor, kZygoteMagic,
                                          sizeof(kZygoteMagic), empty);