Populate versions on individual nodes in sync model and native bookmark model.

chrome/browser/sync/glue/bookmark_change_processor.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_change_processor.h"
 
 #include <stack>
 #include <vector>
 
 #include "base/location.h"
 #include "base/string16.h"
 #include "base/string_number_conversions.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/bookmarks/bookmark_model.h"
 #include "chrome/browser/bookmarks/bookmark_model_factory.h"
 #include "chrome/browser/bookmarks/bookmark_utils.h"
 #include "chrome/browser/favicon/favicon_service.h"
 #include "chrome/browser/favicon/favicon_service_factory.h"
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "content/public/browser/browser_thread.h"
 #include "sync/internal_api/public/change_record.h"
 #include "sync/internal_api/public/read_node.h"
+#include "sync/internal_api/public/read_transaction.h"
 #include "sync/internal_api/public/write_node.h"
 #include "sync/internal_api/public/write_transaction.h"
 #include "sync/syncable/entry.h"  // TODO(tim): Investigating bug 121587.
 #include "ui/gfx/favicon_size.h"
 #include "ui/gfx/image/image_util.h"
 
 using content::BrowserThread;
 
 namespace browser_sync {
 
@@ skipping 70 matching lines @@
   }
   // This node should have no children.
   DCHECK(!sync_node.HasChildren());
   // Remove association and delete the sync node.
   model_associator_->Disassociate(sync_node.GetId());
   sync_node.Remove();
 }
 
 void BookmarkChangeProcessor::RemoveSyncNodeHierarchy(
     const BookmarkNode* topmost) {
-  syncer::WriteTransaction trans(FROM_HERE, share_handle());
+  int64 old_version;

[tim (not reviewing), 2012/11/01 18:31:36]

Should initialize this, if not to zero, to -1 or something.  Could also DCHECK
before calling UpdateTransactionVersion that it was set.

[haitaol1, 2012/11/02 17:57:11]

Done.

+  {
+    syncer::WriteTransaction trans(FROM_HERE, share_handle());
+    old_version = trans.GetModelVersion(syncer::BOOKMARKS);
 
-  // Later logic assumes that |topmost| has been unlinked.
-  DCHECK(topmost->is_root());
+    // Later logic assumes that |topmost| has been unlinked.
+    DCHECK(topmost->is_root());
 
-  // A BookmarkModel deletion event means that |node| and all its children were
-  // deleted. Sync backend expects children to be deleted individually, so we do
-  // a depth-first-search here.  At each step, we consider the |index|-th child
-  // of |node|.  |index_stack| stores index values for the parent levels.
-  std::stack<int> index_stack;
-  index_stack.push(0);  // For the final pop.  It's never used.
-  const BookmarkNode* node = topmost;
-  int index = 0;
-  while (node) {
-    // The top of |index_stack| should always be |node|'s index.
-    DCHECK(node->is_root() || (node->parent()->GetIndexOf(node) ==
-      index_stack.top()));
-    if (index == node->child_count()) {
-      // If we've processed all of |node|'s children, delete |node| and move
-      // on to its successor.
-      RemoveOneSyncNode(&trans, node);
-      node = node->parent();
-      index = index_stack.top() + 1;      // (top() + 0) was what we removed.
-      index_stack.pop();
-    } else {
-      // If |node| has an unprocessed child, process it next after pushing the
-      // current state onto the stack.
-      DCHECK_LT(index, node->child_count());
-      index_stack.push(index);
-      node = node->GetChild(index);
-      index = 0;
+    // A BookmarkModel deletion event means that |node| and all its children
+    // were deleted. Sync backend expects children to be deleted individually,
+    // so we do a depth-first-search here.  At each step, we consider the
+    // |index|-th child of |node|.  |index_stack| stores index values for the
+    // parent levels.
+    std::stack<int> index_stack;
+    index_stack.push(0);  // For the final pop.  It's never used.
+    const BookmarkNode* node = topmost;
+    int index = 0;
+    while (node) {
+      // The top of |index_stack| should always be |node|'s index.
+      DCHECK(node->is_root() || (node->parent()->GetIndexOf(node) ==
+        index_stack.top()));
+      if (index == node->child_count()) {
+        // If we've processed all of |node|'s children, delete |node| and move
+        // on to its successor.
+        RemoveOneSyncNode(&trans, node);
+        node = node->parent();
+        index = index_stack.top() + 1;      // (top() + 0) was what we removed.
+        index_stack.pop();
+      } else {
+        // If |node| has an unprocessed child, process it next after pushing the
+        // current state onto the stack.
+        DCHECK_LT(index, node->child_count());
+        index_stack.push(index);
+        node = node->GetChild(index);
+        index = 0;
+      }
     }
+    DCHECK(index_stack.empty());  // Nothing should be left on the stack.
   }
-  DCHECK(index_stack.empty());  // Nothing should be left on the stack.
+
+  // Don't need to update versions of deleted nodes.
+  UpdateVersion(old_version, bookmark_model_,
+                std::vector<const BookmarkNode*>());
 }
 
 void BookmarkChangeProcessor::Loaded(BookmarkModel* model,
                                      bool ids_reassigned) {
   NOTREACHED();
 }
 
 void BookmarkChangeProcessor::BookmarkModelBeingDeleted(
     BookmarkModel* model) {
   NOTREACHED();
   bookmark_model_ = NULL;
 }
 
 void BookmarkChangeProcessor::BookmarkNodeAdded(BookmarkModel* model,
                                                 const BookmarkNode* parent,
                                                 int index) {
   DCHECK(share_handle());
 
-  // Acquire a scoped write lock via a transaction.
-  syncer::WriteTransaction trans(FROM_HERE, share_handle());
+  int64 old_version;
+  int64 sync_id;

[tim (not reviewing), 2012/11/01 18:31:36]

initialize these two

[haitaol1, 2012/11/02 17:57:11]

Done.

+  {
+    // Acquire a scoped write lock via a transaction.
+    syncer::WriteTransaction trans(FROM_HERE, share_handle());

[tim (not reviewing), 2012/11/01 18:31:36]

Hmm, I see the corner you're in here - you need to get this value from the
transaction, but it is only incremented on destruction, so you don't have an
object to query.  Ideally though, we would not have to open a second transaction
/ mutext to achieve this.  I think it would be cleaner to add a parameter
(perhaps an int64* out param) to the transaction constructor (we could overload
the constructor at least temporarily until we get all data types / callsites
onboard).  On destruction, the transaction would simply set that parameter to
the updated transaction version.

[haitaol1, 2012/11/02 17:57:11]

Done. See changes in write_transaction.cc

+    old_version = trans.GetModelVersion(syncer::BOOKMARKS);
+    sync_id = CreateSyncNode(parent, model, index, &trans,
+                             model_associator_, error_handler());
+  }
 
-  CreateSyncNode(parent, model, index, &trans, model_associator_,
-                 error_handler());
+  if (syncer::kInvalidId != sync_id) {
+    // NOTE(haitaol): Siblings of added node in sync DB will also be updated to
+    //                reflect new PREV_ID/NEXT_ID and thus get a new version.
+    //                But we only update version of added node here. After
+    //                switching to ordinals for positioning, PREV_ID/NEXT_ID
+    //                will be deprecated and siblings will not be updated.
+    UpdateVersion(old_version, model,
+                  std::vector<const BookmarkNode*>(1, parent->GetChild(index)));
+  }
 }
 
 // static
 int64 BookmarkChangeProcessor::CreateSyncNode(const BookmarkNode* parent,
     BookmarkModel* model, int index, syncer::WriteTransaction* trans,
     BookmarkModelAssociator* associator,
     DataTypeErrorHandler* error_handler) {
   const BookmarkNode* child = parent->GetChild(index);
   DCHECK(child);
 
   // Create a WriteNode container to hold the new node.
   syncer::WriteNode sync_child(trans);
 
   // Actually create the node with the appropriate initial position.
   if (!PlaceSyncNode(CREATE, parent, index, trans, &sync_child, associator)) {
     error_handler->OnSingleDatatypeUnrecoverableError(FROM_HERE,
         "Sync node creation failed; recovery unlikely");
     return syncer::kInvalidId;
   }
 
   UpdateSyncNodeProperties(child, model, &sync_child);
 
   // Associate the ID from the sync domain with the bookmark node, so that we
   // can refer back to this item later.
   associator->Associate(child, sync_child.GetId());
 
   return sync_child.GetId();
 }
 
-
 void BookmarkChangeProcessor::BookmarkNodeRemoved(BookmarkModel* model,
-                                                    const BookmarkNode* parent,
-                                                    int index,
-                                                    const BookmarkNode* node) {
+                                                  const BookmarkNode* parent,
+                                                  int index,
+                                                  const BookmarkNode* node) {
   RemoveSyncNodeHierarchy(node);
 }
 
 void BookmarkChangeProcessor::BookmarkNodeChanged(BookmarkModel* model,
                                                   const BookmarkNode* node) {
   // We shouldn't see changes to the top-level nodes.
   if (model->is_permanent_node(node)) {
     NOTREACHED() << "Saw update to permanent node!";
     return;
   }
 
-  // Acquire a scoped write lock via a transaction.
-  syncer::WriteTransaction trans(FROM_HERE, share_handle());
+  int64 old_version;
+  {
+    // Acquire a scoped write lock via a transaction.
+    syncer::WriteTransaction trans(FROM_HERE, share_handle());
+    old_version = trans.GetModelVersion(syncer::BOOKMARKS);
 
-  // Lookup the sync node that's associated with |node|.
-  syncer::WriteNode sync_node(&trans);
-  if (!model_associator_->InitSyncNodeFromChromeId(node->id(), &sync_node)) {
-    // TODO(tim): Investigating bug 121587.
-    if (model_associator_->GetSyncIdFromChromeId(node->id()) ==
-                                                 syncer::kInvalidId) {
-      error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-          "Bookmark id not found in model associator on BookmarkNodeChanged");
-      LOG(ERROR) << "Bad id.";
-    } else if (!sync_node.GetEntry()->good()) {
-      error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-          "Could not InitByIdLookup on BookmarkNodeChanged, good() failed");
-      LOG(ERROR) << "Bad entry.";
-    } else if (sync_node.GetEntry()->Get(syncer::syncable::IS_DEL)) {
-      error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-          "Could not InitByIdLookup on BookmarkNodeChanged, is_del true");
-      LOG(ERROR) << "Deleted entry.";
-    } else {
-      syncer::Cryptographer* crypto = trans.GetCryptographer();
-      syncer::ModelTypeSet encrypted_types(trans.GetEncryptedTypes());
-      const sync_pb::EntitySpecifics& specifics =
-          sync_node.GetEntry()->Get(syncer::syncable::SPECIFICS);
-      CHECK(specifics.has_encrypted());
-      const bool can_decrypt = crypto->CanDecrypt(specifics.encrypted());
-      const bool agreement = encrypted_types.Has(syncer::BOOKMARKS);
-      if (!agreement && !can_decrypt) {
+    // Lookup the sync node that's associated with |node|.
+    syncer::WriteNode sync_node(&trans);
+    if (!model_associator_->InitSyncNodeFromChromeId(node->id(), &sync_node)) {
+      // TODO(tim): Investigating bug 121587.
+      if (model_associator_->GetSyncIdFromChromeId(node->id()) ==
+                                                   syncer::kInvalidId) {
         error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-            "Could not InitByIdLookup on BookmarkNodeChanged, "
-            " Cryptographer thinks bookmarks not encrypted, and CanDecrypt"
-            " failed.");
-        LOG(ERROR) << "Case 1.";
-      } else if (agreement && can_decrypt) {
+            "Bookmark id not found in model associator on BookmarkNodeChanged");
+        LOG(ERROR) << "Bad id.";
+      } else if (!sync_node.GetEntry()->good()) {
         error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-            "Could not InitByIdLookup on BookmarkNodeChanged, "
-            " Cryptographer thinks bookmarks are encrypted, and CanDecrypt"
-            " succeeded (?!), but DecryptIfNecessary failed.");
-        LOG(ERROR) << "Case 2.";
-      } else if (agreement) {
+            "Could not InitByIdLookup on BookmarkNodeChanged, good() failed");
+        LOG(ERROR) << "Bad entry.";
+      } else if (sync_node.GetEntry()->Get(syncer::syncable::IS_DEL)) {
         error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-            "Could not InitByIdLookup on BookmarkNodeChanged, "
-            " Cryptographer thinks bookmarks are encrypted, but CanDecrypt"
-            " failed.");
-        LOG(ERROR) << "Case 3.";
+            "Could not InitByIdLookup on BookmarkNodeChanged, is_del true");
+        LOG(ERROR) << "Deleted entry.";
       } else {
-        error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-            "Could not InitByIdLookup on BookmarkNodeChanged, "
-            " Cryptographer thinks bookmarks not encrypted, but CanDecrypt"
-            " succeeded (super weird, btw)");
-        LOG(ERROR) << "Case 4.";
+        syncer::Cryptographer* crypto = trans.GetCryptographer();
+        syncer::ModelTypeSet encrypted_types(trans.GetEncryptedTypes());
+        const sync_pb::EntitySpecifics& specifics =
+            sync_node.GetEntry()->Get(syncer::syncable::SPECIFICS);
+        CHECK(specifics.has_encrypted());
+        const bool can_decrypt = crypto->CanDecrypt(specifics.encrypted());
+        const bool agreement = encrypted_types.Has(syncer::BOOKMARKS);
+        if (!agreement && !can_decrypt) {
+          error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
+              "Could not InitByIdLookup on BookmarkNodeChanged, "
+              " Cryptographer thinks bookmarks not encrypted, and CanDecrypt"
+              " failed.");
+          LOG(ERROR) << "Case 1.";
+        } else if (agreement && can_decrypt) {
+          error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
+              "Could not InitByIdLookup on BookmarkNodeChanged, "
+              " Cryptographer thinks bookmarks are encrypted, and CanDecrypt"
+              " succeeded (?!), but DecryptIfNecessary failed.");
+          LOG(ERROR) << "Case 2.";
+        } else if (agreement) {
+          error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
+              "Could not InitByIdLookup on BookmarkNodeChanged, "
+              " Cryptographer thinks bookmarks are encrypted, but CanDecrypt"
+              " failed.");
+          LOG(ERROR) << "Case 3.";
+        } else {
+          error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
+              "Could not InitByIdLookup on BookmarkNodeChanged, "
+              " Cryptographer thinks bookmarks not encrypted, but CanDecrypt"
+              " succeeded (super weird, btw)");
+          LOG(ERROR) << "Case 4.";
+        }
       }
