Make DriveFeedProcessor asynchronous.

chrome/browser/chromeos/drive/drive_resource_metadata.h

 // 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.
 
 #ifndef CHROME_BROWSER_CHROMEOS_DRIVE_DRIVE_RESOURCE_METADATA_H_
 #define CHROME_BROWSER_CHROMEOS_DRIVE_DRIVE_RESOURCE_METADATA_H_
 
 #include <map>
+#include <set>
 #include <string>
 #include <vector>
 
 #include "base/callback_forward.h"
 #include "base/file_path.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/time.h"
 #include "chrome/browser/chromeos/drive/drive_file_error.h"
 
@@ skipping 63 matching lines @@
 // If |error| is not DRIVE_FILE_OK, |entry_proto| is set to NULL.
 //
 // |drive_file_path| parameter is provided as DriveEntryProto does not contain
 // the Drive file path (i.e. only contains the base name without parent
 // directory names).
 typedef base::Callback<void(DriveFileError error,
                             const FilePath& drive_file_path,
                             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();
   ~EntryInfoResult();
 
   FilePath path;
   DriveFileError error;
   scoped_ptr<DriveEntryProto> proto;
 };
 
@@ skipping 115 matching lines @@
   // |first_path| is not found, this function does not try to get the
   // entry of |second_path|.
   //
   // Must be called from UI thread. |callback| is run on UI thread.
   // |callback| must not be null.
   void GetEntryInfoPairByPaths(
       const FilePath& first_path,
       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);
 
   // Removes all child files of |directory| and replaces them with
   // |entry_proto_map|. |callback| is called with the directory path.
   // |callback| must not be null.
   void RefreshDirectory(const std::string& directory_resource_id,
                         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);
 
   // Serializes/Parses to/from string via proto classes.
   void SerializeToString(std::string* serialized_proto) const;
   bool ParseFromString(const std::string& serialized_proto);
 
   // Restores from and saves to database, calling |callback| asynchronously.
   // |callback| must not be null.
@@ skipping 33 matching lines @@
   // Continues with GetIntroInfoPairByPaths after the second DriveEntry has been
   // asynchronously fetched.
   void GetEntryInfoPairByPathsAfterGetSecond(
       const FilePath& second_path,
       const GetEntryInfoPairCallback& callback,
       scoped_ptr<EntryInfoPairResult> result,
       DriveFileError error,
       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_;
 
   ResourceMap resource_map_;
 
   scoped_ptr<DriveDirectory> root_;  // Stored in the serialized proto.
 
   base::Time last_serialized_;
   size_t serialized_size_;
   int64 largest_changestamp_;  // Stored in the serialized proto.
   bool loaded_;
 
   // This should remain the last member so it'll be destroyed first and
   // invalidate its weak pointers before other members are destroyed.
   base::WeakPtrFactory<DriveResourceMetadata> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(DriveResourceMetadata);
 };
 
 }  // namespace drive
 
 #endif  // CHROME_BROWSER_CHROMEOS_DRIVE_DRIVE_RESOURCE_METADATA_H_