[Sync] Dont upload an unrecoverable error for missing synced bookmark node

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 313 matching lines @@
   if (bookmark->is_url()) {
     if (bookmark->url() != sync_node->GetURL())
       return false;
   }
   // Don't compare favicons here, because they are not really
   // user-updated and we don't have versioning information -- a site changing
   // its favicon shouldn't result in a bookmark mismatch.
   return true;
 }
 
-syncer::SyncError BookmarkModelAssociator::AssociateTaggedPermanentNode(
+bool BookmarkModelAssociator::AssociateTaggedPermanentNode(
     const BookmarkNode* permanent_node, const std::string&tag) {
   // Do nothing if |permanent_node| is already initialized and associated.
   int64 sync_id = GetSyncIdFromChromeId(permanent_node->id());
   if (sync_id != syncer::kInvalidId)
-    return syncer::SyncError();
+    return true;
   if (!GetSyncIdForTaggedNode(tag, &sync_id))
-    return unrecoverable_error_handler_->CreateAndUploadError(
-        FROM_HERE,
-        "Permanent node not found",
-        model_type());
+    return false;
 
   Associate(permanent_node, sync_id);
-  return syncer::SyncError();
+  return true;
 }
 
 bool BookmarkModelAssociator::GetSyncIdForTaggedNode(const std::string& tag,
                                                      int64* sync_id) {
   syncer::ReadTransaction trans(FROM_HERE, user_share_);
   syncer::ReadNode sync_node(&trans);
   if (sync_node.InitByTagLookup(tag.c_str()) != syncer::BaseNode::INIT_OK)
     return false;
   *sync_id = sync_node.GetId();
   return true;
@@ skipping 25 matching lines @@
   //   corresponding sync node.
   // * When all children sync nodes are done, add the extra children bookmark
   //   nodes to the sync parent node.
   //
   // This algorithm will do a good job of merging when folder names are a good
   // indicator of the two folders being the same. It will handle reordering and
   // new node addition very well (without creating duplicates).
   // This algorithm will not do well if the folder name has changes but the
   // children under them are all the same.
 
   syncer::SyncError error;

[tim (not reviewing), 2012/08/13 20:04:36]

We don't need this anymore.

   DCHECK(bookmark_model_->IsLoaded());
 
   // To prime our association, we associate the top-level nodes, Bookmark Bar
   // and Other Bookmarks.
-  error = AssociateTaggedPermanentNode(bookmark_model_->other_node(),
-                                       kOtherBookmarksTag);
-  if (error.IsSet())
-    return error;
+  if (!AssociateTaggedPermanentNode(bookmark_model_->other_node(),
+                                    kOtherBookmarksTag)) {
+    return unrecoverable_error_handler_->CreateAndUploadError(
+        FROM_HERE,
+        "Other bookmarks node not found",
+        model_type());
+  }
 
-  error = AssociateTaggedPermanentNode(bookmark_model_->bookmark_bar_node(),
-                                       kBookmarkBarTag);
-  if (error.IsSet())
-    return error;
+  if (!AssociateTaggedPermanentNode(bookmark_model_->bookmark_bar_node(),
+                                    kBookmarkBarTag)) {
+    return unrecoverable_error_handler_->CreateAndUploadError(
+        FROM_HERE,
+        "Bookmark bar node not found",
+        model_type());
+  }
 
-  error = AssociateTaggedPermanentNode(bookmark_model_->mobile_node(),
-                                       kMobileBookmarksTag);
-  if (error.IsSet() && expect_mobile_bookmarks_folder_)
-    return error;
+  if (!AssociateTaggedPermanentNode(bookmark_model_->mobile_node(),
+                                    kMobileBookmarksTag) &&
+      expect_mobile_bookmarks_folder_) {
+    return unrecoverable_error_handler_->CreateAndUploadError(
+        FROM_HERE,
+        "Mobile bookmarks node not found",
+        model_type());
+  }
   error = syncer::SyncError();

[tim (not reviewing), 2012/08/13 20:04:36]

Remove

 
   int64 bookmark_bar_sync_id = GetSyncIdFromChromeId(
       bookmark_model_->bookmark_bar_node()->id());
   DCHECK_NE(bookmark_bar_sync_id, syncer::kInvalidId);
   int64 other_bookmarks_sync_id = GetSyncIdFromChromeId(
       bookmark_model_->other_node()->id());
   DCHECK_NE(other_bookmarks_sync_id, syncer::kInvalidId);
   int64 mobile_bookmarks_sync_id = GetSyncIdFromChromeId(
        bookmark_model_->mobile_node()->id());
   if (expect_mobile_bookmarks_folder_) {
@@ skipping 236 matching lines @@
 bool BookmarkModelAssociator::CryptoReadyIfNecessary() {
   // We only access the cryptographer while holding a transaction.
   syncer::ReadTransaction trans(FROM_HERE, user_share_);
   const syncer::ModelTypeSet encrypted_types =
       syncer::GetEncryptedTypes(&trans);
   return !encrypted_types.Has(syncer::BOOKMARKS) ||
       trans.GetCryptographer()->is_ready();
 }
 
 }  // namespace browser_sync