Merge 87981 - Show that CRX unpacking is happening in the download shelf

BUG=80010
TEST=manual for now

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

TBR=skerner@chromium.org
Review URL: http://codereview.chromium.org/7054078

git-svn-id: svn://svn.chromium.org/chrome/branches/782/src@88011 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 114 insertions, 32 deletions

diff --git a/chrome/browser/download/download_item.cc b/chrome/browser/download/download_item.cc
index f2cc384..50bb84a 100644
--- a/chrome/browser/download/download_item.cc
+++ b/chrome/browser/download/download_item.cc
@@ -22,6 +22,7 @@
 #include "chrome/browser/download/download_prefs.h"
 #include "chrome/browser/download/download_state_info.h"
 #include "chrome/browser/download/download_util.h"
+#include "chrome/browser/extensions/crx_installer.h"
 #include "chrome/browser/history/download_history_info.h"
 #include "chrome/browser/platform_util.h"
 #include "chrome/browser/prefs/pref_service.h"
@@ -29,6 +30,7 @@
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/pref_names.h"
 #include "content/browser/browser_thread.h"
+#include "content/common/notification_source.h"
 #include "ui/base/l10n/l10n_util.h"
 
 // A DownloadItem normally goes through the following states:
@@ -257,9 +259,7 @@ void DownloadItem::OpenDownload() {
       return;
 
     if (is_extension_install()) {
-      download_util::OpenChromeExtension(download_manager_->profile(),
-                                         download_manager_,
-                                         *this);
+      download_util::OpenChromeExtension(download_manager_->profile(), *this);
       return;
     }
 #if defined(OS_MACOSX)
@@ -362,11 +362,8 @@ void DownloadItem::Completed() {
   download_manager_->DownloadCompleted(id());
   download_util::RecordDownloadCount(download_util::COMPLETED_COUNT);
 
-  // Handle chrome extensions explicitly and skip the shell execute.
   if (is_extension_install()) {
-    download_util::OpenChromeExtension(download_manager_->profile(),
-                                       download_manager_,
-                                       *this);
+    // Extensions should already have been unpacked and opened.
     auto_opened_ = true;
   } else if (open_when_complete() ||
              download_manager_->ShouldOpenFileBasedOnExtension(
@@ -383,6 +380,45 @@ void DownloadItem::Completed() {
   }
 }
 
+void DownloadItem::StartCrxInstall() {
+  DCHECK(is_extension_install());
+  DCHECK(all_data_saved_);
+
+  scoped_refptr<CrxInstaller> crx_installer =
+      download_util::OpenChromeExtension(
+          download_manager_->profile(),
+          *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,
+                 NotificationType::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();
+}
+
+// NotificationObserver implementation.
+void DownloadItem::Observe(NotificationType type,
+                           const NotificationSource& source,
+                           const NotificationDetails& details) {
+  DCHECK(type == NotificationType::CRX_INSTALLER_DONE);
+
+  // No need to listen for CRX_INSTALLER_DONE anymore.
+  registrar_.Remove(this,
+                    NotificationType::CRX_INSTALLER_DONE,
+                    source);
+
+  auto_opened_ = true;
+  DCHECK(all_data_saved_);
+
+  Completed();
+}
+
 void DownloadItem::Interrupted(int64 size, int os_error) {
   if (!IsInProgress())
     return;
@@ -442,10 +478,14 @@ int64 DownloadItem::CurrentSpeed() const {
 }
 
 int DownloadItem::PercentComplete() const {
-  return (total_bytes_ > 0) ?
-      static_cast<int>(received_bytes_ * 100.0 /
-          total_bytes_) :
-      -1;
+  // 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)
+    return -1;
+
+  return static_cast<int>(received_bytes_ * 100.0 / total_bytes_);
 }
 
 void DownloadItem::Rename(const FilePath& full_path) {
@@ -478,6 +518,7 @@ void DownloadItem::OnDownloadCompleting(DownloadFileManager* file_manager) {
     return;
   }
 
+  DCHECK(!is_extension_install());
   Completed();
 
   BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
@@ -494,6 +535,12 @@ void DownloadItem::OnDownloadRenamedToFinalName(const FilePath& full_path) {
 
   Rename(full_path);
 
+  if (is_extension_install()) {
+    StartCrxInstall();
+    // Completed() will be called when the installer finishes.
+    return;
+  }
+
   Completed();
 }
 
diff --git a/chrome/browser/download/download_item.h b/chrome/browser/download/download_item.h
index 93acbf3..0178d3b 100644
--- a/chrome/browser/download/download_item.h
+++ b/chrome/browser/download/download_item.h
@@ -27,8 +27,11 @@
 #include "base/timer.h"
 #include "chrome/browser/download/download_process_handle.h"
 #include "chrome/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;
@@ -39,7 +42,7 @@ struct DownloadHistoryInfo;
 // Destination tab's download view, may refer to a given DownloadItem.
 //
 // This is intended to be used only on the UI thread.
-class DownloadItem {
+class DownloadItem : public NotificationObserver {
  public:
   enum DownloadState {
     // Download is actively progressing.
@@ -127,6 +130,11 @@ class DownloadItem {
   // Notifies our observers periodically.
   void UpdateObservers();
 
+  // NotificationObserver implementation.
+  virtual void Observe(NotificationType type,
+                       const NotificationSource& source,
+                       const NotificationDetails& details);
+
   // Returns true if it is OK to open this download.
   bool CanOpenDownload();
 
@@ -315,6 +323,13 @@ class DownloadItem {
     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
@@ -336,6 +351,10 @@ class DownloadItem {
   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();
+
   // State information used by the download manager.
   DownloadStateInfo state_info_;
 
@@ -439,6 +458,9 @@ class DownloadItem {
   // only.
   bool open_enabled_;
 
+  // DownloadItem observes CRX installs it initiates.
+  NotificationRegistrar registrar_;
+
   DISALLOW_COPY_AND_ASSIGN(DownloadItem);
 };
 
diff --git a/chrome/browser/download/download_item_model.cc b/chrome/browser/download/download_item_model.cc
index 760d136..bbac94b 100644
--- a/chrome/browser/download/download_item_model.cc
+++ b/chrome/browser/download/download_item_model.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -55,7 +55,12 @@ string16 DownloadItemModel::GetStatusText() {
   string16 status_text;
   switch (download_->state()) {
     case DownloadItem::IN_PROGRESS:
-      if (download_->open_when_complete()) {
+      if (download_->IsCrxInstallRuning()) {
+        // The download is a CRX (app, extension, theme, ...) and it is
+        // being unpacked and validated.
+        status_text = l10n_util::GetStringUTF16(
+            IDS_DOWNLOAD_STATUS_CRX_INSTALL_RUNNING);
+      } else if (download_->open_when_complete()) {
         if (simple_time.empty()) {
           status_text =
               l10n_util::GetStringUTF16(IDS_DOWNLOAD_STATUS_OPEN_WHEN_COMPLETE);
diff --git a/chrome/browser/download/download_util.cc b/chrome/browser/download/download_util.cc
index 09eda93..c00c250 100644
--- a/chrome/browser/download/download_util.cc
+++ b/chrome/browser/download/download_util.cc
@@ -333,9 +333,9 @@ void GenerateSafeFileName(const std::string& mime_type, FilePath* file_name) {
 #endif
 }
 
-void OpenChromeExtension(Profile* profile,
-                         DownloadManager* download_manager,
-                         const DownloadItem& download_item) {
+scoped_refptr<CrxInstaller> OpenChromeExtension(
+    Profile* profile,
+    const DownloadItem& download_item) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(download_item.is_extension_install());
 
@@ -344,7 +344,7 @@ void OpenChromeExtension(Profile* profile,
   NotificationService* nservice = NotificationService::current();
   GURL nonconst_download_url = download_item.GetURL();
   nservice->Notify(NotificationType::EXTENSION_READY_FOR_INSTALL,
-                   Source<DownloadManager>(download_manager),
+                   Source<DownloadManager>(profile->GetDownloadManager()),
                    Details<GURL>(&nonconst_download_url));
 
   scoped_refptr<CrxInstaller> installer(
@@ -355,18 +355,19 @@ void OpenChromeExtension(Profile* profile,
                                   download_item.mime_type())) {
     installer->InstallUserScript(download_item.full_path(),
                                  download_item.GetURL());
-    return;
+  } else {
+    bool is_gallery_download = service->IsDownloadFromGallery(
+        download_item.GetURL(), download_item.referrer_url());
+    installer->set_original_mime_type(download_item.original_mime_type());
+    installer->set_apps_require_extension_mime_type(true);
+    installer->set_original_url(download_item.GetURL());
+    installer->set_is_gallery_install(is_gallery_download);
+    installer->set_allow_silent_install(is_gallery_download);
+    installer->set_install_cause(extension_misc::INSTALL_CAUSE_USER_DOWNLOAD);
+    installer->InstallCrx(download_item.full_path());
   }
 
-  bool is_gallery_download = service->IsDownloadFromGallery(
-      download_item.GetURL(), download_item.referrer_url());
-  installer->set_original_mime_type(download_item.original_mime_type());
-  installer->set_apps_require_extension_mime_type(true);
-  installer->set_original_url(download_item.GetURL());
-  installer->set_is_gallery_install(is_gallery_download);
-  installer->set_allow_silent_install(is_gallery_download);
-  installer->set_install_cause(extension_misc::INSTALL_CAUSE_USER_DOWNLOAD);
-  installer->InstallCrx(download_item.full_path());
+  return installer;
 }
 
 void RecordDownloadCount(DownloadCountTypes type) {
diff --git a/chrome/browser/download/download_util.h b/chrome/browser/download/download_util.h
index 3fedb5a..6880d70 100644
--- a/chrome/browser/download/download_util.h
+++ b/chrome/browser/download/download_util.h
@@ -12,6 +12,7 @@
 
 #include "base/basictypes.h"
 #include "base/file_path.h"
+#include "base/memory/ref_counted.h"
 #include "base/string16.h"
 #include "chrome/browser/download/download_process_handle.h"
 #include "ui/gfx/native_widget_types.h"
@@ -21,6 +22,7 @@
 #endif
 
 class BaseDownloadItemModel;
+class CrxInstaller;
 class DictionaryValue;
 class DownloadItem;
 class DownloadManager;
@@ -83,10 +85,13 @@ void GenerateFileName(const GURL& url,
 // full path to a file.
 void GenerateSafeFileName(const std::string& mime_type, FilePath* file_name);
 
-// Opens downloaded Chrome extension file (*.crx).
-void OpenChromeExtension(Profile* profile,
-                         DownloadManager* download_manager,
-                         const DownloadItem& download_item);
+// Start installing a downloaded item item as a CRX (extension, theme, app,
+// ...).  The installer does work on the file thread, so the installation
+// is not complete when this function returns.  Returns the object managing
+// the installation.
+scoped_refptr<CrxInstaller> OpenChromeExtension(
+    Profile* profile,
+    const DownloadItem& download_item);
 
 // Download progress animations ------------------------------------------------
 
diff --git a/chrome/browser/extensions/crx_installer.cc b/chrome/browser/extensions/crx_installer.cc
index 1928545..86a7f39 100644
--- a/chrome/browser/extensions/crx_installer.cc
+++ b/chrome/browser/extensions/crx_installer.cc
@@ -443,6 +443,8 @@ void CrxInstaller::InstallUIAbort() {
                   NotificationService::NoDetails());
   Release();  // balanced in ConfirmInstall().
 
+  NotifyCrxInstallComplete();
+
   // We're done. Since we don't post any more tasks to ourself, our ref count
   // should go to zero and we die. The destructor will clean up the temp dir.
 }