Make DriveFeedProcessor asynchronous.

chrome/browser/chromeos/drive/drive_resource_metadata.h

Index: chrome/browser/chromeos/drive/drive_resource_metadata.h
===================================================================
--- chrome/browser/chromeos/drive/drive_resource_metadata.h	(revision 167238)
+++ chrome/browser/chromeos/drive/drive_resource_metadata.h	(working copy)
@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_CHROMEOS_DRIVE_DRIVE_RESOURCE_METADATA_H_
 
 #include <map>
+#include <set>
 #include <string>
 #include <vector>
 
@@ -89,6 +90,10 @@
                             scoped_ptr<DriveEntryProto> entry_proto)>
     GetEntryInfoWithFilePathCallback;
 
+// Used to get a set of changed directories for feed processing.
+typedef base::Callback<void(const std::set<FilePath>&)>
+    ChangedDirectoriesCallback;
+
 // This is a part of EntryInfoPairResult.
 struct EntryInfoResult {
   EntryInfoResult();
@@ -224,11 +229,16 @@
       const FilePath& second_path,
       const GetEntryInfoPairCallback& callback);
 
-  // Replaces a file entry with the same resource id as |doc_entry| by deleting
-  // the existing entry, creating a new DriveFile from |doc_entry|, and adding
-  // it to the parent of the old entry. For directories, this just returns the
-  // existing directory proto. |callback| is run with the error, file path and
-  // proto of the entry. |callback| must not be null.
+  // Refreshes a drive entry with the same resource id as |entry_proto|.
+  // |callback| is run with the error, file path and proto of the entry.
+  // |callback| must not be null.
+  void RefreshEntryProto(const DriveEntryProto& entry_proto,
+                         const GetEntryInfoWithFilePathCallback& callback);
+
+  // Refresh a drive entry with resource_id that matches that of |doc_entry|,
+  // with |doc_entry|.
+  // |callback| must not be null.
+  // TODO(achuith): Deprecate this in favor of RefreshEntryProto above.
   void RefreshFile(scoped_ptr<google_apis::DocumentEntry> doc_entry,
                    const GetEntryInfoWithFilePathCallback& callback);
 
@@ -239,9 +249,27 @@
                         const DriveEntryProtoMap& entry_proto_map,
                         const FileMoveCallback& callback);
 
+  // Add |entry_proto| to the metadata tree.
+  // |callback| must not be null.
+  void AddEntryToParent(const DriveEntryProto& entry_proto,

[hashimoto, 2012/11/14 10:15:56]

Why are you adding this method instead of rewriting AddEntryToDirectory?

[achuithb, 2012/11/14 11:11:29]

AddEntryToParent has a DriveEntryProto, but does not know the path of the parent
directory. AddEntryToDirectory has a directory path, but does not have access to
the directory resource id. It's convenient to have two functions for now. It's
not obvious that they can be combined, and if they can, it's probably not right
to do it in this CL.

+                        const FileMoveCallback& callback);
+
+  // Get changed directories related to entry pointed to by |resource_id|. This
+  // includes the immediate parent, self, and children.
+  void GetChangedDirectories(

[hashimoto, 2012/11/14 10:15:56]

The term 'changed' seems not obvious in the context of DriveResourceMetadata.
How about moving this method to drive_feed_processor.cc?

[achuithb, 2012/11/14 11:11:29]

Ah, I can't do that. The code necessary to determine the changed file paths
needs to touch DriveEntry objects, and we need to move that out of
DriveFeedProcessor. We want DriveFeedProcessor to only handle DriveEntryProto
objects (which is almost true after this CL lands).

We can probably add some more to the comments, but it seems like this logic will
have to belong to DriveResourceMetadata even in the future when we use a
database.

[hashimoto, 2012/11/15 04:10:41]

The main problem here is that, to understand what "changed" means, it requires
to look at both DriveResourceMetadata and DriveFeedProcessor.
How about doing one of the following?
 (1) Rename this method to a more descriptive name like
"GetParentAndAllChildDirectoriesIfEntryIsDirectory".
 (2) Instead of adding this method, call ReadDirectoryByPath recursively from
DriveFeedProcessor.
 (3) Fix this method to just call DriveDirectory::GetChildDirectoryPaths() and
rename to "GetChildDirectoryPaths"

IIUC, the only reason why you need this method is
DriveDirectory::GetChildDirectoryPaths.
If so, I think (2) or (3) may be better than (1) because the current
implementation of this method seems too specialized.

[achuithb, 2012/11/15 23:39:54]

I've decided to go with (3). The problem with (2) is that it will require a
number of async calls which, when leveldb is implemented, result in a lot of
unnecessary ping-pong with the ui and blocking thread. It's more efficient to
get all the children in one step.

+      const std::string& resource_id,
+      const ChangedDirectoriesCallback& changed_dirs_callback);
+
+  // Helper function to get a parent directory given |parent_resource_id|.
+  // Returns root if |parent_resource_id| is empty. Returns NULL if
+  // |parent_resource_id| is not empty and the corresponding entry is not a
+  // directory.
+  DriveDirectory* GetParent(const std::string& parent_resource_id);

[hashimoto, 2012/11/14 10:15:56]

Does this method need to be public?

[achuithb, 2012/11/14 11:11:29]

Done.

+
   // Moves all child entries from the directory represented by
   // |source_resource_id| to the directory respresented by
   // |destination_resource_id|. |callback| must not be null.
+  // TODO(achuith): Delete this.
   void TakeOverEntries(const std::string& source_resource_id,
                        const std::string& destination_resource_id,
                        const FileMoveCallback& callback);
@@ -295,9 +323,14 @@
       scoped_ptr<DriveEntryProto> entry_proto);
 
   // Searches for |file_path| synchronously.
-  // TODO(satorux): Replace this with an async version crbug.com/137160
   DriveEntry* FindEntryByPathSync(const FilePath& file_path);
 
+  // Helper function to add |entry_proto| as a child to |directory|.
+  // |callback| must not be null.
+  void AddEntryToDirectoryInternal(DriveDirectory* directory,
+                                   const DriveEntryProto& entry_proto,
+                                   const FileMoveCallback& callback);
+
   // Private data members.
   scoped_refptr<base::SequencedTaskRunner> blocking_task_runner_;
   scoped_ptr<ResourceMetadataDB> resource_metadata_db_;