Refactor GDataWapiFeedLoader::LoadFromServer() parameters

chrome/browser/chromeos/gdata/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/gdata/gdata_wapi_feed_loader.h"
 
 #include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/format_macros.h"
 #include "base/json/json_reader.h"
@@ skipping 120 matching lines @@
   }
 }
 
 bool UseLevelDB() {
   return CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kUseLevelDBForGData);
 }
 
 }  // namespace
 
-LoadRootFeedParams::LoadRootFeedParams(
-    bool should_load_from_server,
-    const FileOperationCallback& callback)
-    : should_load_from_server(should_load_from_server),
-      load_error(GDATA_FILE_OK),
-      load_start_time(base::Time::Now()),
-      callback(callback) {
-}
-
-LoadRootFeedParams::~LoadRootFeedParams() {
-}
-
 GetDocumentsParams::GetDocumentsParams(
     int64 start_changestamp,
     int64 root_feed_changestamp,
     std::vector<DocumentFeed*>* feed_list,
-    bool should_fetch_multiple_feeds,

[satorux1, 2012/08/17 11:01:31]

so the parameter was unsed in the existing code?

[kochi, 2012/08/17 15:56:37]

Yes, it is unused.
The default value was true, and it was never changed.

Added a note in the CL description.

     const std::string& search_query,
     const std::string& directory_resource_id,
     const FileOperationCallback& callback,
     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_query(search_query),
       directory_resource_id(directory_resource_id),
       callback(callback),
       ui_state(ui_state) {
 }
 
 GetDocumentsParams::~GetDocumentsParams() {
   STLDeleteElements(feed_list.get());
 }
 
+LoadRootFeedParams::LoadRootFeedParams(
+    bool should_load_from_server,
+    const FileOperationCallback& callback)
+    : should_load_from_server(should_load_from_server),
+      load_error(GDATA_FILE_OK),
+      load_start_time(base::Time::Now()),
+      callback(callback) {
+}
+
+LoadRootFeedParams::~LoadRootFeedParams() {
+}
+
+// Defines set of parameters for calling GDataWapiFeedLoader::LoadFromServer().
+// Value of |start_changestamp| determines the type of feed to load - 0 means
+// root feed, every other value would trigger delta feed.
+// In the case of loading the root feed we use |root_feed_changestamp| as its
+// initial changestamp value since it does not come with that info.
+//
+// When all feeds are loaded, |feed_load_callback| is invoked with the retrieved
+// feeds. Then |load_finished_callback| is invoked with the error code.
+//
+// If invoked as a part of content search, query will be set in |search_query|.
+// If |feed_to_load| is set, this is feed url that will be used to load feed.
+//
+// |load_finished_callback| may be null.
+// |feed_load_callback| must not be null.
+struct GDataWapiFeedLoader::LoadFeedParams {
+  LoadFeedParams(ContentOrigin initial_origin,
+                 const LoadDocumentFeedCallback& feed_load_callback)
+      : initial_origin(initial_origin),
+        start_changestamp(0),
+        root_feed_changestamp(0),
+        feed_load_callback(feed_load_callback) {}
+  ~LoadFeedParams() {}
+
+  ContentOrigin initial_origin;
+  int64 start_changestamp;
+  int64 root_feed_changestamp;
+  std::string search_query;
+  GURL feed_to_load;
+  std::string directory_resource_id;
+  FileOperationCallback load_finished_callback;
+  const LoadDocumentFeedCallback feed_load_callback;
+};
+
 // Defines set of parameters sent to callback OnNotifyDocumentFeedFetched().
 // 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 GetDocumentsUiState {
   explicit GetDocumentsUiState(base::TimeTicks start_time)
       : num_fetched_documents(0),
@@ skipping 80 matching lines @@
 }
 
 void GDataWapiFeedLoader::OnGetAccountMetadata(
     ContentOrigin initial_origin,
     int64 local_changestamp,
     const FileOperationCallback& callback,
     GDataErrorCode status,
     scoped_ptr<base::Value> feed_data) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
