Update the number of fetched files more frequently.

chrome/browser/chromeos/gdata/gdata_file_system.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/gdata/gdata_file_system.h"
 
 #include <set>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 37 matching lines @@
 
 const char kEmptyFilePath[] = "/dev/null";
 
 // GData update check interval (in seconds).
 #ifndef NDEBUG
 const int kGDataUpdateCheckIntervalInSec = 5;
 #else
 const int kGDataUpdateCheckIntervalInSec = 60;
 #endif
 
+const int kNumUiUpdateStepsPerFetch = 50;

[satorux1, 2012/08/02 08:03:14]

With kMaxDocumentsPerFeed = 500 defined elsewhere, and 50 here, we update the
fetch progress UI per every 10 feeds. If we change kMaxDocumentsPerFeed to 700,
we'll end up counting per every 14 feeds, which would look bad.

That said, Wouldn't it be better to define something like

// Update the fetch progress UI per every 10 feeds, though it's giving illusion
to users. The feeds are actually processed per kMaxDocumentsPerFeed 
const int kFetchUiUpdateStep = 10;

[hidehiko, 2012/08/02 13:47:05]

Done.

+
 // Schedule for dumping root file system proto buffers to disk depending its
 // total protobuffer size in MB.
 typedef struct {
   double size;
   int timeout;
 } SerializationTimetable;
 
 SerializationTimetable kSerializeTimetable[] = {
 #ifndef NDEBUG
     {0.5, 0},    // Less than 0.5MB, dump immediately.
@@ skipping 535 matching lines @@
     const GetEntryInfoWithFilePathCallback& callback,
     const FilePath& path,
     GDataFileError error,
     scoped_ptr<GDataEntryProto> entry_proto) {
   if (!callback.is_null())
     callback.Run(error, path, entry_proto.Pass());
 }
 
 }  // namespace
 
+// GDataFileSystem::GetDocumentsUIState struct implementation.

[satorux1, 2012/08/02 08:03:14]

UI -> Ui

[hidehiko, 2012/08/02 13:47:05]

Done.

+// This is a trick to update the number of fetched documents frequently on
+// UI. Due to performance reason, we need to fetch a number of files at
+// a time. However, it'll take long time, and a user has no way to know
+// the current update state. In order to make users confortable,
+// we increment the number of fetched documents with more frequent but smaller
+// steps than actual fetching.
+struct GDataFileSystem::GetDocumentsUIState {
+  GetDocumentsUIState(
+      base::TimeTicks start_time,
+      const base::Callback<void(GetDocumentsUIState*)>& callback)
+      : num_fetched_documents(0),
+        num_showing_documents(0),
+        num_remaining_ui_update(0),
+        start_time(start_time),
+        callback(callback),
+        ui_weak_ptr_factory(this),
+        ui_weak_ptr(ui_weak_ptr_factory.GetWeakPtr()) {
+  }
+
+  // Callback trampoline. By introducing this callback, the main procedure
+  // can cancel the pending events by just deleteing this instance.
+  void Run() {

[satorux1, 2012/08/02 08:03:14]

I'm guessing that this method is unnecessary. see below.

[hidehiko, 2012/08/02 13:47:05]

Done.

+    callback.Run(this);
+  }
+
+  // The number of fetched documents.
+  int num_fetched_documents;
+
+  // The number documents shown on UI.
+  int num_showing_documents;
+
+  // How many times the UI will be updated.
+  int num_remaining_ui_update;
+
+  // When the UI update has started.
+  base::TimeTicks start_time;
+
+  // How long duration the actual fetching takes in total.
+  // Heuristically, we use the duration to decide the UI update step period.
+  base::TimeDelta fetched_duration;
+
+  base::Callback<void(GetDocumentsUIState*)> callback;
+  base::WeakPtrFactory<GetDocumentsUIState> ui_weak_ptr_factory;
+  base::WeakPtr<GetDocumentsUIState> ui_weak_ptr;
+};
+
+
 // GDataFileSystem::GetDocumentsParams struct implementation.
 struct GDataFileSystem::GetDocumentsParams {
   GetDocumentsParams(int start_changestamp,
                      int root_feed_changestamp,
                      std::vector<DocumentFeed*>* feed_list,
                      bool should_fetch_multiple_feeds,
                      const FilePath& search_file_path,
                      const std::string& search_query,
                      const std::string& directory_resource_id,
-                     const FindEntryCallback& callback);
+                     const FindEntryCallback& callback,
+                     GDataFileSystem::GetDocumentsUIState *ui_state);
   ~GetDocumentsParams();
 
   // Changestamps are positive numbers in increasing order. The difference
   // between two changestamps is proportional equal to number of items in
   // delta feed between them - bigger the difference, more likely bigger
   // number of items in delta feeds.
   int start_changestamp;
   int root_feed_changestamp;
   scoped_ptr<std::vector<DocumentFeed*> > feed_list;
   // Should we stop after getting first feed chunk, even if there is more
   // data.
   bool should_fetch_multiple_feeds;
   FilePath search_file_path;
   std::string search_query;
   std::string directory_resource_id;
   FindEntryCallback callback;
