gdata: Remove FindEntryByPathAndRunSync() from GDataWapiFeedLoader.

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 123 matching lines @@
 bool UseLevelDB() {
   return CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kUseLevelDBForGData);
 }
 
 }  // namespace
 
 LoadRootFeedParams::LoadRootFeedParams(
     FilePath search_file_path,
     bool should_load_from_server,
-    const FindEntryCallback& callback)
+    const FileOperationCallback& callback)
     : search_file_path(search_file_path),
       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,
     const FilePath& search_file_path,
     const std::string& search_query,
     const std::string& directory_resource_id,
-    const FindEntryCallback& callback,
+    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_file_path(search_file_path),
       search_query(search_query),
       directory_resource_id(directory_resource_id),
       callback(callback),
       ui_state(ui_state) {
@@ skipping 57 matching lines @@
 
 void GDataWapiFeedLoader::RemoveObserver(Observer* observer) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   observers_.RemoveObserver(observer);
 }
 
 void GDataWapiFeedLoader::ReloadFromServerIfNeeded(
     ContentOrigin initial_origin,
     int64 local_changestamp,
     const FilePath& search_file_path,
-    const FindEntryCallback& callback) {
+    const FileOperationCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   DVLOG(1) << "ReloadFeedFromServerIfNeeded local_changestamp="
            << local_changestamp << ", initial_origin=" << initial_origin;
 
   // First fetch the latest changestamp to see if there were any new changes
   // there at all.
   if (gdata::util::IsDriveV2ApiEnabled()) {
     documents_service_->GetAboutResource(
         base::Bind(&GDataWapiFeedLoader::OnGetAboutResource,
@@ skipping 11 matching lines @@
                  initial_origin,
                  local_changestamp,
                  search_file_path,
                  callback));
 }
 
 void GDataWapiFeedLoader::OnGetAccountMetadata(
     ContentOrigin initial_origin,
     int64 local_changestamp,
     const FilePath& search_file_path,
-    const FindEntryCallback& callback,
+    const FileOperationCallback& callback,
     GDataErrorCode status,
     scoped_ptr<base::Value> feed_data) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   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 */
@@ skipping 47 matching lines @@
                    << ", 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, continue with search as planned.
+  // No changes detected, tell the client that the loading was successful.
   if (!changes_detected) {
-    if (!callback.is_null()) {
-      directory_service_->FindEntryByPathAndRunSync(search_file_path,
-                                                    callback);
-    }
+    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 */
                  search_file_path,
                  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()));
 }
 
 void GDataWapiFeedLoader::OnGetAboutResource(
     ContentOrigin initial_origin,
     int64 local_changestamp,
     const FilePath& search_file_path,
-    const FindEntryCallback& callback,
+    const FileOperationCallback& callback,
     GDataErrorCode status,
     scoped_ptr<base::Value> feed_data) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   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 */
@@ skipping 36 matching lines @@
                    << ", 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, continue with search as planned.
+  // No changes detected, tell the client that the loading was successful.
   if (!changes_detected) {
-    if (!callback.is_null()) {
-      directory_service_->FindEntryByPathAndRunSync(search_file_path,
-                                                    callback);
-    }
+    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 */
                  search_file_path,
                  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()));
 }
 
 // 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 FilePath& search_file_path,
     const std::string& search_query,
     const GURL& feed_to_load,
     const std::string& directory_resource_id,
-    const FindEntryCallback& entry_found_callback,
+    const FileOperationCallback& load_finished_callback,
     const LoadDocumentFeedCallback& feed_load_callback) {
   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,
         base::Bind(&GDataWapiFeedLoader::OnGetChangelist,
                    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_file_path,
                        search_query,
                        directory_resource_id,
-                       entry_found_callback,
+                       load_finished_callback,
                        NULL)),
                    start_time));
     return;
   }
 
   documents_service_->GetDocuments(
       feed_to_load,
       start_changestamp,
       search_query,
       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_file_path,
                                                     search_query,
                                                     directory_resource_id,
-                                                    entry_found_callback,
+                                                    load_finished_callback,
                                                     NULL)),
                  start_time));
 }
 
 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, NULL);
+      params->callback.Run(error);
     return;
   }
 
   error = UpdateFromFeed(*params->feed_list,
                          params->start_changestamp,
                          params->root_feed_changestamp);
 
   if (error != GDATA_FILE_OK) {
     if (!params->callback.is_null())
-      params->callback.Run(error, NULL);
+      params->callback.Run(error);
 
     return;
   }
 
   // Save file system metadata to disk.
   SaveFileSystem();
 
