Create DownloadStatusUpdater to keep track of download progress

and update the application icon.

We can't do that in DownloadManager because there are multiple
DownloadManagers (one per Profile) and they would overwrite
each other's updates.

BUG=none
TEST=none

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

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

12 files changed, 378 insertions, 37 deletions

diff --git a/chrome/browser/browser_process.h b/chrome/browser/browser_process.h
index ff9599a..e417f2d 100644
--- a/chrome/browser/browser_process.h
+++ b/chrome/browser/browser_process.h
@@ -21,6 +21,7 @@ class AutomationProviderList;
 class Clipboard;
 class DevToolsManager;
 class DownloadRequestLimiter;
+class DownloadStatusUpdater;
 class GoogleURLTracker;
 class IntranetRedirectDetector;
 class IconManager;
@@ -124,6 +125,7 @@ class BrowserProcess {
   virtual void SetApplicationLocale(const std::string& locale) = 0;
 
   DownloadRequestLimiter* download_request_limiter();
+  virtual DownloadStatusUpdater* download_status_updater() = 0;
 
   // Returns an event that is signaled when the browser shutdown.
   virtual base::WaitableEvent* shutdown_event() = 0;
diff --git a/chrome/browser/browser_process_impl.cc b/chrome/browser/browser_process_impl.cc
index e9f2b83..88b41a9 100644
--- a/chrome/browser/browser_process_impl.cc
+++ b/chrome/browser/browser_process_impl.cc
@@ -458,6 +458,10 @@ void BrowserProcessImpl::SetApplicationLocale(const std::string& locale) {
   extension_l10n_util::SetProcessLocale(locale);
 }
 
+DownloadStatusUpdater* BrowserProcessImpl::download_status_updater() {
+  return &download_status_updater_;
+}
+
 base::WaitableEvent* BrowserProcessImpl::shutdown_event() {
   return shutdown_event_.get();
 }
diff --git a/chrome/browser/browser_process_impl.h b/chrome/browser/browser_process_impl.h
index 109b8e6..898a1b9 100644
--- a/chrome/browser/browser_process_impl.h
+++ b/chrome/browser/browser_process_impl.h
@@ -19,6 +19,7 @@
 #include "base/timer.h"
 #include "base/scoped_ptr.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/download/download_status_updater.h"
 #include "chrome/browser/prefs/pref_change_registrar.h"
 #include "chrome/browser/tab_contents/thumbnail_generator.h"
 #include "ipc/ipc_message.h"
@@ -65,6 +66,7 @@ class BrowserProcessImpl : public BrowserProcess, public NonThreadSafe {
   virtual IntranetRedirectDetector* intranet_redirect_detector();
   virtual const std::string& GetApplicationLocale();
   virtual void SetApplicationLocale(const std::string& locale);
+  virtual DownloadStatusUpdater* download_status_updater();
   virtual base::WaitableEvent* shutdown_event();
   virtual TabCloseableStateWatcher* tab_closeable_state_watcher();
   virtual void CheckForInspectorFiles();
@@ -186,6 +188,11 @@ class BrowserProcessImpl : public BrowserProcess, public NonThreadSafe {
   // nothing in the constructor so we don't have to worry about lazy init.
   ThumbnailGenerator thumbnail_generator_;
 
+  // Download status updates (like a changing application icon on dock/taskbar)
+  // are global per-application. DownloadStatusUpdater does no work in the ctor
+  // so we don't have to worry about lazy initialization.
+  DownloadStatusUpdater download_status_updater_;
+
   // An event that notifies when we are shutting-down.
   scoped_ptr<base::WaitableEvent> shutdown_event_;
 
diff --git a/chrome/browser/download/download_manager.cc b/chrome/browser/download/download_manager.cc
index 190894a..155df46 100644
--- a/chrome/browser/download/download_manager.cc
+++ b/chrome/browser/download/download_manager.cc
@@ -24,6 +24,7 @@
 #include "chrome/browser/download/download_history.h"
 #include "chrome/browser/download/download_item.h"
 #include "chrome/browser/download/download_prefs.h"
+#include "chrome/browser/download/download_status_updater.h"
 #include "chrome/browser/download/download_util.h"
 #include "chrome/browser/extensions/extensions_service.h"
 #include "chrome/browser/history/download_create_info.h"
@@ -49,14 +50,19 @@
 #include "app/win_util.h"
 #endif
 
-DownloadManager::DownloadManager()
+DownloadManager::DownloadManager(DownloadStatusUpdater* status_updater)
     : shutdown_needed_(false),
       profile_(NULL),
-      file_manager_(NULL) {
+      file_manager_(NULL),
+      status_updater_(status_updater) {
+  if (status_updater_)
+    status_updater_->AddDelegate(this);
 }
 
 DownloadManager::~DownloadManager() {
   DCHECK(!shutdown_needed_);
+  if (status_updater_)
+    status_updater_->RemoveDelegate(this);
 }
 
 void DownloadManager::Shutdown() {
@@ -661,33 +667,8 @@ void DownloadManager::PauseDownload(int32 download_id, bool pause) {
 }
 
 void DownloadManager::UpdateAppIcon() {
-  int64 total_bytes = 0;
-  int64 received_bytes = 0;
-  int download_count = 0;
-  bool progress_known = true;
-
-  for (DownloadMap::iterator i = in_progress_.begin();
-       i != in_progress_.end();
-       ++i) {
-    ++download_count;
-    const DownloadItem* item = i->second;
-    if (item->total_bytes() > 0) {
-      total_bytes += item->total_bytes();
-      received_bytes += item->received_bytes();
-    } else {
-      // This download didn't specify a Content-Length, so the combined progress
-      // bar neeeds to be indeterminate.
-      progress_known = false;
-    }
-  }
-
-  float progress = 0;
-  if (progress_known && download_count)
-    progress = static_cast<float>(received_bytes) / total_bytes;
-
-  download_util::UpdateAppIconDownloadProgress(download_count,
-                                               progress_known,
-                                               progress);
+  if (status_updater_)
+    status_updater_->Update();
 }
 
 void DownloadManager::RenameDownload(DownloadItem* download,
@@ -836,6 +817,40 @@ bool DownloadManager::ShouldOpenFileBasedOnExtension(
   return download_prefs_->IsAutoOpenEnabledForExtension(extension);
 }
 
+bool DownloadManager::IsDownloadProgressKnown() {
+  for (DownloadMap::iterator i = in_progress_.begin();
+       i != in_progress_.end(); ++i) {
+    if (i->second->total_bytes() <= 0)
+      return false;
+  }
+
+  return true;
+}
+
+int64 DownloadManager::GetInProgressDownloadCount() {
+  return in_progress_.size();
+}
+
+int64 DownloadManager::GetReceivedDownloadBytes() {
+  DCHECK(IsDownloadProgressKnown());
+  int64 received_bytes = 0;
+  for (DownloadMap::iterator i = in_progress_.begin();
+       i != in_progress_.end(); ++i) {
+    received_bytes += i->second->received_bytes();
+  }
+  return received_bytes;
+}
+
+int64 DownloadManager::GetTotalDownloadBytes() {
+  DCHECK(IsDownloadProgressKnown());
+  int64 total_bytes = 0;
+  for (DownloadMap::iterator i = in_progress_.begin();
+       i != in_progress_.end(); ++i) {
+    total_bytes += i->second->total_bytes();
+  }
+  return total_bytes;
+}
+
 void DownloadManager::FileSelected(const FilePath& path,
                                    int index, void* params) {
   DownloadCreateInfo* info = reinterpret_cast<DownloadCreateInfo*>(params);
diff --git a/chrome/browser/download/download_manager.h b/chrome/browser/download/download_manager.h
index 4bbf49a..02c7aa1 100644
--- a/chrome/browser/download/download_manager.h
+++ b/chrome/browser/download/download_manager.h
@@ -40,12 +40,14 @@
 #include "base/scoped_ptr.h"
 #include "base/time.h"
 #include "chrome/browser/chrome_thread.h"
+#include "chrome/browser/download/download_status_updater_delegate.h"
 #include "chrome/browser/shell_dialogs.h"
 
 class DownloadFileManager;
 class DownloadHistory;
 class DownloadItem;
 class DownloadPrefs;
+class DownloadStatusUpdater;
 class GURL;
 class Profile;
 class ResourceDispatcherHost;
@@ -58,13 +60,14 @@ struct DownloadSaveInfo;
 class DownloadManager
     : public base::RefCountedThreadSafe<DownloadManager,
                                         ChromeThread::DeleteOnUIThread>,
+      public DownloadStatusUpdaterDelegate,
       public SelectFileDialog::Listener {
   // For testing.
   friend class DownloadManagerTest;
   friend class MockDownloadManager;
 
  public:
-  DownloadManager();
+  explicit DownloadManager(DownloadStatusUpdater* status_updater);
 
   // Shutdown the download manager. Must be called before destruction.
   void Shutdown();
@@ -186,6 +189,12 @@ class DownloadManager
   // Tests if a file type should be opened automatically.
   bool ShouldOpenFileBasedOnExtension(const FilePath& path) const;
 
+  // Overridden from DownloadStatusUpdaterDelegate:
+  virtual bool IsDownloadProgressKnown();
+  virtual int64 GetInProgressDownloadCount();
+  virtual int64 GetReceivedDownloadBytes();
+  virtual int64 GetTotalDownloadBytes();
+
   // Overridden from SelectFileDialog::Listener:
   virtual void FileSelected(const FilePath& path, int index, void* params);
   virtual void FileSelectionCanceled(void* params);
@@ -246,8 +255,7 @@ class DownloadManager
   // dangerous downloads are downloaded to temporary files that need to be
   // renamed on the file thread first.
   // Invoked on the UI thread.
-  // Marked virtual for testing.
-  virtual void ContinueDownloadFinished(DownloadItem* download);
+  void ContinueDownloadFinished(DownloadItem* download);
 
   // Renames a finished dangerous download from its temporary file name to its
   // real file name.
@@ -263,8 +271,7 @@ class DownloadManager
                                 int new_path_uniquifier);
 
   // Updates the app icon about the overall download progress.
-  // Marked virtual for testing.
-  virtual void UpdateAppIcon();
+  void UpdateAppIcon();
 
   // Changes the paths and file name of the specified |download|, propagating
   // the change to the history system.
@@ -332,6 +339,9 @@ class DownloadManager
   // Non-owning pointer for handling file writing on the download_thread_.
   DownloadFileManager* file_manager_;
 
+  // Non-owning pointer for updating the download status.
+  DownloadStatusUpdater* status_updater_;
+
   // The user's last choice for download directory. This is only used when the
   // user wants us to prompt for a save location for each download.
   FilePath last_download_path_;
diff --git a/chrome/browser/download/download_manager_unittest.cc b/chrome/browser/download/download_manager_unittest.cc
index ded2494..ca71804 100644
--- a/chrome/browser/download/download_manager_unittest.cc
+++ b/chrome/browser/download/download_manager_unittest.cc
@@ -22,8 +22,10 @@
 
 class MockDownloadManager : public DownloadManager {
  public:
+  MockDownloadManager() : DownloadManager(NULL) {
+  }
+
   // Override some functions.
-  virtual void UpdateAppIcon() { }
   virtual void UpdateHistoryForDownload(DownloadItem*) { }
   virtual void ContinueDownloadFinished(DownloadItem*) { }
 };
diff --git a/chrome/browser/download/download_status_updater.cc b/chrome/browser/download/download_status_updater.cc
new file mode 100644
index 0000000..6d6b565
--- /dev/null
+++ b/chrome/browser/download/download_status_updater.cc
@@ -0,0 +1,66 @@
+// Copyright (c) 2010 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.
+
+#include "chrome/browser/download/download_status_updater.h"
+
+#include "base/logging.h"
+#include "chrome/browser/download/download_status_updater_delegate.h"
+#include "chrome/browser/download/download_util.h"
+
+DownloadStatusUpdater::DownloadStatusUpdater() {
+}
+
+DownloadStatusUpdater::~DownloadStatusUpdater() {
+  DCHECK(delegates_.empty());
+}
+
+void DownloadStatusUpdater::AddDelegate(
+    DownloadStatusUpdaterDelegate* delegate) {
+  delegates_.insert(delegate);
+}
+
+void DownloadStatusUpdater::RemoveDelegate(
+    DownloadStatusUpdaterDelegate* delegate) {
+  delegates_.erase(delegate);
+}
+
+void DownloadStatusUpdater::Update() {
+  float progress = 0;
+  bool progress_known = GetProgress(&progress);
+  download_util::UpdateAppIconDownloadProgress(
+      static_cast<int>(GetInProgressDownloadCount()),
+      progress_known,
+      progress);
+}
+
+bool DownloadStatusUpdater::GetProgress(float* progress) {
+  *progress = 0;
+
+  bool progress_known = true;
+  int64 received_bytes = 0;
+  int64 total_bytes = 0;
+  for (DelegateSet::iterator i = delegates_.begin();
+       i != delegates_.end(); ++i) {
+    progress_known = progress_known && (*i)->IsDownloadProgressKnown();
+    received_bytes += (*i)->GetReceivedDownloadBytes();
+    total_bytes += (*i)->GetTotalDownloadBytes();
+  }
+
+  if (!progress_known)
+    return false;
+
+  if (total_bytes > 0)
+    *progress = static_cast<float>(received_bytes) / total_bytes;
+  return true;
+}
+
+int64 DownloadStatusUpdater::GetInProgressDownloadCount() {
+  int64 download_count = 0;
+  for (DelegateSet::iterator i = delegates_.begin();
+       i != delegates_.end(); ++i) {
+    download_count += (*i)->GetInProgressDownloadCount();
+  }
+
+  return download_count;
+}
diff --git a/chrome/browser/download/download_status_updater.h b/chrome/browser/download/download_status_updater.h
new file mode 100644
index 0000000..cdfd5b8
--- /dev/null
+++ b/chrome/browser/download/download_status_updater.h
@@ -0,0 +1,45 @@
+// Copyright (c) 2010 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.
+
+#ifndef CHROME_BROWSER_DOWNLOAD_DOWNLOAD_STATUS_UPDATER_H_
+#define CHROME_BROWSER_DOWNLOAD_DOWNLOAD_STATUS_UPDATER_H_
+
+#include <set>
+
+#include "base/basictypes.h"
+#include "base/gtest_prod_util.h"
+
+class DownloadStatusUpdaterDelegate;
+
+// Keeps track of download progress for the entire browser.
+class DownloadStatusUpdater {
+ public:
+  DownloadStatusUpdater();
+  ~DownloadStatusUpdater();
+
+  void AddDelegate(DownloadStatusUpdaterDelegate* delegate);
+  void RemoveDelegate(DownloadStatusUpdaterDelegate* delegate);
+
+  // Updates the download status based on data from delegates.
+  void Update();
+
+ private:
+  FRIEND_TEST_ALL_PREFIXES(DownloadStatusUpdaterTest, Basic);
+  FRIEND_TEST_ALL_PREFIXES(DownloadStatusUpdaterTest, OneDelegate);
+  FRIEND_TEST_ALL_PREFIXES(DownloadStatusUpdaterTest, MultipleDelegates);
+
+  // If the progress is known (i.e. we know the final size of all downloads),
+  // returns true and puts a percentage (in range [0-1]) in |progress|.
+  bool GetProgress(float* progress);
+
+  // Returns the number of downloads that are in progress.
+  int64 GetInProgressDownloadCount();
+
+  typedef std::set<DownloadStatusUpdaterDelegate*> DelegateSet;
+  DelegateSet delegates_;
+
+  DISALLOW_COPY_AND_ASSIGN(DownloadStatusUpdater);
+};
+
+#endif  // CHROME_BROWSER_DOWNLOAD_DOWNLOAD_STATUS_UPDATER_H_
diff --git a/chrome/browser/download/download_status_updater_delegate.h b/chrome/browser/download/download_status_updater_delegate.h
new file mode 100644
index 0000000..17503b2
--- /dev/null
+++ b/chrome/browser/download/download_status_updater_delegate.h
@@ -0,0 +1,29 @@
+// Copyright (c) 2010 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.
+
+#ifndef CHROME_BROWSER_DOWNLOAD_DOWNLOAD_STATUS_UPDATER_DELEGATE_H_
+#define CHROME_BROWSER_DOWNLOAD_DOWNLOAD_STATUS_UPDATER_DELEGATE_H_
+
+#include "base/basictypes.h"
+
+class DownloadStatusUpdaterDelegate {
+ public:
+  // Returns true if the progress is known (i.e. we know the final size
+  // of all downloads).
+  virtual bool IsDownloadProgressKnown() = 0;
+
+  // Returns the number of downloads that are in progress.
+  virtual int64 GetInProgressDownloadCount() = 0;
+
+  // Returns the amount of received data, in bytes.
+  virtual int64 GetReceivedDownloadBytes() = 0;
+
+  // Returns the final size of all downloads, in bytes.
+  virtual int64 GetTotalDownloadBytes() = 0;
+
+ protected:
+  virtual ~DownloadStatusUpdaterDelegate() {}
+};
+
+#endif  // CHROME_BROWSER_DOWNLOAD_DOWNLOAD_STATUS_UPDATER_DELEGATE_H_
diff --git a/chrome/browser/download/download_status_updater_unittest.cc b/chrome/browser/download/download_status_updater_unittest.cc
new file mode 100644
index 0000000..c74c0bf
--- /dev/null
+++ b/chrome/browser/download/download_status_updater_unittest.cc
@@ -0,0 +1,159 @@
+// Copyright (c) 2010 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.
+
+#include "base/scoped_ptr.h"
+#include "chrome/browser/download/download_status_updater.h"
+#include "chrome/browser/download/download_status_updater_delegate.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+class MockDelegate : public DownloadStatusUpdaterDelegate {
+ public:
+  explicit MockDelegate(DownloadStatusUpdater* updater)
+      : updater_(updater),
+        is_download_progress_known_(true),
+        in_progress_download_count_(0),
+        received_bytes_(0),
+        total_bytes_(0) {
+    updater_->AddDelegate(this);
+  }
+
+  ~MockDelegate() {
+    updater_->RemoveDelegate(this);
+  }
+
+  // Overriden from DownloadStatusUpdaterDelegate:
+  virtual bool IsDownloadProgressKnown() {
+    return is_download_progress_known_;
+  }
+
+  virtual int64 GetInProgressDownloadCount() {
+    return in_progress_download_count_;
+  }
+
+  virtual int64 GetReceivedDownloadBytes() {
+    return received_bytes_;
+  }
+
+  virtual int64 GetTotalDownloadBytes() {
+    return total_bytes_;
+  }
+
+  void set_is_download_progress_known(bool progress_known) {
+    is_download_progress_known_ = progress_known;
+  }
+
+  void set_in_progress_download_count(int64 download_count) {
+    in_progress_download_count_ = download_count;
+  }
+
+  void set_received_bytes(int64 received_bytes) {
+    received_bytes_ = received_bytes;
+  }
+
+  void set_total_bytes(int64 total_bytes) {
+    total_bytes_ = total_bytes;
+  }
+
+ private:
+  DownloadStatusUpdater* updater_;
+
+  bool is_download_progress_known_;
+  int64 in_progress_download_count_;
+  int64 received_bytes_;
+  int64 total_bytes_;
+
+  DISALLOW_COPY_AND_ASSIGN(MockDelegate);
+};
+
+}  // namespace
+
+class DownloadStatusUpdaterTest : public testing::Test {
+ protected:
+  DownloadStatusUpdater updater_;
+};
+
+TEST_F(DownloadStatusUpdaterTest, Basic) {
+  float progress = -1;
+  EXPECT_TRUE(updater_.GetProgress(&progress));
+  EXPECT_FLOAT_EQ(0, progress);
+  EXPECT_EQ(0, updater_.GetInProgressDownloadCount());
+}
+
+TEST_F(DownloadStatusUpdaterTest, OneDelegate) {
+  MockDelegate delegate(&updater_);
+
+  {
+    float progress = -1;
+    EXPECT_TRUE(updater_.GetProgress(&progress));
+    EXPECT_FLOAT_EQ(0, progress);
+    EXPECT_EQ(0, updater_.GetInProgressDownloadCount());
+  }
+
+  delegate.set_in_progress_download_count(1);
+  delegate.set_received_bytes(21);
+  delegate.set_total_bytes(42);
+
+  {
+    float progress = -1;
+    EXPECT_TRUE(updater_.GetProgress(&progress));
+    EXPECT_FLOAT_EQ(static_cast<float>(1) / 2, progress);
+    EXPECT_EQ(1, updater_.GetInProgressDownloadCount());
+  }
+
+  delegate.set_is_download_progress_known(false);
+
+  {
+    float progress = -1;
+    EXPECT_FALSE(updater_.GetProgress(&progress));
+    EXPECT_FLOAT_EQ(0, progress);
+    EXPECT_EQ(1, updater_.GetInProgressDownloadCount());
+  }
+}
+
+TEST_F(DownloadStatusUpdaterTest, MultipleDelegates) {
+  scoped_ptr<MockDelegate> delegate1(new MockDelegate(&updater_));
+  scoped_ptr<MockDelegate> delegate2(new MockDelegate(&updater_));
+
+  {
+    float progress = -1;
+    EXPECT_TRUE(updater_.GetProgress(&progress));
+    EXPECT_FLOAT_EQ(0, progress);
+    EXPECT_EQ(0, updater_.GetInProgressDownloadCount());
+  }
+
+  delegate1->set_in_progress_download_count(1);
+  delegate1->set_received_bytes(14);
+  delegate1->set_total_bytes(21);
+
+  delegate2->set_in_progress_download_count(2);
+  delegate2->set_received_bytes(7);
+  delegate2->set_total_bytes(21);
+
+  {
+    float progress = -1;
+    EXPECT_TRUE(updater_.GetProgress(&progress));
+    EXPECT_FLOAT_EQ(static_cast<float>(1) / 2, progress);
+    EXPECT_EQ(3, updater_.GetInProgressDownloadCount());
+  }
+
+  delegate1->set_is_download_progress_known(false);
+
+  {
+    float progress = -1;
+    EXPECT_FALSE(updater_.GetProgress(&progress));
+    EXPECT_FLOAT_EQ(0, progress);
+    EXPECT_EQ(3, updater_.GetInProgressDownloadCount());
+  }
+
+  delegate1.reset();
+
+  {
+    float progress = -1;
+    EXPECT_TRUE(updater_.GetProgress(&progress));
+    EXPECT_FLOAT_EQ(static_cast<float>(1) / 3, progress);
+    EXPECT_EQ(2, updater_.GetInProgressDownloadCount());
+  }
+}
diff --git a/chrome/browser/profile.cc b/chrome/browser/profile.cc
index e222bd0d..36fabbd 100644
--- a/chrome/browser/profile.cc
+++ b/chrome/browser/profile.cc
@@ -304,7 +304,8 @@ class OffTheRecordProfileImpl : public Profile,
 
   virtual DownloadManager* GetDownloadManager() {
     if (!download_manager_.get()) {
-      scoped_refptr<DownloadManager> dlm(new DownloadManager);
+      scoped_refptr<DownloadManager> dlm(
+          new DownloadManager(g_browser_process->download_status_updater()));
       dlm->Init(this);
       download_manager_.swap(dlm);
     }
diff --git a/chrome/browser/profile_impl.cc b/chrome/browser/profile_impl.cc
index 22c456a..5a61e1c 100644
--- a/chrome/browser/profile_impl.cc
+++ b/chrome/browser/profile_impl.cc
@@ -1025,7 +1025,8 @@ void ProfileImpl::CreatePasswordStore() {
 
 DownloadManager* ProfileImpl::GetDownloadManager() {
   if (!created_download_manager_) {
-    scoped_refptr<DownloadManager> dlm(new DownloadManager);
+    scoped_refptr<DownloadManager> dlm(
+        new DownloadManager(g_browser_process->download_status_updater()));
     dlm->Init(this);
     created_download_manager_ = true;
     download_manager_.swap(dlm);