+  LoadFeedParams params(initial_origin,
+                        base::Bind(&GDataWapiFeedLoader::OnFeedFromServerLoaded,
+                                   weak_ptr_factory_.GetWeakPtr()));
+  params.start_changestamp = local_changestamp + 1;
+  params.load_finished_callback = callback;
+
   GDataFileError error = util::GDataToGDataFileError(status);
   if (error != GDATA_FILE_OK) {
     // Get changes starting from the next changestamp from what we have locally.
-    LoadFromServer(initial_origin,
-                   local_changestamp + 1, 0,
-                   true,  /* should_fetch_multiple_feeds */
-                   std::string() /* no search query */,
-                   GURL(), /* feed not explicitly set */
-                   std::string() /* no directory resource ID */,
-                   callback,
-                   base::Bind(&GDataWapiFeedLoader::OnFeedFromServerLoaded,
-                              weak_ptr_factory_.GetWeakPtr()));
+    LoadFromServer(params);
     return;
   }
 
   scoped_ptr<AccountMetadataFeed> account_metadata;
   if (feed_data.get()) {
     account_metadata = AccountMetadataFeed::CreateFrom(*feed_data);
 #ifndef NDEBUG
     // Save account metadata feed for analysis.
     const FilePath path =
         cache_->GetCacheDirectoryPath(GDataCache::CACHE_TYPE_META).Append(
             kAccountMetadataFile);
     util::PostBlockingPoolSequencedTask(
         FROM_HERE,
         blocking_task_runner_,
         base::Bind(&SaveFeedOnBlockingPoolForDebugging,
                    path, base::Passed(&feed_data)));
 #endif
   }
 
   if (!account_metadata.get()) {
-    LoadFromServer(initial_origin,
-                   local_changestamp + 1, 0,
-                   true,  /* should_fetch_multiple_feeds */
-                   std::string() /* no search query */,
-                   GURL(), /* feed not explicitly set */
-                   std::string() /* no directory resource ID */,
-                   callback,
-                   base::Bind(&GDataWapiFeedLoader::OnFeedFromServerLoaded,
-                              weak_ptr_factory_.GetWeakPtr()));

[satorux1, 2012/08/17 11:01:31]

glad that the large copy-paste is gone.

+    LoadFromServer(params);
     return;
   }
 
   webapps_registry_->UpdateFromFeed(*account_metadata.get());
 
   bool changes_detected = true;
   if (local_changestamp >= account_metadata->largest_changestamp()) {
     if (local_changestamp > account_metadata->largest_changestamp()) {
       LOG(WARNING) << "Cached client feed is fresher than server, client = "
                    << local_changestamp
                    << ", server = "
                    << account_metadata->largest_changestamp();
     }
     // If our cache holds the latest state from the server, change the
     // state to FROM_SERVER.
     directory_service_->set_origin(
         initial_origin == FROM_CACHE ? FROM_SERVER : initial_origin);
     changes_detected = false;
   }
 
   // No changes detected, tell the client that the loading was successful.
   if (!changes_detected) {
     if (!callback.is_null())
       callback.Run(GDATA_FILE_OK);
     return;
   }
 
   // Load changes from the server.
-  LoadFromServer(initial_origin,
-                 local_changestamp > 0 ? local_changestamp + 1 : 0,
-                 account_metadata->largest_changestamp(),
-                 true,  /* should_fetch_multiple_feeds */
-                 std::string() /* no search query */,
-                 GURL(), /* feed not explicitly set */
-                 std::string() /* no directory resource ID */,
-                 callback,
-                 base::Bind(&GDataWapiFeedLoader::OnFeedFromServerLoaded,
-                            weak_ptr_factory_.GetWeakPtr()));
+  params.start_changestamp = local_changestamp > 0 ? local_changestamp + 1 : 0;
+  params.root_feed_changestamp = account_metadata->largest_changestamp();
+  LoadFromServer(params);
 }
 
 void GDataWapiFeedLoader::OnGetAboutResource(
     ContentOrigin initial_origin,
     int64 local_changestamp,
     const FileOperationCallback& callback,
     GDataErrorCode status,
     scoped_ptr<base::Value> feed_data) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