+  scoped_ptr<GDataFileSystem::GetDocumentsUIState> ui_state;
 };
 
 GDataFileSystem::GetDocumentsParams::GetDocumentsParams(
     int start_changestamp,
     int root_feed_changestamp,
     std::vector<DocumentFeed*>* feed_list,
     bool should_fetch_multiple_feeds,
     const FilePath& search_file_path,
     const std::string& search_query,
     const std::string& directory_resource_id,
-    const FindEntryCallback& callback)
+    const FindEntryCallback& callback,
+    GDataFileSystem::GetDocumentsUIState *ui_state)
     : start_changestamp(start_changestamp),
       root_feed_changestamp(root_feed_changestamp),
       feed_list(feed_list),
       should_fetch_multiple_feeds(should_fetch_multiple_feeds),
       search_file_path(search_file_path),
       search_query(search_query),
       directory_resource_id(directory_resource_id),
-      callback(callback) {
+      callback(callback),
+      ui_state(ui_state) {
 }
 
 GDataFileSystem::GetDocumentsParams::~GetDocumentsParams() {
   STLDeleteElements(feed_list.get());
 }
 
 // GDataFileSystem::CreateDirectoryParams struct implementation.
 struct GDataFileSystem::CreateDirectoryParams {
   CreateDirectoryParams(const FilePath& created_directory_path,
                         const FilePath& target_directory_path,
@@ skipping 434 matching lines @@
                  ui_weak_ptr_,
                  initial_origin,
                  feed_load_callback,
                  base::Owned(new GetDocumentsParams(start_changestamp,
                                                     root_feed_changestamp,
                                                     feed_list.release(),
                                                     should_fetch_multiple_feeds,
                                                     search_file_path,
                                                     search_query,
                                                     directory_resource_id,
-                                                    entry_found_callback)),
+                                                    entry_found_callback,
+                                                    NULL)),
                  start_time));
 }
 
 void GDataFileSystem::OnFeedFromServerLoaded(GetDocumentsParams* params,
                                              GDataFileError error) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   if (error != GDATA_FILE_OK) {
     if (!params->callback.is_null())
       params->callback.Run(error, NULL);
@@ skipping 1719 matching lines @@
                  base::Passed(&data)));
 #endif
 
   // Add the current feed to the list of collected feeds for this directory.
   params->feed_list->push_back(current_feed.release());
 
   // 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->at(i)->entries().size();
-  NotifyDocumentFeedFetched(num_accumulated_entries);
 
   // Check if we need to collect more data to complete the directory list.
   if (params->should_fetch_multiple_feeds && 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(),
+          base::Bind(&GDataFileSystem::OnNotifyDocumentFeedFetched,
+                     ui_weak_ptr_));

[satorux1, 2012/08/02 08:03:14]

I have a feeling that this base::Bind() is unnecessary.

[satorux1, 2012/08/02 08:03:14]

I'm guessing that this callback parameter is unnecessary. see below.

[hidehiko, 2012/08/02 13:47:05]

Done.

[hidehiko, 2012/08/02 13:47:05]

Done.

+      params->ui_state.reset(ui_state);
+    }
+    DCHECK(ui_state != NULL);

[satorux1, 2012/08/02 08:03:14]

remove  != NULL, to be consistent with elsewhere.

[hidehiko, 2012/08/02 13:47:05]

Done.