-  // If we had someone to report this too, then this retrieval was done in a
-  // context of search... so continue search.
+  // Tell the client that the loading was successful.
   if (!params->callback.is_null()) {
-    directory_service_->FindEntryByPathAndRunSync(params->search_file_path,
-                                                  params->callback);
+    params->callback.Run(GDATA_FILE_OK);
   }
 
   FOR_EACH_OBSERVER(Observer, observers_, OnFeedFromServerLoaded());
 }
 
 void GDataWapiFeedLoader::OnGetDocuments(
     ContentOrigin initial_origin,
     const LoadDocumentFeedCallback& callback,
     GetDocumentsParams* params,
     base::TimeTicks start_time,
     GDataErrorCode status,
     scoped_ptr<base::Value> data) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(!callback.is_null());
 
   if (params->feed_list->empty()) {
     UMA_HISTOGRAM_TIMES("Gdata.InitialFeedLoadTime",
                         base::TimeTicks::Now() - start_time);
   }
 
   GDataFileError error = util::GDataToGDataFileError(status);
   if (error == GDATA_FILE_OK &&
       (!data.get() || data->GetType() != Value::TYPE_DICTIONARY)) {
     error = GDATA_FILE_ERROR_FAILED;
   }
 
   if (error != GDATA_FILE_OK) {
     directory_service_->set_origin(initial_origin);
-
-    if (!callback.is_null())
-      callback.Run(params, error);
-
+    callback.Run(params, error);
     return;
   }
 
   GURL next_feed_url;
   scoped_ptr<DocumentFeed> current_feed(DocumentFeed::ExtractAndParse(*data));
   if (!current_feed.get()) {
-    if (!callback.is_null()) {
-      callback.Run(params, GDATA_FILE_ERROR_FAILED);
-    }
+    callback.Run(params, GDATA_FILE_ERROR_FAILED);
     return;
   }
   const bool has_next_feed_url = current_feed->GetNextFeedURL(&next_feed_url);
 
 #ifndef NDEBUG
   // Save initial root feed for analysis.
   std::string file_name =
       base::StringPrintf("DEBUG_feed_%" PRId64 ".json",
                          params->start_changestamp);
   util::PostBlockingPoolSequencedTask(
@@ skipping 54 matching lines @@
                            params->should_fetch_multiple_feeds,
                            params->search_file_path,
                            params->search_query,
                            params->directory_resource_id,
                            params->callback,
                            params->ui_state.release())),
                    start_time));
     return;
   }
 
-  // Notify the observers that a document feed is fetched.
+  // Notify the observers that all document feeds are fetched.
   FOR_EACH_OBSERVER(Observer, observers_,
                     OnDocumentFeedFetched(num_accumulated_entries));
 
   UMA_HISTOGRAM_TIMES("Gdata.EntireFeedLoadTime",
                       base::TimeTicks::Now() - start_time);
 
-  if (!callback.is_null())
-    callback.Run(params, error);
+  // Run the callback so the client can process the retrieved feeds.
+  callback.Run(params, error);
 }
 
 void GDataWapiFeedLoader::OnGetChangelist(
     ContentOrigin initial_origin,
     const LoadDocumentFeedCallback& callback,
     GetDocumentsParams* params,
     base::TimeTicks start_time,
     GDataErrorCode status,
     scoped_ptr<base::Value> data) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(!callback.is_null());
 
   if (params->feed_list->empty()) {
     UMA_HISTOGRAM_TIMES("Drive.InitialFeedLoadTime",
                         base::TimeTicks::Now() - start_time);
   }
 
   GDataFileError error = util::GDataToGDataFileError(status);
   if (error == GDATA_FILE_OK &&
       (!data.get() || data->GetType() != Value::TYPE_DICTIONARY)) {
     error = GDATA_FILE_ERROR_FAILED;
   }
 
   if (error != GDATA_FILE_OK) {
     directory_service_->set_origin(initial_origin);
-
-    if (!callback.is_null())
-      callback.Run(params, error);
-
+    callback.Run(params, error);
     return;
   }
 
   GURL next_feed_url;
   scoped_ptr<ChangeList> current_feed(ChangeList::CreateFrom(*data));
   if (!current_feed.get()) {
-    if (!callback.is_null()) {
-      callback.Run(params, GDATA_FILE_ERROR_FAILED);
-    }
+    callback.Run(params, GDATA_FILE_ERROR_FAILED);
     return;
   }
   const bool has_next_feed = !current_feed->next_page_token().empty();
 
 #ifndef NDEBUG
   // Save initial root feed for analysis.
   std::string file_name =
       base::StringPrintf("DEBUG_changelist_%" PRId64 ".json",
                          params->start_changestamp);
   util::PostBlockingPoolSequencedTask(
@@ skipping 54 matching lines @@
                            params->should_fetch_multiple_feeds,
                            params->search_file_path,
                            params->search_query,
                            params->directory_resource_id,
                            params->callback,
                            NULL)),
                    start_time));
     return;
   }
 