+  LoadFeedParams params(initial_origin,
+                        base::Bind(&GDataWapiFeedLoader::OnFeedFromServerLoaded,
+                                   weak_ptr_factory_.GetWeakPtr()));
+  params.load_finished_callback = callback;
+
   GDataFileError error = util::GDataToGDataFileError(status);
   if (error != GDATA_FILE_OK) {
     // Get changes starting from the next changestamp from what we have locally.
-    LoadFromServer(initial_origin,
-                   local_changestamp + 1, 0,
-                   true,  /* should_fetch_multiple_feeds */
-                   std::string() /* no search query */,
-                   GURL(), /* feed not explicitly set */
-                   std::string() /* no directory resource ID */,
-                   callback,
-                   base::Bind(&GDataWapiFeedLoader::OnFeedFromServerLoaded,
-                              weak_ptr_factory_.GetWeakPtr()));
+    LoadFromServer(params);
     return;
   }
 
   scoped_ptr<AboutResource> about_resource;
   if (feed_data.get())
     about_resource = AboutResource::CreateFrom(*feed_data);
 
   if (!about_resource.get()) {
-    LoadFromServer(initial_origin,
-                   local_changestamp + 1, 0,
-                   true,  /* should_fetch_multiple_feeds */
-                   std::string() /* no search query */,
-                   GURL(), /* feed not explicitly set */
-                   std::string() /* no directory resource ID */,
-                   callback,
-                   base::Bind(&GDataWapiFeedLoader::OnFeedFromServerLoaded,
-                              weak_ptr_factory_.GetWeakPtr()));
+    LoadFromServer(params);
     return;
   }
 
   bool changes_detected = true;
   int64 largest_changestamp = about_resource->largest_change_id();
   directory_service_->InitializeRootEntry(about_resource->root_folder_id());
 
   if (local_changestamp >= largest_changestamp) {
     if (local_changestamp > largest_changestamp) {
       LOG(WARNING) << "Cached client feed is fresher than server, client = "
                    << local_changestamp
                    << ", server = "
                    << largest_changestamp;
     }
     // If our cache holds the latest state from the server, change the
     // state to FROM_SERVER.
     directory_service_->set_origin(
         initial_origin == FROM_CACHE ? FROM_SERVER : initial_origin);
     changes_detected = false;
   }
 
   // No changes detected, tell the client that the loading was successful.
   if (!changes_detected) {
     if (!callback.is_null())
       callback.Run(GDATA_FILE_OK);
     return;
   }
 
   // Load changes from the server.
-  LoadFromServer(initial_origin,
-                 local_changestamp > 0 ? local_changestamp + 1 : 0,
-                 largest_changestamp,
-                 true,  /* should_fetch_multiple_feeds */
-                 std::string() /* no search query */,
-                 GURL(), /* feed not explicitly set */
-                 std::string() /* no directory resource ID */,
-                 callback,
-                 base::Bind(&GDataWapiFeedLoader::OnFeedFromServerLoaded,
-                            weak_ptr_factory_.GetWeakPtr()));
+  params.start_changestamp = local_changestamp > 0 ? local_changestamp + 1 : 0;
+  params.root_feed_changestamp = largest_changestamp;
+  LoadFromServer(params);
 }
 
 void GDataWapiFeedLoader::OnGetApplicationList(
     GDataErrorCode status,
     scoped_ptr<base::Value> json) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   GDataFileError error = util::GDataToGDataFileError(status);
   if (error != GDATA_FILE_OK)
     return;
 
   if (json.get()) {
     scoped_ptr<AppList> applist(AppList::CreateFrom(*json));
     if (applist.get()) {
       VLOG(1) << "applist get success";
       webapps_registry_->UpdateFromApplicationList(*applist.get());
     }
   }
 }
 
-// TODO(kochi): Fix too many parameters.  http://crbug.com/141359
-void GDataWapiFeedLoader::LoadFromServer(
-    ContentOrigin initial_origin,
-    int64 start_changestamp,
-    int64 root_feed_changestamp,
-    bool should_fetch_multiple_feeds,
-    const std::string& search_query,
-    const GURL& feed_to_load,
-    const std::string& directory_resource_id,
-    const FileOperationCallback& load_finished_callback,
-    const LoadDocumentFeedCallback& feed_load_callback) {
+void GDataWapiFeedLoader::LoadFromServer(const LoadFeedParams& params) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK(!feed_load_callback.is_null());
 
   // |feed_list| will contain the list of all collected feed updates that
   // we will receive through calls of DocumentsService::GetDocuments().
   scoped_ptr<std::vector<DocumentFeed*> > feed_list(
       new std::vector<DocumentFeed*>);
   const base::TimeTicks start_time = base::TimeTicks::Now();
 
   if (gdata::util::IsDriveV2ApiEnabled()) {
     documents_service_->GetChangelist(
-        feed_to_load,
-        start_changestamp,
+        params.feed_to_load,
+        params.start_changestamp,
         base::Bind(&GDataWapiFeedLoader::OnGetChangelist,
                    weak_ptr_factory_.GetWeakPtr(),
-                   initial_origin,
-                   feed_load_callback,
+                   params.initial_origin,
+                   params.feed_load_callback,
                    base::Owned(new GetDocumentsParams(
-                       start_changestamp,
-                       root_feed_changestamp,
+                       params.start_changestamp,
+                       params.root_feed_changestamp,
                        feed_list.release(),
-                       should_fetch_multiple_feeds,
-                       search_query,
-                       directory_resource_id,
-                       load_finished_callback,
+                       params.search_query,
+                       params.directory_resource_id,
+                       params.load_finished_callback,
                        NULL)),
                    start_time));
     return;
   }
 
   documents_service_->GetDocuments(
-      feed_to_load,
-      start_changestamp,
-      search_query,
-      directory_resource_id,
+      params.feed_to_load,
+      params.start_changestamp,
+      params.search_query,
+      params.directory_resource_id,
       base::Bind(&GDataWapiFeedLoader::OnGetDocuments,
                  weak_ptr_factory_.GetWeakPtr(),
-                 initial_origin,
-                 feed_load_callback,
-                 base::Owned(new GetDocumentsParams(start_changestamp,
-                                                    root_feed_changestamp,
-                                                    feed_list.release(),
-                                                    should_fetch_multiple_feeds,
-                                                    search_query,
-                                                    directory_resource_id,
-                                                    load_finished_callback,
-                                                    NULL)),
+                 params.initial_origin,
+                 params.feed_load_callback,
+                 base::Owned(
+                     new GetDocumentsParams(params.start_changestamp,
+                                            params.root_feed_changestamp,
+                                            feed_list.release(),
+                                            params.search_query,
+                                            params.directory_resource_id,
+                                            params.load_finished_callback,
+                                            NULL)),
                  start_time));
 }
 