+      return;
     }
-    return;
+
+    UpdateSyncNodeProperties(node, model, &sync_node);
+
+    DCHECK_EQ(sync_node.GetIsFolder(), node->is_folder());
+    DCHECK_EQ(model_associator_->GetChromeNodeFromSyncId(
+              sync_node.GetParentId()),
+              node->parent());
+    // This node's index should be one more than the predecessor's index.
+    DCHECK_EQ(node->parent()->GetIndexOf(node),
+              CalculateBookmarkModelInsertionIndex(node->parent(),
+                                                   &sync_node));
   }
 
-  UpdateSyncNodeProperties(node, model, &sync_node);
-
-  DCHECK_EQ(sync_node.GetIsFolder(), node->is_folder());
-  DCHECK_EQ(model_associator_->GetChromeNodeFromSyncId(
-            sync_node.GetParentId()),
-            node->parent());
-  // This node's index should be one more than the predecessor's index.
-  DCHECK_EQ(node->parent()->GetIndexOf(node),
-            CalculateBookmarkModelInsertionIndex(node->parent(),
-                                                 &sync_node));
+  UpdateVersion(old_version, model, std::vector<const BookmarkNode*>(1, node));
 }
 
-
 void BookmarkChangeProcessor::BookmarkNodeMoved(BookmarkModel* model,
       const BookmarkNode* old_parent, int old_index,
       const BookmarkNode* new_parent, int new_index) {
   const BookmarkNode* child = new_parent->GetChild(new_index);
   // We shouldn't see changes to the top-level nodes.
   if (model->is_permanent_node(child)) {
     NOTREACHED() << "Saw update to permanent node!";
     return;
   }
 
-  // Acquire a scoped write lock via a transaction.
-  syncer::WriteTransaction trans(FROM_HERE, share_handle());
+  int64 old_version;
+  {
+    // Acquire a scoped write lock via a transaction.
+    syncer::WriteTransaction trans(FROM_HERE, share_handle());
+    old_version = trans.GetModelVersion(syncer::BOOKMARKS);
 
-  // Lookup the sync node that's associated with |child|.
-  syncer::WriteNode sync_node(&trans);
-  if (!model_associator_->InitSyncNodeFromChromeId(child->id(), &sync_node)) {
-    error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-                                                        std::string());
-    return;
+    // Lookup the sync node that's associated with |child|.
+    syncer::WriteNode sync_node(&trans);
+    if (!model_associator_->InitSyncNodeFromChromeId(child->id(), &sync_node)) {
+      error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
+                                                          std::string());
+      return;
+    }
+
+    if (!PlaceSyncNode(MOVE, new_parent, new_index, &trans, &sync_node,
+                       model_associator_)) {
+      error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
+                                                          std::string());
+      return;
+    }
   }
 
