Use delete journal to remove bookmarks that are already deleted in sync model

chrome/browser/sync/glue/bookmark_model_associator.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/sync/glue/bookmark_model_associator.h"
 
 #include <stack>
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 69 matching lines @@
 // Provides the following abstraction: given a parent bookmark node, find best
 // matching child node for many sync nodes.
 class BookmarkNodeFinder {
  public:
   // Creates an instance with the given parent bookmark node.
   explicit BookmarkNodeFinder(const BookmarkNode* parent_node);
 
   // Finds best matching node for the given sync node.
   // Returns the matching node if one exists; NULL otherwise. If a matching
   // node is found, it's removed for further matches.
-  const BookmarkNode* FindBookmarkNode(const syncer::BaseNode& sync_node);
+  const BookmarkNode* FindBookmarkNode(const GURL& url,
+                                       const std::string& title,
+                                       bool is_folder);
 
  private:
   typedef std::multiset<const BookmarkNode*, BookmarkComparer> BookmarkNodesSet;
 
   const BookmarkNode* parent_node_;
   BookmarkNodesSet child_nodes_;
 
   DISALLOW_COPY_AND_ASSIGN(BookmarkNodeFinder);
 };
 
@@ skipping 15 matching lines @@
 };
 
 BookmarkNodeFinder::BookmarkNodeFinder(const BookmarkNode* parent_node)
     : parent_node_(parent_node) {
   for (int i = 0; i < parent_node_->child_count(); ++i) {
     child_nodes_.insert(parent_node_->GetChild(i));
   }
 }
 
 const BookmarkNode* BookmarkNodeFinder::FindBookmarkNode(
-    const syncer::BaseNode& sync_node) {
-  // Create a bookmark node from the given sync node.
-  BookmarkNode temp_node(GURL(sync_node.GetBookmarkSpecifics().url()));
-  temp_node.SetTitle(UTF8ToUTF16(sync_node.GetTitle()));
-  if (sync_node.GetIsFolder())
+    const GURL& url, const std::string& title, bool is_folder) {
+  // Create a bookmark node from the given bookmark attributes.
+  BookmarkNode temp_node(url);
+  temp_node.SetTitle(UTF8ToUTF16(title));
+  if (is_folder)
     temp_node.set_type(BookmarkNode::FOLDER);
   else
     temp_node.set_type(BookmarkNode::URL);
 
   const BookmarkNode* result = NULL;
   BookmarkNodesSet::iterator iter = child_nodes_.find(&temp_node);
   if (iter != child_nodes_.end()) {
     result = *iter;
     // Remove the matched node so we don't match with it again.
     child_nodes_.erase(iter);
@@ skipping 290 matching lines @@
 
   // WARNING: The order in which we push these should match their order in the
   // bookmark model (see BookmarkModel::DoneLoading(..)).
   std::stack<int64> dfs_stack;
   dfs_stack.push(bookmark_bar_sync_id);
   dfs_stack.push(other_bookmarks_sync_id);
   if (mobile_bookmarks_sync_id != syncer::kInvalidId)
     dfs_stack.push(mobile_bookmarks_sync_id);
 
   syncer::WriteTransaction trans(FROM_HERE, user_share_);
+
+  ApplyDeletesFromSyncJournal(&trans);
+
   syncer::ReadNode bm_root(&trans);
   if (bm_root.InitByTagLookup(syncer::ModelTypeToRootTag(syncer::BOOKMARKS)) ==
       syncer::BaseNode::INIT_OK) {
     syncer_merge_result->set_num_items_before_association(
         bm_root.GetTotalNodeCount());
   }
   local_merge_result->set_num_items_before_association(
       bookmark_model_->root_node()->GetTotalNodeCount());
 
   while (!dfs_stack.empty()) {
@@ skipping 22 matching lines @@
       syncer::WriteNode sync_child_node(&trans);
       if (sync_child_node.InitByIdLookup(sync_child_id) !=
               syncer::BaseNode::INIT_OK) {
         return unrecoverable_error_handler_->CreateAndUploadError(
             FROM_HERE,
             "Failed to lookup node.",
             model_type());
       }
 
       const BookmarkNode* child_node = NULL;
-      child_node = node_finder.FindBookmarkNode(sync_child_node);
+      child_node = node_finder.FindBookmarkNode(
+          GURL(sync_child_node.GetBookmarkSpecifics().url()),
+          sync_child_node.GetTitle(),
+          sync_child_node.GetIsFolder());
       if (child_node)
         Associate(child_node, sync_child_id);
       // All bookmarks are currently modified at association time (even if
       // it doesn't change anything).
       // TODO(sync): introduce logic to only modify the bookmark model if
       // necessary.
       const BookmarkNode* new_child_node =
           BookmarkChangeProcessor::CreateOrUpdateBookmarkNode(
               &sync_child_node,
               bookmark_model_,
@@ skipping 35 matching lines @@
   }
 
   local_merge_result->set_num_items_after_association(
       bookmark_model_->root_node()->GetTotalNodeCount());
   syncer_merge_result->set_num_items_after_association(
       bm_root.GetTotalNodeCount());
 
   return syncer::SyncError();
 }
 
+struct FolderInfo {
+  FolderInfo(const BookmarkNode* f, const BookmarkNode* p, int64 id)
+      : folder(f), parent(p), sync_id(id) {}
+  const BookmarkNode* folder;
+  const BookmarkNode* parent;
+  int64 sync_id;
+};
+typedef std::vector<FolderInfo> FolderInfoList;
+
+int64 BookmarkModelAssociator::ApplyDeletesFromSyncJournal(
+    syncer::BaseTransaction* trans) {
+  int64 num_deleted = 0;
+
+  syncer::SyncDataList deleted_sync_bookmarks;
+  trans->GetDeletedSyncData(syncer::BOOKMARKS, &deleted_sync_bookmarks);

[tim (not reviewing), 2012/12/11 19:58:33]

How can we make it so that this data is *sent* to the glue layer
(ModelAssociators, SyncableService) by the sync engine only when needed? Versus
having to add an instruction to Building a Sync Data type of the form "Don't
forget to look up the delete journal"..  It would be better if this information
was part of / embedded in GetAllSyncData (for example), or even sent via a
separate callback (though I prefer the former). 

+  if (deleted_sync_bookmarks.empty())
+    return 0;
+
+  std::stack<const BookmarkNode*> dfs_stack;
+  dfs_stack.push(bookmark_model_->bookmark_bar_node());
+  dfs_stack.push(bookmark_model_->other_node());
+  if (expect_mobile_bookmarks_folder_)
+    dfs_stack.push(bookmark_model_->mobile_node());
+
+  FolderInfoList folders_deleted;
+  while (!dfs_stack.empty()) {
+    const BookmarkNode* parent = dfs_stack.top();
+    dfs_stack.pop();
+
+    BookmarkNodeFinder finder(parent);
+    syncer::SyncDataList::iterator it = deleted_sync_bookmarks.begin();
+    while (it != deleted_sync_bookmarks.end()) {
+      const BookmarkNode* child = finder.FindBookmarkNode(
+          GURL(it->GetSpecifics().bookmark().url()), it->GetTitle(),
+          it->IsFolder());
+      if (child) {
+        if (child->is_folder()) {
+          // Save folders first and delete only empty ones later.
+          folders_deleted.push_back(FolderInfo(child, parent,
+                                               it->GetRemoteId()));
+        } else {
+          bookmark_model_->Remove(parent, child->GetIndexOf(parent));
+          ++num_deleted;
+        }
+        deleted_sync_bookmarks.erase(it++);
+      } else {
+        ++it;
+      }
+    }
+
+    if (deleted_sync_bookmarks.empty())
+      break;
+  }
+
+  // Ids of sync nodes not found in bookmark model, i.e. deletion is persisted.
+  std::set<int64> confirmed_sync_deletes;
+
+  // Remove empty folders from bottom to top.
+  for (FolderInfoList::const_reverse_iterator it = folders_deleted.rbegin();
+      it != folders_deleted.rend(); ++it) {
+    if (it->folder->child_count() == 0) {
+      bookmark_model_->Remove(it->parent, it->folder->GetIndexOf(it->parent));
+      ++num_deleted;
+    } else {
+      // Keep non-empty folder and remove its journal so that it won't match
+      // again in the future.
+      confirmed_sync_deletes.insert(it->sync_id);
+    }
+  }
+
+  // Consider deletion of unmatched sync nodes to be confirmed.
+  for (size_t i = 0; i < deleted_sync_bookmarks.size(); ++i)
+    confirmed_sync_deletes.insert(deleted_sync_bookmarks[i].GetRemoteId());
+  trans->PurgeDeletedSyncData(confirmed_sync_deletes);
+
+  return num_deleted;
+}
+
 void BookmarkModelAssociator::PostPersistAssociationsTask() {
   // No need to post a task if a task is already pending.
   if (weak_factory_.HasWeakPtrs())
     return;
   MessageLoop::current()->PostTask(
       FROM_HERE,
       base::Bind(
           &BookmarkModelAssociator::PersistAssociations,
           weak_factory_.GetWeakPtr()));
 }
@@ skipping 57 matching lines @@
       UMA_HISTOGRAM_ENUMERATION("Sync.LocalModelOutOfSync",
                                 syncer::BOOKMARKS, syncer::MODEL_TYPE_COUNT);
       // Clear version on bookmark model so that we only report error once.
       bookmark_model_->DeleteNodeMetaInfo(bookmark_model_->root_node(),
                                           kBookmarkTransactionVersionKey);
     }
   }
 }
 
 }  // namespace browser_sync