Mountain Lion: use the system download progress.

chrome/browser/download/download_status_updater_unittest.cc

Index: chrome/browser/download/download_status_updater_unittest.cc
diff --git a/chrome/browser/download/download_status_updater_unittest.cc b/chrome/browser/download/download_status_updater_unittest.cc
index 6cc7ec03cbf801bbe5a9008956d5b8f1eb3f7176..9683669927af6c6860103710ecc40eee84210a20 100644
--- a/chrome/browser/download/download_status_updater_unittest.cc
+++ b/chrome/browser/download/download_status_updater_unittest.cc
@@ -22,10 +22,34 @@ using ::testing::StrictMock;
 using ::testing::_;
 
 class TestDownloadStatusUpdater : public DownloadStatusUpdater {
+ public:
+  TestDownloadStatusUpdater() : started_count_(0),
+                                progressed_count_(0),
+                                completed_count_(0) {
+  }
+  size_t GetStartedCount() const { return started_count_; }
+  size_t GetProgressedCount() const { return progressed_count_; }
+  size_t GetCompletedCount() const { return completed_count_; }
  protected:
   virtual void UpdateAppIconDownloadProgress() OVERRIDE {
     return;
   }
+  virtual void UpdateDownloadProgressForItemStarted(
+      content::DownloadItem* download) OVERRIDE {
+    ++started_count_;
+  }
+  virtual void UpdateDownloadProgressForItemProgressed(
+      content::DownloadItem* download) OVERRIDE {
+    ++progressed_count_;
+  }
+  virtual void UpdateDownloadProgressForItemCompleted(
+      content::DownloadItem* download) OVERRIDE {
+    ++completed_count_;
+  }
+ private:
+  size_t started_count_;
+  size_t progressed_count_;
+  size_t completed_count_;
 };
 
 class DownloadStatusUpdaterTest : public testing::Test {
@@ -120,13 +144,16 @@ class DownloadStatusUpdaterTest : public testing::Test {
   }
 
   // Set return values relevant to |DownloadStatusUpdater::GetProgress()|
-  // for the specified item
+  // for the specified item.
   void SetItemValues(int manager_index, int item_index,
-                     int received_bytes, int total_bytes) {
-    EXPECT_CALL(*Item(manager_index, item_index), GetReceivedBytes())
+                     int received_bytes, int total_bytes, bool notify) {
+    content::MockDownloadItem* item(Item(manager_index, item_index));
+    EXPECT_CALL(*item, GetReceivedBytes())
         .WillRepeatedly(Return(received_bytes));
-    EXPECT_CALL(*Item(manager_index, item_index), GetTotalBytes())
+    EXPECT_CALL(*item, GetTotalBytes())
         .WillRepeatedly(Return(total_bytes));
+    if (notify)
+      updater_->OnDownloadUpdated(item);
   }
 
   // Transition specified item to completed.
@@ -157,7 +184,7 @@ class DownloadStatusUpdaterTest : public testing::Test {
 
   // Pointer so we can verify that destruction triggers appropriate
   // changes.
-  DownloadStatusUpdater *updater_;
+  TestDownloadStatusUpdater *updater_;
 
   // Thread so that the DownloadManager (which is a DeleteOnUIThread
   // object) can be deleted.
@@ -198,9 +225,9 @@ TEST_F(DownloadStatusUpdaterTest, OneManagerManyItems) {
   LinkManager(0);
 
   // Prime items
-  SetItemValues(0, 0, 10, 20);
-  SetItemValues(0, 1, 50, 60);
-  SetItemValues(0, 2, 90, 90);
+  SetItemValues(0, 0, 10, 20, false);
+  SetItemValues(0, 1, 50, 60, false);
+  SetItemValues(0, 2, 90, 90, false);
 
   float progress = -1;
   int download_count = -1;
@@ -218,13 +245,42 @@ TEST_F(DownloadStatusUpdaterTest, OneManagerManyItems) {
   // Add a new item to manager and confirm progress is updated properly.
   AddItems(0, 1, 1);
   updater_->ModelChanged(Manager(0));
-  SetItemValues(0, 3, 150, 200);
+  SetItemValues(0, 3, 150, 200, false);
 
   EXPECT_TRUE(updater_->GetProgress(&progress, &download_count));
   EXPECT_FLOAT_EQ((50+150)/(60+200.0f), progress);
   EXPECT_EQ(2, download_count);
 }
 
+// Test that the progress for items callbacks are called in a reasonable way.
+TEST_F(DownloadStatusUpdaterTest, ProgressForItem) {
+  size_t orig_started = updater_->GetStartedCount();
+  size_t orig_progressed = updater_->GetProgressedCount();
+  size_t orig_completed = updater_->GetCompletedCount();
+  SetupManagers(1);
+  AddItems(0, 1, 1);
+  LinkManager(0);
+
+  EXPECT_GE(updater_->GetStartedCount(), orig_started);

[Randy Smith (Not in Mondays), 2012/08/09 20:12:25]

Why are these GE rather than GT (throughout this function)?  It seems like
you're really only testing "We *don't* update progressed/completed on add, We
*don't* set completed on progress" rather than testing that you actually do get
progress info.

+  EXPECT_EQ(updater_->GetProgressedCount(), orig_progressed);
+  EXPECT_EQ(updater_->GetCompletedCount(), orig_completed);
+
+  // Make progress.
+  SetItemValues(0, 0, 10, 20, true);
+
+  EXPECT_GE(updater_->GetStartedCount(), orig_started);
+  EXPECT_GE(updater_->GetProgressedCount(), orig_progressed);
+  EXPECT_EQ(updater_->GetCompletedCount(), orig_completed);
+
+  // Transition one item to completed and confirm progress is updated
+  // properly.
+  CompleteItem(0, 0);
+
+  EXPECT_GE(updater_->GetStartedCount(), orig_started);
+  EXPECT_GE(updater_->GetProgressedCount(), orig_progressed);
+  EXPECT_GE(updater_->GetCompletedCount(), orig_completed);
+}
+
 // Confirm we recognize the situation where we have an unknown size.
 TEST_F(DownloadStatusUpdaterTest, UnknownSize) {
   SetupManagers(1);
@@ -232,8 +288,8 @@ TEST_F(DownloadStatusUpdaterTest, UnknownSize) {
   LinkManager(0);
 
   // Prime items
-  SetItemValues(0, 0, 10, 20);
-  SetItemValues(0, 1, 50, -1);
+  SetItemValues(0, 0, 10, 20, false);
+  SetItemValues(0, 1, 50, -1, false);
 
   float progress = -1;
   int download_count = -1;
@@ -276,9 +332,9 @@ TEST_F(DownloadStatusUpdaterTest, ManyManagersMixedItems) {
   AddItems(1, 3, 1);
   LinkManager(1);
 
-  SetItemValues(0, 0, 10, 20);
-  SetItemValues(0, 1, 50, 60);
-  SetItemValues(1, 0, 80, 90);
+  SetItemValues(0, 0, 10, 20, false);
+  SetItemValues(0, 1, 50, 60, false);
+  SetItemValues(1, 0, 80, 90, false);
 
   float progress = -1;
   int download_count = -1;