drive: Fix computation for showing the number of fetched files

chrome/browser/chromeos/drive/gdata_wapi_feed_loader.cc

 // Copyright (c) 2012 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/chromeos/drive/gdata_wapi_feed_loader.h"
 
 #include <set>
 
 #include "base/command_line.h"
 #include "base/file_util.h"
@@ skipping 187 matching lines @@
         start_time(start_time),
         weak_ptr_factory(this) {
   }
 
   // The number of fetched documents.
   int num_fetched_documents;
 
   // 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;
 };
 
 GDataWapiFeedLoader::GDataWapiFeedLoader(
     DriveResourceMetadata* resource_metadata,
     DriveServiceInterface* drive_service,
     DriveWebAppsRegistryInterface* webapps_registry,
     DriveCache* cache,
     scoped_refptr<base::SequencedTaskRunner> blocking_task_runner)
@@ skipping 354 matching lines @@
   // Compute and notify the number of entries fetched so far.
   int num_accumulated_entries = 0;
   for (size_t i = 0; i < params->feed_list.size(); ++i)
     num_accumulated_entries += params->feed_list[i]->entries().size();
 
   // Check if we need to collect more data to complete the directory list.
   if (has_next_feed_url && !next_feed_url.is_empty()) {
     // 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);
 
     if ((ui_state->num_fetched_documents - ui_state->num_showing_documents)
         < kFetchUiUpdateStep) {
       // Currently the UI update is stopped. Start UI periodic callback.
       base::MessageLoopProxy::current()->PostTask(
           FROM_HERE,
           base::Bind(&GDataWapiFeedLoader::OnNotifyDocumentFeedFetched,
                      weak_ptr_factory_.GetWeakPtr(),
                      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.
     LoadFeedParams* params_ptr = params.get();
     // Kick off the remaining part of the feeds.
     drive_service_->GetDocuments(
         next_feed_url,
         params_ptr->start_changestamp,
         params_ptr->search_query,
         params_ptr->directory_resource_id,
@@ skipping 70 matching lines @@
   // Compute and notify the number of entries fetched so far.
   int num_accumulated_entries = 0;
   for (size_t i = 0; i < params->feed_list.size(); ++i)
     num_accumulated_entries += params->feed_list[i]->entries().size();
 
   // Check if we need to collect more data to complete the directory list.
   if (has_next_feed) {
     // 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);
 
     if ((ui_state->num_fetched_documents - ui_state->num_showing_documents)
         < kFetchUiUpdateStep) {
       // Currently the UI update is stopped. Start UI periodic callback.
       base::MessageLoopProxy::current()->PostTask(
           FROM_HERE,
           base::Bind(&GDataWapiFeedLoader::OnNotifyDocumentFeedFetched,
                      weak_ptr_factory_.GetWeakPtr(),
                      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.
     LoadFeedParams* params_ptr = params.get();
     drive_service_->GetDocuments(
         current_feed->next_link(),
         params_ptr->start_changestamp,
         std::string(),  // No search query.
         std::string(),  // No directory resource ID.
         base::Bind(&GDataWapiFeedLoader::OnGetChangelist,
@@ skipping 17 matching lines @@
 void GDataWapiFeedLoader::OnNotifyDocumentFeedFetched(
     base::WeakPtr<GetDocumentsUiState> ui_state) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   if (!ui_state) {
     // The ui state instance is already released, which means the fetching
     // is done and we don't need to update any more.
     return;
   }
 
-  base::TimeDelta elapsed_time =
-      base::TimeTicks::Now() - ui_state->start_time;
+  // 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));
 
-  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);
-    }
+  // 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);
   }
 }
 
 void GDataWapiFeedLoader::LoadFromCache(
     bool should_load_from_server,
     const FileOperationCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   LoadRootFeedParams* params = new LoadRootFeedParams(should_load_from_server,
                                                       callback);
@@ skipping 132 matching lines @@
         dir_iter != changed_dirs.end(); ++dir_iter) {
       FOR_EACH_OBSERVER(GDataWapiFeedLoaderObserver, observers_,
                         OnDirectoryChanged(*dir_iter));
     }
   }
 
   return error;
 }
 
 }  // namespace drive