gdata: Trigger delta feed when search results contained unknown entries.

chrome/browser/chromeos/gdata/gdata_file_system.cc

Index: chrome/browser/chromeos/gdata/gdata_file_system.cc
diff --git a/chrome/browser/chromeos/gdata/gdata_file_system.cc b/chrome/browser/chromeos/gdata/gdata_file_system.cc
index dbac872e1e02f01cbcb98b8a5e338196cd80d7bd..12fedb29068239d16ee4e7e744f0c92971863704 100644
--- a/chrome/browser/chromeos/gdata/gdata_file_system.cc
+++ b/chrome/browser/chromeos/gdata/gdata_file_system.cc
@@ -451,22 +451,35 @@ void RemoveStaleEntryOnUpload(const std::string& resource_id,
 // |run_callback| is true.

[satorux1, 2012/07/30 10:17:18]

please document about the new parameters.

[kinaba, 2012/07/31 08:46:04]

Done.

 void AddEntryToSearchResults(
     std::vector<SearchResultInfo>* results,
+    bool* entry_skipped,
     const SearchCallback& callback,
+    const base::Closure& entry_skipped_callback,
     GDataFileError error,
     bool run_callback,
     GDataEntry* entry) {
-  // If a result is not present in our local file system snapshot, ignore it.
+  DCHECK(entry_skipped);
+
+  // If a result is not present in our local file system snapshot, invoke
+  // |entry_skipped_callback| and refreshes the snapshot with delta feed.
   // 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));
+  } else {
+    *entry_skipped = true;
   }
 
   if (run_callback) {
     scoped_ptr<std::vector<SearchResultInfo> > result_vec(results);
     if (!callback.is_null())
       callback.Run(error, result_vec.Pass());
+
+    if (*entry_skipped) {
+      delete entry_skipped;

[satorux1, 2012/07/30 10:17:18]

does this mean |entry_skipped| will be leaked, if *entry_skipped is false?

Do we need to create and pass entry_skipped from the caller? Can we just have a
bool entry_skipped declared in this function?

[tbarzic, 2012/07/30 23:32:33]

agree, I don't really see the point of using bool* for entry skipped (vagrind
also says we leak it)

[kinaba, 2012/07/31 08:46:04]

Done. Removed the boolean variable.

The leak is completely my silly mistake, but my intent for sharing
a single flag is to request delta feed only once per one search.
The current version calls |entry_skipped_callback| as many time as
the number of unknown entries, but, yes, it is ok. |CheckForUpdates|
anyhow controls the redundant refresh requests.

[tbarzic, 2012/07/31 17:43:25]

yeah, I figured the leak was not intentional :)

I missed the fact that this is behind |run_callback| in initial review (and thus
didn't see the point of bool*)... sorry for that

I'm fine with both your former and current solution..

+      if (!entry_skipped_callback.is_null())
+        entry_skipped_callback.Run();
+    }
   }
 }
 
@@ -2723,6 +2736,7 @@ void GDataFileSystem::OnSearch(const SearchCallback& callback,
   // The directory is not really part of the file system, so it has no parent or
   // root.
   std::vector<SearchResultInfo>* results(new std::vector<SearchResultInfo>());
+  bool* entry_skipped(new bool(false));
 
   DCHECK_EQ(1u, params->feed_list->size());
   DocumentFeed* feed = params->feed_list->at(0);
@@ -2764,7 +2778,9 @@ void GDataFileSystem::OnSearch(const SearchCallback& callback,
     directory_service_->GetEntryByResourceIdAsync(entry_resource_id,
         base::Bind(&AddEntryToSearchResults,
                    results,
+                   entry_skipped,
                    callback,
+                   base::Bind(&GDataFileSystem::CheckForUpdates, ui_weak_ptr_),
                    error,
                    i+1 == feed->entries().size()));
   }