drive: Fix computation for showing the number of fetched files

chrome/browser/chromeos/drive/gdata_wapi_feed_loader.cc

Index: chrome/browser/chromeos/drive/gdata_wapi_feed_loader.cc
diff --git a/chrome/browser/chromeos/drive/gdata_wapi_feed_loader.cc b/chrome/browser/chromeos/drive/gdata_wapi_feed_loader.cc
index 2e0d2d46fb9162c6b7b3fba563845aaa56f9bd81..5138f3f398d4162b9657c0d2f0d39e7c7469ce8d 100644
--- a/chrome/browser/chromeos/drive/gdata_wapi_feed_loader.cc
+++ b/chrome/browser/chromeos/drive/gdata_wapi_feed_loader.cc
@@ -205,11 +205,11 @@ struct GetDocumentsUiState {
   // The number documents shown on UI.
   int num_showing_documents;
 
-  // When the UI update has started.
+  // When the loading started.
   base::TimeTicks start_time;
 
-  // Time elapsed since the feed fetching was started.
-  base::TimeDelta feed_fetching_elapsed_time;
+  // When the last feed was fetched.
+  base::TimeTicks last_fetch_time;
 
   base::WeakPtrFactory<GetDocumentsUiState> weak_ptr_factory;
 };
@@ -584,7 +584,7 @@ void GDataWapiFeedLoader::OnParseFeed(
     // Post an UI update event to make the UI smoother.
     GetDocumentsUiState* ui_state = params->ui_state.get();
     if (ui_state == NULL) {
-      ui_state = new GetDocumentsUiState(base::TimeTicks::Now());
+      ui_state = new GetDocumentsUiState(start_time);
       params->ui_state.reset(ui_state);
     }
     DCHECK(ui_state);
@@ -599,7 +599,7 @@ void GDataWapiFeedLoader::OnParseFeed(
                      ui_state->weak_ptr_factory.GetWeakPtr()));
     }
     ui_state->num_fetched_documents = num_accumulated_entries;
-    ui_state->feed_fetching_elapsed_time = base::TimeTicks::Now() - start_time;
+    ui_state->last_fetch_time = base::TimeTicks::Now();
 
     // |params| will be passed to the callback and thus nulled. Extract the
     // pointer so we can use it bellow.
@@ -690,7 +690,7 @@ void GDataWapiFeedLoader::OnGetChangelist(
     // Post an UI update event to make the UI smoother.
     GetDocumentsUiState* ui_state = params->ui_state.get();
     if (ui_state == NULL) {
-      ui_state = new GetDocumentsUiState(base::TimeTicks::Now());
+      ui_state = new GetDocumentsUiState(start_time);
       params->ui_state.reset(ui_state);
     }
     DCHECK(ui_state);
@@ -705,7 +705,7 @@ void GDataWapiFeedLoader::OnGetChangelist(
                      ui_state->weak_ptr_factory.GetWeakPtr()));
     }
     ui_state->num_fetched_documents = num_accumulated_entries;
-    ui_state->feed_fetching_elapsed_time = base::TimeTicks::Now() - start_time;
+    ui_state->last_fetch_time = base::TimeTicks::Now();
 
     // Kick off the remaining part of the feeds.
     // Extract the pointer so we can use it bellow.
@@ -743,30 +743,31 @@ void GDataWapiFeedLoader::OnNotifyDocumentFeedFetched(
     return;
   }
 
-  base::TimeDelta elapsed_time =
-      base::TimeTicks::Now() - ui_state->start_time;
-
-  if (ui_state->num_showing_documents + kFetchUiUpdateStep <=
-      ui_state->num_fetched_documents) {
-    ui_state->num_showing_documents += kFetchUiUpdateStep;
-    FOR_EACH_OBSERVER(GDataWapiFeedLoaderObserver, observers_,
-                      OnDocumentFeedFetched(ui_state->num_showing_documents));
-
-    int num_remaining_ui_updates =
-        (ui_state->num_fetched_documents - ui_state->num_showing_documents)
-        / kFetchUiUpdateStep;
-    if (num_remaining_ui_updates > 0) {
-      // Heuristically, we use fetched time duration to calculate the next
-      // UI update timing.
-      base::TimeDelta remaining_duration =
-          ui_state->feed_fetching_elapsed_time - elapsed_time;
-      base::MessageLoopProxy::current()->PostDelayedTask(
-          FROM_HERE,
-          base::Bind(&GDataWapiFeedLoader::OnNotifyDocumentFeedFetched,
-                     weak_ptr_factory_.GetWeakPtr(),
-                     ui_state->weak_ptr_factory.GetWeakPtr()),
-          remaining_duration / num_remaining_ui_updates);
-    }
+  // Compute the number of documents to be shown in UI.
+  ui_state->num_showing_documents = std::min(
+      ui_state->num_showing_documents + kFetchUiUpdateStep,
+      ui_state->num_fetched_documents);
+  // Update UI via observers.
+  FOR_EACH_OBSERVER(GDataWapiFeedLoaderObserver, observers_,
+                    OnDocumentFeedFetched(ui_state->num_showing_documents));
+
+  // UI should be updated if num_showing_documents is behind
+  // num_fetched_documents.
+  if (ui_state->num_showing_documents < ui_state->num_fetched_documents) {
+    // Time elapsed from the beginning to the last fetch;
+    const base::TimeDelta elapsed_time =
+        ui_state->last_fetch_time - ui_state ->start_time;

[hidehiko, 2012/10/17 06:26:41]

s/ui_state ->/ui_state->/

[satorux1, 2012/10/17 08:00:43]

Done.

+    // How long did it take to fetch one document?
+    const base::TimeDelta rate =
+        elapsed_time / ui_state->num_fetched_documents;
+    // How long would it take to fetch the next kFetchUiUpdateStep documents?
+    const base::TimeDelta interval = rate * kFetchUiUpdateStep;

[hidehiko, 2012/10/17 06:26:41]

By this expression's change, UI update duration may be longer than fetched
duration. Is it expected?

[satorux1, 2012/10/17 08:00:43]

Chatted off-line. Here's the summary of your concern:

1) if one fetch takes unusually long for some reason, the |interval| here
becomes big, as it's computed by the average fetch time.
3) but the subsequent fetches may be back to the normal speed
4) Then, |interval| computed may be irrelevantly long

I think that's a valid concern. Let me take another look...

+    base::MessageLoopProxy::current()->PostDelayedTask(
+        FROM_HERE,
+        base::Bind(&GDataWapiFeedLoader::OnNotifyDocumentFeedFetched,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   ui_state->weak_ptr_factory.GetWeakPtr()),
+        interval);
   }
 }