Replace GetEntryByResourceId with GetEntryByResourceIdAsync in search and file upload

chrome/browser/chromeos/gdata/gdata_file_system.cc

Index: chrome/browser/chromeos/gdata/gdata_file_system.cc
===================================================================
--- chrome/browser/chromeos/gdata/gdata_file_system.cc	(revision 145967)
+++ chrome/browser/chromeos/gdata/gdata_file_system.cc	(working copy)
@@ -419,6 +419,40 @@
   }
 }
 
+// Callback for GetEntryByResourceIdAsync.
+// Removes stale entry upon upload of file.
+void RemoveStaleEntryOnUpload(const std::string& resource_id,
+                              GDataDirectory* parent_dir,
+                              GDataEntry* existing_entry) {
+  if (existing_entry &&
+      // This should always match, but just in case.
+      existing_entry->parent() == parent_dir) {
+    parent_dir->RemoveEntry(existing_entry);
+  } else {
+    LOG(ERROR) << "Entry for the existing file not found: " << resource_id;
+  }
+}
+
+// Callback for GetEntryByResourceIdAsync.
+// Adds |entry| to |results|. Runs |callback| with |results| when
+// |run_callback| is true.
+void AddEntryToSearchResults(
+    std::vector<SearchResultInfo>* results,
+    const SearchCallback& callback,
+    base::PlatformFileError error,
+    bool run_callback,
+    GDataEntry* entry) {
+  // If a result is not present in our local file system snapshot, ignore it.
+  // For example, this may happen if the entry has recently been added to the
+  // drive (and we still haven't received its delta feed).
+  if (entry) {
+    const bool is_directory = entry->AsGDataDirectory() != NULL;
+    results->push_back(SearchResultInfo(entry->GetFilePath(), is_directory));
+  }
+  if (run_callback && !callback.is_null())
+    callback.Run(error, scoped_ptr<std::vector<SearchResultInfo> >(results));

[hshi1, 2012/07/11 00:23:44]

Is it possible that |callback| is null? It seems that if callback is null, we
may leak memory associated with the std::vector<SearchResultInfo> |results|,
because the scoped_ptr would not be initialized with the |results|.

[achuithb, 2012/07/11 00:38:10]

I don't think the callback would be NULL since we're doing a lot of work to
collect the results. This is Toni's code so I'm not sure. I've added code to
defend against this potential leak.

+}
+
 // Runs task on UI thread.
 void RunTaskOnUIThread(const base::Closure& task) {
   RunTaskOnThread(
@@ -2502,8 +2536,7 @@
   // The search results will be returned using virtual directory.
   // The directory is not really part of the file system, so it has no parent or
   // root.
-  scoped_ptr<std::vector<SearchResultInfo> > results(
-      new std::vector<SearchResultInfo>());
+  std::vector<SearchResultInfo>* results(new std::vector<SearchResultInfo>());
 
   DCHECK_EQ(1u, params->feed_list->size());
   DocumentFeed* feed = params->feed_list->at(0);
@@ -2535,21 +2568,13 @@
     // We can't use |entry| anymore, so we have to refetch entry from file
     // system. Also, |entry| doesn't have file path set before |RefreshFile|
     // call, so we can't get file path from there.
-    GDataEntry* saved_entry = root_->GetEntryByResourceId(entry_resource_id);
-
-    // If a result is not present in our local file system snapshot, ignore it.
-    // For example, this may happen if the entry has recently been added to the
-    // drive (and we still haven't received its delta feed).
-    if (!saved_entry)
-      continue;
-
-    bool is_directory = saved_entry->AsGDataDirectory() != NULL;
-    results->push_back(SearchResultInfo(saved_entry->GetFilePath(),
-                                        is_directory));
+    root_->GetEntryByResourceIdAsync(entry_resource_id,
+        base::Bind(&AddEntryToSearchResults,
+                   results,
+                   callback,
+                   error,
+                   i+1 == feed->entries().size()));
   }
-
-  if (!callback.is_null())
-    callback.Run(error, results.Pass());
 }
 
 void GDataFileSystem::Search(const std::string& search_query,
@@ -3560,16 +3585,9 @@
 
   if (upload_mode == UPLOAD_EXISTING_FILE) {
     // Remove an existing entry, which should be present.
-    GDataEntry* existing_entry = root_->GetEntryByResourceId(
-        new_entry->resource_id());
-    if (existing_entry &&
-        // This should always match, but just in case.
-        existing_entry->parent() == parent_dir) {
-      parent_dir->RemoveEntry(existing_entry);
-    } else {
-      LOG(ERROR) << "Entry for the existing file not found: "
-                 << new_entry->resource_id();
-    }
+    const std::string& resource_id = new_entry->resource_id();
+    root_->GetEntryByResourceIdAsync(resource_id,
+        base::Bind(&RemoveStaleEntryOnUpload, resource_id, parent_dir));
   }
 
   GDataFile* file = new_entry->AsGDataFile();