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

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 430 matching lines @@
       // 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.

[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();
+    }
   }
 }
 
 // Runs task on UI thread.
 void RunTaskOnUIThread(const base::Closure& task) {
   RunTaskOnThread(
       BrowserThread::GetMessageLoopProxyForThread(BrowserThread::UI), task);
 }
 
 // RelayCallback relays arguments for callback running on the given thread.
@@ skipping 2236 matching lines @@
   if (error != GDATA_FILE_OK) {
     if (!callback.is_null())
       callback.Run(error, scoped_ptr<std::vector<SearchResultInfo> >());
     return;
   }
 
   // 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.
   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);
 
   if (feed->entries().empty()) {
     scoped_ptr<std::vector<SearchResultInfo> > result_vec(results);
     if (!callback.is_null())
       callback.Run(error, result_vec.Pass());
     return;
   }
@@ skipping 21 matching lines @@
       DCHECK(!entry.get());
     }
 
     // We will need information about result entry to create info for callback.
     // 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.
     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()));
   }
 }
 
 void GDataFileSystem::Search(const std::string& search_query,
                              const SearchCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI) ||
          BrowserThread::CurrentlyOn(BrowserThread::IO));
   RunTaskOnUIThread(base::Bind(&GDataFileSystem::SearchAsyncOnUIThread,
@@ skipping 1501 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