+void GDataWapiFeedLoader::LoadDirectoryFromServer(
+    ContentOrigin initial_origin,
+    const std::string& directory_resource_id,
+    const LoadDocumentFeedCallback& feed_load_callback) {
+  LoadFeedParams params(initial_origin, feed_load_callback);
+  params.directory_resource_id = directory_resource_id;
+  LoadFromServer(params);
+}
+
+void GDataWapiFeedLoader::SearchFromServer(
+    ContentOrigin initial_origin,
+    const std::string& search_query,
+    const GURL& next_feed,
+    const LoadDocumentFeedCallback& feed_load_callback) {
+  LoadFeedParams params(initial_origin, feed_load_callback);
+  params.search_query = search_query;
+  params.feed_to_load = next_feed;
+  LoadFromServer(params);
+}
+
 void GDataWapiFeedLoader::OnFeedFromServerLoaded(GetDocumentsParams* params,
                                                  GDataFileError error) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   if (error != GDATA_FILE_OK) {
     if (!params->callback.is_null())
       params->callback.Run(error);
     return;
   }
 
@@ skipping 70 matching lines @@
 
   // 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();
 
   // 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()) {
+  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());
       params->ui_state.reset(ui_state);
     }
     DCHECK(ui_state);
 
     if ((ui_state->num_fetched_documents - ui_state->num_showing_documents)
         < kFetchUiUpdateStep) {
@@ skipping 15 matching lines @@
         params->directory_resource_id,
         base::Bind(&GDataWapiFeedLoader::OnGetDocuments,
                    weak_ptr_factory_.GetWeakPtr(),
                    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_query,
                            params->directory_resource_id,
                            params->callback,
                            params->ui_state.release())),
                    start_time));
     return;
   }
 
   // Notify the observers that all document feeds are fetched.
   FOR_EACH_OBSERVER(Observer, observers_,
@@ skipping 59 matching lines @@
   scoped_ptr<DocumentFeed> feed =
       DocumentFeed::CreateFromChangeList(*current_feed);
   params->feed_list->push_back(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();
 
   // Check if we need to collect more data to complete the directory list.
-  if (params->should_fetch_multiple_feeds && has_next_feed) {
+  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());
       params->ui_state.reset(ui_state);
     }
     DCHECK(ui_state);
 
     if ((ui_state->num_fetched_documents - ui_state->num_showing_documents)
@@ skipping 14 matching lines @@
         params->start_changestamp,
         base::Bind(&GDataWapiFeedLoader::OnGetChangelist,
                    weak_ptr_factory_.GetWeakPtr(),
                    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_query,
                            params->directory_resource_id,
                            params->callback,
                            NULL)),
                    start_time));
     return;
   }
 
   // Notify the observers that all document feeds are fetched.
   FOR_EACH_OBSERVER(Observer, observers_,
@@ skipping 185 matching lines @@
         dir_iter != changed_dirs.end(); ++dir_iter) {
       FOR_EACH_OBSERVER(Observer, observers_,
                         OnDirectoryChanged(*dir_iter));
     }
   }
 
   return error;
 }
 
 }  // namespace gdata