-  if (!PlaceSyncNode(MOVE, new_parent, new_index, &trans, &sync_node,
-                     model_associator_)) {
-    error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-                                                        std::string());
-    return;
-  }
+  UpdateVersion(old_version, model, std::vector<const BookmarkNode*>(1, child));
 }
 
 void BookmarkChangeProcessor::BookmarkNodeFaviconChanged(
     BookmarkModel* model,
     const BookmarkNode* node) {
   BookmarkNodeChanged(model, node);
 }
 
 void BookmarkChangeProcessor::BookmarkNodeChildrenReordered(
     BookmarkModel* model, const BookmarkNode* node) {
+  int64 old_version;
+  std::vector<const BookmarkNode*> children;
+  {
+    // Acquire a scoped write lock via a transaction.
+    syncer::WriteTransaction trans(FROM_HERE, share_handle());
+    old_version = trans.GetModelVersion(syncer::BOOKMARKS);
 
-  // Acquire a scoped write lock via a transaction.
-  syncer::WriteTransaction trans(FROM_HERE, share_handle());
+    // The given node's children got reordered. We need to reorder all the
+    // children of the corresponding sync node.
+    for (int i = 0; i < node->child_count(); ++i) {
+      const BookmarkNode* child = node->GetChild(i);
+      children.push_back(child);
 
-  // The given node's children got reordered. We need to reorder all the
-  // children of the corresponding sync node.
-  for (int i = 0; i < node->child_count(); ++i) {
-    syncer::WriteNode sync_child(&trans);
-    if (!model_associator_->InitSyncNodeFromChromeId(node->GetChild(i)->id(),
-                                                     &sync_child)) {
-      error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-                                                          std::string());
-      return;
-    }
-    DCHECK_EQ(sync_child.GetParentId(),
-              model_associator_->GetSyncIdFromChromeId(node->id()));
+      syncer::WriteNode sync_child(&trans);
+      if (!model_associator_->InitSyncNodeFromChromeId(child->id(),
+                                                       &sync_child)) {
+        error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
+                                                            std::string());
+        return;
+      }
+      DCHECK_EQ(sync_child.GetParentId(),
+                model_associator_->GetSyncIdFromChromeId(node->id()));
 
-    if (!PlaceSyncNode(MOVE, node, i, &trans, &sync_child,
-                       model_associator_)) {
-      error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
-                                                          std::string());
-      return;
+      if (!PlaceSyncNode(MOVE, node, i, &trans, &sync_child,
+                         model_associator_)) {
+        error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
+                                                            std::string());
+        return;
+      }
     }
   }
