DriveFeedProcessor refactor #2

* Always process parent directories before their children.
* Get rid of ResourceMap, using DriveEntryMap instead.
* Make DriveFeedProcessor::ApplyFeeds and DriveFeedLoader::UpdateFromFeed asynchronous - they now take callbacks.
* Make drive_entry_map_, changed_dirs_ and callback_ member variables of DriveFeedProcessor. 
* Introduce methods ApplyNextEntryProto, ApplyNextByIterator, and ApplyEntryProto. ApplyEntryProto contains the logic of the inner-most loop of the old ApplyEntryProtoMap.

BUG=137374
TEST=unit tests
Review URL: https://codereview.chromium.org/11369020

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@166511 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 242 insertions, 169 deletions

diff --git a/chrome/browser/chromeos/drive/drive_feed_loader.cc b/chrome/browser/chromeos/drive/drive_feed_loader.cc
index 95d704d..d65e916 100644
--- a/chrome/browser/chromeos/drive/drive_feed_loader.cc
+++ b/chrome/browser/chromeos/drive/drive_feed_loader.cc
@@ -473,27 +473,18 @@ void DriveFeedLoader::OnFeedFromServerLoaded(scoped_ptr<LoadFeedParams> params,
   DCHECK(!params->load_finished_callback.is_null());
   DCHECK(refreshing_);
 
-  if (error == DRIVE_FILE_OK) {
-    UpdateFromFeed(params->feed_list,
-                   params->start_changestamp,
-                   params->root_feed_changestamp);
-  }
-  refreshing_ = false;
-
   if (error != DRIVE_FILE_OK) {
+    refreshing_ = false;
     params->load_finished_callback.Run(error);
     return;
   }
 
-  // Save file system metadata to disk.
-  SaveFileSystem();
-
-  // Tell the client that the loading was successful.
-  params->load_finished_callback.Run(DRIVE_FILE_OK);
-
-  FOR_EACH_OBSERVER(DriveFeedLoaderObserver,
-                    observers_,
-                    OnFeedFromServerLoaded());
+  UpdateFromFeed(params->feed_list,
+                 params->start_changestamp,
+                 params->root_feed_changestamp,
+                 base::Bind(&DriveFeedLoader::OnUpdateFromFeed,
+                            weak_ptr_factory_.GetWeakPtr(),
+                            params->load_finished_callback));
 }
 
 void DriveFeedLoader::OnGetDocuments(scoped_ptr<LoadFeedParams> params,
@@ -848,29 +839,62 @@ void DriveFeedLoader::SaveFileSystem() {
 void DriveFeedLoader::UpdateFromFeed(
     const ScopedVector<google_apis::DocumentFeed>& feed_list,
     int64 start_changestamp,
-    int64 root_feed_changestamp) {
+    int64 root_feed_changestamp,
+    const base::Closure& update_finished_callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DVLOG(1) << "Updating directory with a feed";
 
-  std::set<FilePath> changed_dirs;
-
-  DriveFeedProcessor feed_processor(resource_metadata_);
-  feed_processor.ApplyFeeds(
+  feed_processor_.reset(new DriveFeedProcessor(resource_metadata_));
+  // Don't send directory content change notification while performing
+  // the initial content retrieval.
+  const bool should_notify_changed_directories = (start_changestamp != 0);
+  feed_processor_->ApplyFeeds(
       feed_list,
       start_changestamp,
       root_feed_changestamp,
-      &changed_dirs);
+      base::Bind(&DriveFeedLoader::NotifyDirectoryChanged,
+                 weak_ptr_factory_.GetWeakPtr(),
+                 should_notify_changed_directories,
+                 update_finished_callback));
+}
 
-  // Don't send directory content change notification while performing
-  // the initial content retrieval.
-  const bool should_notify_directory_changed = (start_changestamp != 0);
-  if (should_notify_directory_changed) {
-    for (std::set<FilePath>::iterator dir_iter = changed_dirs.begin();
-        dir_iter != changed_dirs.end(); ++dir_iter) {
+void DriveFeedLoader::NotifyDirectoryChanged(
+    bool should_notify_changed_directories,
+    const base::Closure& update_finished_callback) {
+  DCHECK(feed_processor_.get());
+  DCHECK(!update_finished_callback.is_null());
+
+  if (should_notify_changed_directories) {
+    for (std::set<FilePath>::iterator dir_iter =
+            feed_processor_->changed_dirs().begin();
+        dir_iter != feed_processor_->changed_dirs().end();
+        ++dir_iter) {
       FOR_EACH_OBSERVER(DriveFeedLoaderObserver, observers_,
                         OnDirectoryChanged(*dir_iter));
     }
   }
+
+  update_finished_callback.Run();
+
+  // Cannot delete feed_processor_ yet because we are in on_complete_callback_,
+  // which is owned by feed_processor_.
+}
+
+void DriveFeedLoader::OnUpdateFromFeed(
+    const FileOperationCallback& load_finished_callback) {
+  DCHECK(!load_finished_callback.is_null());
+
+  refreshing_ = false;
+
+  // Save file system metadata to disk.
+  SaveFileSystem();
+
+  // Run the callback now that the filesystem is ready.
+  load_finished_callback.Run(DRIVE_FILE_OK);
+
+  FOR_EACH_OBSERVER(DriveFeedLoaderObserver,
+                    observers_,
+                    OnFeedFromServerLoaded());
 }
 
 }  // namespace drive
diff --git a/chrome/browser/chromeos/drive/drive_feed_loader.h b/chrome/browser/chromeos/drive/drive_feed_loader.h
index ebe9823..04e7b4d 100644
--- a/chrome/browser/chromeos/drive/drive_feed_loader.h
+++ b/chrome/browser/chromeos/drive/drive_feed_loader.h
@@ -30,6 +30,7 @@ namespace drive {
 
 class DriveCache;
 class DriveFeedLoaderObserver;
+class DriveFeedProcessor;
 class DriveWebAppsRegistryInterface;
 struct GetDocumentsUiState;
 struct LoadFeedParams;
@@ -141,7 +142,8 @@ class DriveFeedLoader {
   void UpdateFromFeed(
     const ScopedVector<google_apis::DocumentFeed>& feed_list,
     int64 start_changestamp,
-    int64 root_feed_changestamp);
+    int64 root_feed_changestamp,
+    const base::Closure& update_finished_callback);
 
   // Indicates whether there is a feed refreshing server request is in flight.
   bool refreshing() const { return refreshing_; }
@@ -218,12 +220,21 @@ class DriveFeedLoader {
   void OnNotifyDocumentFeedFetched(
       base::WeakPtr<GetDocumentsUiState> ui_state);
 
+  // Callback for DriveFeedProcessor::ApplyFeeds.
+  void NotifyDirectoryChanged(
+      bool should_notify,
+      const base::Closure& update_finished_callback);
+
+  // Callback for UpdateFromFeed.
+  void OnUpdateFromFeed(const FileOperationCallback& load_finished_callback);
+
   DriveResourceMetadata* resource_metadata_;  // Not owned.
   google_apis::DriveServiceInterface* drive_service_;  // Not owned.
   DriveWebAppsRegistryInterface* webapps_registry_;  // Not owned.
   DriveCache* cache_;  // Not owned.
   scoped_refptr<base::SequencedTaskRunner> blocking_task_runner_;
   ObserverList<DriveFeedLoaderObserver> observers_;
+  scoped_ptr<DriveFeedProcessor> feed_processor_;
 
   // Indicates whether there is a feed refreshing server request is in flight.
   bool refreshing_;
diff --git a/chrome/browser/chromeos/drive/drive_feed_processor.cc b/chrome/browser/chromeos/drive/drive_feed_processor.cc
index cc16974..e4242bf 100644
--- a/chrome/browser/chromeos/drive/drive_feed_processor.cc
+++ b/chrome/browser/chromeos/drive/drive_feed_processor.cc
@@ -43,7 +43,8 @@ class DriveFeedProcessor::FeedToEntryProtoMapUMAStats {
 
 DriveFeedProcessor::DriveFeedProcessor(
     DriveResourceMetadata* resource_metadata)
-  : resource_metadata_(resource_metadata) {
+  : resource_metadata_(resource_metadata),
+    ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) {
 }
 
 DriveFeedProcessor::~DriveFeedProcessor() {
@@ -53,23 +54,24 @@ void DriveFeedProcessor::ApplyFeeds(
     const ScopedVector<google_apis::DocumentFeed>& feed_list,
     int64 start_changestamp,
     int64 root_feed_changestamp,
-    std::set<FilePath>* changed_dirs) {
-  bool is_delta_feed = start_changestamp != 0;
+    const base::Closure& on_complete_callback) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(!on_complete_callback.is_null());
+
+  const bool is_delta_feed = start_changestamp != 0;
 
   int64 delta_feed_changestamp = 0;
   FeedToEntryProtoMapUMAStats uma_stats;
-  DriveEntryProtoMap entry_proto_map;
   GURL root_upload_url;
   FeedToEntryProtoMap(feed_list,
-                      &entry_proto_map,
                       &delta_feed_changestamp,
                       &root_upload_url,
                       &uma_stats);
+  // Note FeedToEntryProtoMap calls Clear() which resets on_complete_callback_.
+  on_complete_callback_ = on_complete_callback;
   ApplyEntryProtoMap(
-      entry_proto_map,
       is_delta_feed,
-      is_delta_feed ? delta_feed_changestamp : root_feed_changestamp,
-      changed_dirs);
+      is_delta_feed ? delta_feed_changestamp : root_feed_changestamp);
   if (root_upload_url.is_valid())
     resource_metadata_->root()->set_upload_url(root_upload_url);
 
@@ -78,88 +80,107 @@ void DriveFeedProcessor::ApplyFeeds(
     uma_stats.UpdateFileCountUmaHistograms();
 }
 
-void DriveFeedProcessor::ApplyEntryProtoMap(
-    const DriveEntryProtoMap& entry_proto_map,
-    bool is_delta_feed,
-    int64 feed_changestamp,
-    std::set<FilePath>* changed_dirs) {
+void DriveFeedProcessor::ApplyEntryProtoMap(bool is_delta_feed,
+                                            int64 feed_changestamp) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK(changed_dirs);
 
   if (!is_delta_feed) {  // Full update.
     resource_metadata_->root()->RemoveChildren();
-    changed_dirs->insert(resource_metadata_->root()->GetFilePath());
+    changed_dirs_.insert(resource_metadata_->root()->GetFilePath());
     resource_metadata_->set_loaded(true);
   }
   resource_metadata_->set_largest_changestamp(feed_changestamp);
 
-  // TODO(achuith): Get rid of this conversion of DriveEntryProtoMap to
-  // ResourceMap.
-  ResourceMap resource_map;
-  for (DriveEntryProtoMap::const_iterator it = entry_proto_map.begin();
-      it != entry_proto_map.end(); ++it) {
-    scoped_ptr<DriveEntry> entry =
-        resource_metadata_->CreateDriveEntryFromProto(it->second);
-    resource_map.insert(std::make_pair(it->first, entry.release()));
-  }
-
   // Go through all entries generated by the feed and apply them to the local
   // snapshot of the file system.
-  for (ResourceMap::iterator it = resource_map.begin();
-       it != resource_map.end();) {
-    // Ensure that the entry is deleted, unless the ownership is explicitly
-    // transferred by entry.release().
-    scoped_ptr<DriveEntry> entry(it->second);
-    DCHECK_EQ(it->first, entry->resource_id());
-    // Erase the entry so the deleted entry won't be referenced.
-    resource_map.erase(it++);
+  ApplyNextEntryProtoAsync();
+}
 
-    DriveEntry* old_entry =
-        resource_metadata_->GetEntryByResourceId(entry->resource_id());
+void DriveFeedProcessor::ApplyNextEntryProtoAsync() {
+  base::MessageLoopProxy::current()->PostTask(
+      FROM_HERE,
+      base::Bind(&DriveFeedProcessor::ApplyNextEntryProto,
+                 weak_ptr_factory_.GetWeakPtr()));
 
-    if (entry->is_deleted()) {
-      // Deleted file/directory.
-      DVLOG(1) << "Removing file " << entry->base_name();
-      if (!old_entry)
-        continue;
-      RemoveEntryFromParentAndCollectChangedDirectories(
-          old_entry, changed_dirs);
-
-    } else if (old_entry) {
-      // Change or move of existing entry.
-      // Please note that entry rename is just a special case of change here
-      // since name is just one of the properties that can change.
-      DVLOG(1) << "Changed file " << entry->base_name();
-
-      // Move children files over if we are dealing with directories.
-      if (old_entry->AsDriveDirectory() && entry->AsDriveDirectory()) {
-        entry->AsDriveDirectory()->TakeOverEntries(
-            old_entry->AsDriveDirectory());
-      }
+}
+
+void DriveFeedProcessor::ApplyNextEntryProto() {
+  DCHECK(!on_complete_callback_.is_null());
+
+  if (entry_proto_map_.empty()) {
+    // All entries have been processed.
+    on_complete_callback_.Run();
+    return;
+  }
+
+  ApplyNextByIterator(entry_proto_map_.begin());
+}
 
-      // Remove the old instance of this entry.
-      RemoveEntryFromParentAndCollectChangedDirectories(
-          old_entry, changed_dirs);
-
-      // Add to parent.
-      AddEntryToParentAndCollectChangedDirectories(
-          entry.release(), resource_map, changed_dirs);
-    } else {
-      // Adding a new file.
-      AddEntryToParentAndCollectChangedDirectories(
-          entry.release(), resource_map, changed_dirs);
+void DriveFeedProcessor::ApplyNextByIterator(DriveEntryProtoMap::iterator it) {
+  DriveEntryProto entry_proto = it->second;
+  DCHECK_EQ(it->first, entry_proto.resource_id());
+
+  // The parent of this entry may not yet be processed. We need the parent
+  // to be rooted in the metadata tree before we can add the child, so process
+  // the parent first.
+  DriveEntryProtoMap::iterator parent_it = entry_proto_map_.find(
+      entry_proto.parent_resource_id());
+  if (parent_it != entry_proto_map_.end()) {
+    base::MessageLoopProxy::current()->PostTask(
+        FROM_HERE,
+        base::Bind(&DriveFeedProcessor::ApplyNextByIterator,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   parent_it));
+  } else {
+    // Erase the entry so the deleted entry won't be referenced.
+    entry_proto_map_.erase(it);
+    ApplyEntryProto(entry_proto);
+  }
+}
+
+void DriveFeedProcessor::ApplyEntryProto(const DriveEntryProto& entry_proto) {
+  scoped_ptr<DriveEntry> entry =
+      resource_metadata_->CreateDriveEntryFromProto(entry_proto);
+  DCHECK(entry.get());
+  DriveEntry* old_entry =
+      resource_metadata_->GetEntryByResourceId(entry->resource_id());
+
+  if (entry->is_deleted()) {
+    // Deleted file/directory.
+    DVLOG(1) << "Removing file " << entry->base_name();
+    if (!old_entry)
+      return;
+    RemoveEntryFromParent(old_entry);
+
+  } else if (old_entry) {
+    // Change or move of existing entry.
+    // Please note that entry rename is just a special case of change here
+    // since name is just one of the properties that can change.
+    DVLOG(1) << "Changed file " << entry->base_name();
+
+    // Move children files over if we are dealing with directories.
+    if (old_entry->AsDriveDirectory() && entry->AsDriveDirectory()) {
+      entry->AsDriveDirectory()->TakeOverEntries(
+          old_entry->AsDriveDirectory());
     }
+
+    // Remove the old instance of this entry.
+    RemoveEntryFromParent(old_entry);
+
+    // Add to parent.
+    AddEntryToParent(entry.release());
+  } else {
+    // Adding a new file.
+    AddEntryToParent(entry.release());
   }
-  // All entries must be erased from the map.
-  DCHECK(resource_map.empty());
+
+  // Process the next DriveEntryProto from the map.
+  ApplyNextEntryProtoAsync();
 }
 
-void DriveFeedProcessor::AddEntryToParentAndCollectChangedDirectories(
-    DriveEntry* entry,
-    const ResourceMap& resource_map,
-    std::set<FilePath>* changed_dirs) {
-  DriveDirectory* parent =
-      ResolveParentDirectoryForNewEntry(entry, resource_map);
+void DriveFeedProcessor::AddEntryToParent(
+    DriveEntry* entry) {
+  DriveDirectory* parent = ResolveParent(entry);
 
   if (!parent) {  // Orphan.
     delete entry;
@@ -169,18 +190,17 @@ void DriveFeedProcessor::AddEntryToParentAndCollectChangedDirectories(
 
   // Notify this directory that has been created.
   if (entry->AsDriveDirectory())
-    changed_dirs->insert(entry->GetFilePath());
+    changed_dirs_.insert(entry->GetFilePath());
 
   // Notify |parent| only if it already exists by checking if it is attached to
   // the tree (has parent) or if it is root. The parent of |parent| may not
   // exist here if it is about to be created later in the same feed.
   if (parent->parent() || parent == resource_metadata_->root())
-    changed_dirs->insert(parent->GetFilePath());
+    changed_dirs_.insert(parent->GetFilePath());
 }
 
-void DriveFeedProcessor::RemoveEntryFromParentAndCollectChangedDirectories(
-    DriveEntry* entry,
-    std::set<FilePath>* changed_dirs) {
+void DriveFeedProcessor::RemoveEntryFromParent(
+    DriveEntry* entry) {
   DriveDirectory* parent = entry->parent();
   if (!parent) {
     NOTREACHED();
@@ -190,50 +210,39 @@ void DriveFeedProcessor::RemoveEntryFromParentAndCollectChangedDirectories(
   DriveDirectory* dir = entry->AsDriveDirectory();
   if (dir) {
     // We need to notify all children of entry if entry is a directory.
-    dir->GetChildDirectoryPaths(changed_dirs);
+    dir->GetChildDirectoryPaths(&changed_dirs_);
     // Besides children, notify this removed directory too.
-    changed_dirs->insert(dir->GetFilePath());
+    changed_dirs_.insert(dir->GetFilePath());
   }
 
   parent->RemoveEntry(entry);
 
   // Notify parent.
-  changed_dirs->insert(parent->GetFilePath());
+  changed_dirs_.insert(parent->GetFilePath());
 }
 
-DriveDirectory* DriveFeedProcessor::ResolveParentDirectoryForNewEntry(
-    DriveEntry* new_entry,
-    const ResourceMap& resource_map) {
+DriveDirectory* DriveFeedProcessor::ResolveParent(
+    DriveEntry* new_entry) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DriveDirectory* dir = NULL;
-  // Added file.
+
   const std::string& parent_resource_id = new_entry->parent_resource_id();
   if (parent_resource_id.empty()) {
-    dir = resource_metadata_->root();
     DVLOG(1) << "Root parent for " << new_entry->base_name();
-  } else {
-    DriveEntry* entry =
-        resource_metadata_->GetEntryByResourceId(parent_resource_id);
-    dir = entry ? entry->AsDriveDirectory() : NULL;
-    if (!dir) {
-      // The parent directory was also added with this set of feeds.
-      ResourceMap::const_iterator iter = resource_map.find(parent_resource_id);
-      if (iter != resource_map.end())
-          dir = iter->second ? iter->second->AsDriveDirectory() : NULL;
-    }
+    return resource_metadata_->root();
   }
-  DVLOG(1) << new_entry->base_name() << " parent " << parent_resource_id
-           << (dir ? " found" : " not found");
-  return dir;
+
+  DriveEntry* parent =
+      resource_metadata_->GetEntryByResourceId(parent_resource_id);
+  return parent ? parent->AsDriveDirectory() : NULL;
 }
 
 void DriveFeedProcessor::FeedToEntryProtoMap(
     const ScopedVector<google_apis::DocumentFeed>& feed_list,
-    DriveEntryProtoMap* entry_proto_map,
     int64* feed_changestamp,
     GURL* root_upload_url,
     FeedToEntryProtoMapUMAStats* uma_stats) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  Clear();
 
   for (size_t i = 0; i < feed_list.size(); ++i) {
     const google_apis::DocumentFeed* feed = feed_list[i];
@@ -265,7 +274,7 @@ void DriveFeedProcessor::FeedToEntryProtoMap(
             entry_proto.file_specific_info().is_hosted_document());
       }
 
-      std::pair<DriveEntryProtoMap::iterator, bool> ret = entry_proto_map->
+      std::pair<DriveEntryProtoMap::iterator, bool> ret = entry_proto_map_.
           insert(std::make_pair(entry_proto.resource_id(), entry_proto));
       DCHECK(ret.second);
       if (!ret.second)
@@ -274,4 +283,10 @@ void DriveFeedProcessor::FeedToEntryProtoMap(
   }
 }
 
+void DriveFeedProcessor::Clear() {
+  entry_proto_map_.clear();
+  changed_dirs_.clear();
+  on_complete_callback_.Reset();
+}
+
 }  // namespace drive
diff --git a/chrome/browser/chromeos/drive/drive_feed_processor.h b/chrome/browser/chromeos/drive/drive_feed_processor.h
index fb878f5..146a413 100644
--- a/chrome/browser/chromeos/drive/drive_feed_processor.h
+++ b/chrome/browser/chromeos/drive/drive_feed_processor.h
@@ -10,7 +10,9 @@
 #include <string>
 #include <vector>
 
+#include "base/callback.h"
 #include "base/file_path.h"
+#include "base/memory/weak_ptr.h"
 #include "chrome/browser/chromeos/drive/drive_file_error.h"
 #include "chrome/browser/google_apis/gdata_errorcode.h"
 #include "chrome/browser/google_apis/gdata_wapi_parser.h"
@@ -42,53 +44,69 @@ class DriveFeedProcessor {
   // In the case of processing the root feeds |root_feed_changestamp| is used
   // as its initial changestamp value. The value comes from
   // google_apis::AccountMetadataFeed.
+  // |on_complete_callback| is run after the feed is applied.
+  // |on_complete_callback| must not be null.
   void ApplyFeeds(
       const ScopedVector<google_apis::DocumentFeed>& feed_list,
       int64 start_changestamp,
       int64 root_feed_changestamp,
-      std::set<FilePath>* changed_dirs);
+      const base::Closure& on_complete_callback);
 
   // Converts list of document feeds from collected feeds into a
   // DriveEntryProtoMap. |feed_changestamp|, |root_upload_url|, and/or
   // |uma_stats| may be NULL.
   void FeedToEntryProtoMap(
-      const ScopedVector<google_apis::DocumentFeed>& feed_list,
-      DriveEntryProtoMap* entry_proto_map,
-      int64* feed_changestamp,
-      GURL* root_upload_url,
-      FeedToEntryProtoMapUMAStats* uma_stats);
+    const ScopedVector<google_apis::DocumentFeed>& feed_list,
+    int64* feed_changestamp,
+    GURL* root_upload_url,
+    FeedToEntryProtoMapUMAStats* uma_stats);
 
- private:
-  typedef std::map<std::string /* resource_id */, DriveEntry*> ResourceMap;
+  // A map of DriveEntryProto's representing a feed.
+  const DriveEntryProtoMap& entry_proto_map() const { return entry_proto_map_; }
+
+  // The set of changed directories as a result of feed processing.
+  const std::set<FilePath>& changed_dirs() const { return changed_dirs_; }
 
+ private:
   // Applies the pre-processed feed from |entry_proto_map| onto the filesystem.
-  void ApplyEntryProtoMap(const DriveEntryProtoMap& entry_proto_map,
-                          bool is_delta_feed,
-                          int64 feed_changestamp,
-                          std::set<FilePath>* changed_dirs);
+  void ApplyEntryProtoMap(bool is_delta_feed, int64 feed_changestamp);
+
+  // Apply the next item from entry_proto_map_ to the file system. The async
+  // version posts to the message loop to avoid recursive stack-overflow.
+  void ApplyNextEntryProto();
+  void ApplyNextEntryProtoAsync();
 
-  // Helper function for adding a |entry| from the feed to its new parent.
-  // |changed_dirs| are updated if this operation needs to raise a directory
+  // Apply |entry_proto| to resource_metadata_.
+  void ApplyEntryProto(const DriveEntryProto& entry_proto);
+
+  // Apply the DriveEntryProto pointed to by |it| to resource_metadata_.
+  void ApplyNextByIterator(DriveEntryProtoMap::iterator it);
+
+  // Helper function for adding an |entry| from the feed to its new parent.
+  // changed_dirs_ are updated if this operation needs to raise a directory
   // change notification.
-  void AddEntryToParentAndCollectChangedDirectories(
-      DriveEntry* entry,
-      const ResourceMap& resource_map,
-      std::set<FilePath>* changed_dirs);
+  void AddEntryToParent(DriveEntry* entry);
 
   // Helper function for removing |entry| from its parent.
-  // |changed_dirs| are updated if this operation needs to raise a directory
+  // changed_dirs_ are updated if this operation needs to raise a directory
   // change notification, including child directories.
-  void RemoveEntryFromParentAndCollectChangedDirectories(
-      DriveEntry* entry,
-      std::set<FilePath>* changed_dirs);
+  void RemoveEntryFromParent(DriveEntry* entry);
+
+  // Resolves parent directory for |new_entry| from the feed.
+  DriveDirectory* ResolveParent(DriveEntry* new_entry);
 
-  // Resolves directory where |new_entry| should be added to during feed
-  // processing.
-  DriveDirectory* ResolveParentDirectoryForNewEntry(
-      DriveEntry* new_entry,
-      const ResourceMap& resource_map);
+  // Reset the state of this object.
+  void Clear();
 
   DriveResourceMetadata* resource_metadata_;  // Not owned.
+
+  DriveEntryProtoMap entry_proto_map_;
+  std::set<FilePath> changed_dirs_;
+  base::Closure on_complete_callback_;
+
+  // Note: This should remain the last member so it'll be destroyed and
+  // invalidate its weak pointers before any other members are destroyed.
+  base::WeakPtrFactory<DriveFeedProcessor> weak_ptr_factory_;
   DISALLOW_COPY_AND_ASSIGN(DriveFeedProcessor);
 };
 
diff --git a/chrome/browser/chromeos/drive/drive_file_system.cc b/chrome/browser/chromeos/drive/drive_file_system.cc
index dd95611..0296d73 100644
--- a/chrome/browser/chromeos/drive/drive_file_system.cc
+++ b/chrome/browser/chromeos/drive/drive_file_system.cc
@@ -1263,18 +1263,12 @@ void DriveFileSystem::OnRequestDirectoryRefresh(
     return;
   }
 
-  DriveFeedProcessor::DriveEntryProtoMap entry_proto_map;
   DriveFeedProcessor feed_processor(resource_metadata_.get());
-  feed_processor.FeedToEntryProtoMap(
-      params->feed_list,
-      &entry_proto_map,
-      NULL,   // feed_changestamp.
-      NULL,   // root_upload_url.
-      NULL);  // uma_stats.
+  feed_processor.FeedToEntryProtoMap(params->feed_list, NULL, NULL, NULL);
 
   resource_metadata_->RefreshDirectory(
       params->directory_resource_id,
-      entry_proto_map,
+      feed_processor.entry_proto_map(),
       base::Bind(&DriveFileSystem::OnDirectoryChangeFileMoveCallback,
                  ui_weak_ptr_,
                  FileOperationCallback()));
diff --git a/chrome/browser/chromeos/drive/drive_test_util.cc b/chrome/browser/chromeos/drive/drive_test_util.cc
index 1e0f492..256b3f7 100644
--- a/chrome/browser/chromeos/drive/drive_test_util.cc
+++ b/chrome/browser/chromeos/drive/drive_test_util.cc
@@ -14,6 +14,13 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace drive {
+namespace {
+
+void OnUpdateFromFeed() {
+}
+
+}  // namespace
+
 namespace test_util {
 
 DriveCacheEntry ToCacheEntry(int cache_state) {
@@ -121,9 +128,13 @@ void LoadChangeFeed(const std::string& relative_path,
   ScopedVector<google_apis::DocumentFeed> feed_list;
   feed_list.push_back(document_feed.release());
 
-  file_system->feed_loader()->UpdateFromFeed(feed_list,
-                                             start_changestamp,
-                                             root_feed_changestamp);
+  file_system->feed_loader()->UpdateFromFeed(
+      feed_list,
+      start_changestamp,
+      root_feed_changestamp,
+      base::Bind(&OnUpdateFromFeed));
+  // DriveFeedLoader::UpdateFromFeed is asynchronous, so wait for it to finish.
+  google_apis::test_util::RunBlockingPoolTask();
 }
 
 }  // namespace test_util