+
+    if (ui_state->num_remaining_ui_update == 0) {
+      // Currently the UI update is stopped. Start UI periodical callback.
+      MessageLoop::current()->PostTask(
+          FROM_HERE,
+          base::Bind(&GetDocumentsUIState::Run, ui_state->ui_weak_ptr));

[satorux1, 2012/08/02 08:03:14]

Cannot we just do the following here?

 base::Bind(&GDataFileSystem::OnNotifyDocumentFeedFetched, ui_weak_ptr_,
ui_state) 

[hidehiko, 2012/08/02 13:47:05]

Done.

+    }
+    ui_state->num_fetched_documents = num_accumulated_entries;
+    ui_state->num_remaining_ui_update += kNumUiUpdateStepsPerFetch;
+    ui_state->fetched_duration = base::TimeTicks::Now() - start_time;
+
     // Kick of the remaining part of the feeds.
     documents_service_->GetDocuments(
         next_feed_url,
         params->start_changestamp,
         params->search_query,
         params->directory_resource_id,
         base::Bind(&GDataFileSystem::OnGetDocuments,
                    ui_weak_ptr_,
                    initial_origin,
                    callback,
                    base::Owned(
                        new GetDocumentsParams(
                            params->start_changestamp,
                            params->root_feed_changestamp,
                            params->feed_list.release(),
                            params->should_fetch_multiple_feeds,
                            params->search_file_path,
                            params->search_query,
                            params->directory_resource_id,
-                           params->callback)),
+                           params->callback,
+                           params->ui_state.release())),
                    start_time));
     return;
   }
+  NotifyDocumentFeedFetched(num_accumulated_entries);
 
   UMA_HISTOGRAM_TIMES("Gdata.EntireFeedLoadTime",
                       base::TimeTicks::Now() - start_time);
 
   if (!callback.is_null())
     callback.Run(params, error);
 }
 
 void GDataFileSystem::LoadRootFeedFromCache(
     bool should_load_from_server,
@@ skipping 706 matching lines @@
 
 void GDataFileSystem::NotifyDocumentFeedFetched(int num_accumulated_entries) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   DVLOG(1) << "Document feed fetched: " << num_accumulated_entries;
   // Notify the observers that a document feed is fetched.
   FOR_EACH_OBSERVER(Observer, observers_,
                     OnDocumentFeedFetched(num_accumulated_entries));
 }
 
+void GDataFileSystem::OnNotifyDocumentFeedFetched(
+    GDataFileSystem::GetDocumentsUIState* ui_state) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  base::TimeDelta consumed_duration =
+      base::TimeTicks::Now() - ui_state->start_time;
+
+  // Split the remaining (i.e. not yet shown on UI) fetched documents evenly
+  // to the remaining updates, cumulate one of them to the showing number of
+  // fetched documents on UI, and notify it to the observers.
+  int update_num_documents =
+      (ui_state->num_fetched_documents - ui_state->num_showing_documents)
+      / ui_state->num_remaining_ui_update;
+  ui_state->num_showing_documents += update_num_documents;
+  NotifyDocumentFeedFetched(ui_state->num_showing_documents);
+
+  --ui_state->num_remaining_ui_update;
+  if (ui_state->num_remaining_ui_update > 0) {
+    // Heuristically, we use fetched time duration to calculate the next
+    // UI update timing.
+    base::TimeDelta remaining_duration =
+        ui_state->fetched_duration - consumed_duration;
+    MessageLoop::current()->PostDelayedTask(
+        FROM_HERE,
+        base::Bind(&GetDocumentsUIState::Run, ui_state->ui_weak_ptr),
+        remaining_duration / ui_state->num_remaining_ui_update);
+  }
+}
+
+
 void GDataFileSystem::RunAndNotifyInitialLoadFinished(
     const FindEntryCallback& callback,
     GDataFileError error,
     GDataEntry* entry) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   DVLOG(1) << "Initial load finished";
   if (!callback.is_null())
     callback.Run(error, entry);
 
@@ skipping 651 matching lines @@
   }
 
   PlatformFileInfoProto entry_file_info;
   GDataEntry::ConvertPlatformFileInfoToProto(*file_info, &entry_file_info);
   *entry_proto->mutable_file_info() = entry_file_info;
   if (!callback.is_null())
     callback.Run(GDATA_FILE_OK, entry_proto.Pass());
 }
 
 }  // namespace gdata