+
+  // TODO(haitaol): Filter out children that didn't actually change.
+  UpdateVersion(old_version, model, children);
 }
 
 // static
 bool BookmarkChangeProcessor::PlaceSyncNode(MoveOrCreate operation,
       const BookmarkNode* parent, int index, syncer::WriteTransaction* trans,
       syncer::WriteNode* dst, BookmarkModelAssociator* associator) {
   syncer::ReadNode sync_parent(trans);
   if (!associator->InitSyncNodeFromChromeId(parent->id(), &sync_parent)) {
     LOG(WARNING) << "Parent lookup failed";
     return false;
@@ skipping 136 matching lines @@
           << "ACTION_ADD should be seen if and only if the node is unknown.";
       passed_deletes = true;
 
       syncer::ReadNode src(trans);
       if (src.InitByIdLookup(it->id) != syncer::BaseNode::INIT_OK) {
         error_handler()->OnSingleDatatypeUnrecoverableError(FROM_HERE,
             "ApplyModelChanges was passed a bad ID");
         return;
       }
 
-      if (!CreateOrUpdateBookmarkNode(&src, model)) {
+      const BookmarkNode* node = CreateOrUpdateBookmarkNode(&src, model);
+      if (node) {
+        bookmark_model_->SetNodeMetaInfo(node, kBookmarkTransactionVersionKey,
+                                         base::Int64ToString(model_version));
+      } else {
         // Because the Synced Bookmarks node can be created server side, it's
         // possible it'll arrive at the client as an update. In that case it
         // won't have been associated at startup, the GetChromeNodeFromSyncId
         // call above will return NULL, and we won't detect it as a permanent
         // node, resulting in us trying to create it here (which will
         // fail). Therefore, we add special logic here just to detect the
         // Synced Bookmarks folder.
         syncer::ReadNode synced_bookmarks(trans);
         if (synced_bookmarks.InitByTagLookup(kMobileBookmarksTag) ==
                 syncer::BaseNode::INIT_OK &&
@@ skipping 63 matching lines @@
     if (!src->GetIsFolder())
       model->SetURL(dst, src->GetURL());
     model->SetTitle(dst, UTF8ToUTF16(src->GetTitle()));
 
     SetBookmarkFavicon(src, dst, model);
   }
 
   return dst;
 }
 
+void BookmarkChangeProcessor::UpdateVersion(
+    int64 old_version,
+    BookmarkModel* model,
+    const std::vector<const BookmarkNode*>& nodes) {
+  // Open a ReadTransaction to read updated version.
+  syncer::ReadTransaction read_trans(FROM_HERE, share_handle());
+  int64 new_version = read_trans.GetModelVersion(syncer::BOOKMARKS);
+  DCHECK_GE(new_version, old_version);
+  if (new_version > old_version) {
+    model->SetNodeMetaInfo(model->root_node(), kBookmarkTransactionVersionKey,
+                           base::Int64ToString(new_version));
+    for (uint32 i = 0; i < nodes.size(); ++i) {
+      model->SetNodeMetaInfo(nodes[i], kBookmarkTransactionVersionKey,
+                             base::Int64ToString(new_version));
+    }
+  }
+}
+
 // static
 // Creates a bookmark node under the given parent node from the given sync
 // node. Returns the newly created node.
 const BookmarkNode* BookmarkChangeProcessor::CreateBookmarkNode(
     syncer::BaseNode* sync_node,
     const BookmarkNode* parent,
     BookmarkModel* model,
     int index) {
   DCHECK(parent);
   DCHECK(index >= 0 && index <= parent->child_count());
@@ skipping 65 matching lines @@
     const BookmarkNode* bookmark_node,
     BookmarkModel* model,
     syncer::WriteNode* sync_node) {
   std::vector<unsigned char> favicon_bytes;
   EncodeFavicon(bookmark_node, model, &favicon_bytes);
   if (!favicon_bytes.empty())
     sync_node->SetFaviconBytes(favicon_bytes);
 }
 
 }  // namespace browser_sync