-  // Notify the observers that a document feed is fetched.
+  // Notify the observers that all document feeds are fetched.
   FOR_EACH_OBSERVER(Observer, observers_,
                     OnDocumentFeedFetched(num_accumulated_entries));
 
   UMA_HISTOGRAM_TIMES("Drive.EntireFeedLoadTime",
                       base::TimeTicks::Now() - start_time);
 
-  if (!callback.is_null())
-    callback.Run(params, error);
+  // Run the callback so the client can process the retrieved feeds.
+  callback.Run(params, error);
 }
 
 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;
@@ skipping 22 matching lines @@
                      weak_ptr_factory_.GetWeakPtr(),
                      ui_state->weak_ptr_factory.GetWeakPtr()),
           remaining_duration / num_remaining_ui_updates);
     }
   }
 }
 
 void GDataWapiFeedLoader::LoadFromCache(
     bool should_load_from_server,
     const FilePath& search_file_path,
-    const FindEntryCallback& callback) {
+    const FileOperationCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   LoadRootFeedParams* params = new LoadRootFeedParams(search_file_path,
                                                       should_load_from_server,
                                                       callback);
   FilePath path = cache_->GetCacheDirectoryPath(GDataCache::CACHE_TYPE_META);
   if (UseLevelDB()) {
     path = path.Append(kResourceMetadataDBFile);
     directory_service_->InitFromDB(path, blocking_task_runner_,
         base::Bind(
@@ skipping 35 matching lines @@
 
 void GDataWapiFeedLoader::ContinueWithInitializedDirectoryService(
     LoadRootFeedParams* params,
     GDataFileError error) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   DVLOG(1) << "Time elapsed to load directory service from disk="
            << (base::Time::Now() - params->load_start_time).InMilliseconds()
            << " milliseconds";
 
-  FindEntryCallback callback = params->callback;
-  // If we got feed content from cache, try search over it.
+  FileOperationCallback callback = params->callback;

[achuithb, 2012/08/14 23:57:36]

This callback code is a bit difficult to follow.

We make a copy of params->callback, then we may Run + Reset it, and then we pass
along the copy (not the original params->callback) to ReloadFromServerIfNeeded.
It would be nice if we didn't have callback and params->callback be different.

WDYT? Don't have to fix this here if you don't want to.

[achuithb, 2012/08/15 00:13:15]

ping

[satorux1, 2012/08/15 00:26:29]

Oops, missed this. Fully agree it's hard to follow. Let me clean this up in a
separate patch

[achuithb, 2012/08/15 00:42:33]

Could you please add a TODO in this CL?

[satorux1, 2012/08/15 01:26:23]

Done.

+  // If we got feed content from cache, tell the client that the loading was
+  // successful.
   if (error == GDATA_FILE_OK && !callback.is_null()) {
-    // Continue file content search operation if the delegate hasn't terminated
-    // this search branch already.
-    directory_service_->FindEntryByPathAndRunSync(params->search_file_path,
-                                                  callback);
+    callback.Run(GDATA_FILE_OK);
+    // Reset the callback so we don't run the same callback once
+    // ReloadFeedFromServerIfNeeded() is complete.
     callback.Reset();
   }
 
   if (!params->should_load_from_server)
     return;
 
   // Decide the |initial_origin| to pass to ReloadFromServerIfNeeded().
   // This is used to restore directory content origin to its initial value when
   // we fail to retrieve the feed from server.
   // By default, if directory content is not yet initialized, restore content
@@ skipping 65 matching lines @@
         dir_iter != changed_dirs.end(); ++dir_iter) {
       FOR_EACH_OBSERVER(Observer, observers_,
                         OnDirectoryChanged(*dir_iter));
     }
   }
 
   return error;
 }
 
 }  // namespace gdata