At some point we changed the synchronous model associator methods to not call OnUnrecoverableError on errors and rather just return "false". However, we seem to have regressed and this makes bad stuff happen when there are errors when starting sync.

BUG=40867

Review URL: http://codereview.chromium.org/1558034

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

5 files changed, 16 insertions, 31 deletions

diff --git a/chrome/browser/sync/glue/autofill_model_associator.cc b/chrome/browser/sync/glue/autofill_model_associator.cc
index 019c7b0..ca3ae96 100644
--- a/chrome/browser/sync/glue/autofill_model_associator.cc
+++ b/chrome/browser/sync/glue/autofill_model_associator.cc
@@ -93,7 +93,6 @@ bool AutofillModelAssociator::TraverseAndAssociateChromeAutofillEntries(
       if (!node.InitUniqueByCreation(syncable::AUTOFILL,
                                      autofill_root, tag)) {
         LOG(ERROR) << "Failed to create autofill sync node.";
-        error_handler_->OnUnrecoverableError();
         return false;
       }
       node.SetTitle(UTF16ToWide(ix->key().name() + ix->key().value()));
@@ -129,13 +128,14 @@ bool AutofillModelAssociator::TraverseAndAssociateChromeAutoFillProfiles(
         // We just looked up something we already associated.  Move aside.
         label = MakeUniqueLabel(label, write_trans);
         if (label.empty()) {
-          error_handler_->OnUnrecoverableError();
           return false;
         }
         tag = ProfileLabelToTag(label);
         (*ix)->set_label(label);
-        sync_id = MakeNewAutofillProfileSyncNode(write_trans, autofill_root,
-                                                 tag, **ix);
+        if (!MakeNewAutofillProfileSyncNode(write_trans, autofill_root,
+                                            tag, **ix, &sync_id)) {
+          return false;
+        }
         updated_profiles->push_back(*ix);
       } else {
         // Overwrite local with cloud state.
@@ -146,8 +146,11 @@ bool AutofillModelAssociator::TraverseAndAssociateChromeAutoFillProfiles(
 
       Associate(&tag, sync_id);
     } else {
-      int64 id = MakeNewAutofillProfileSyncNode(write_trans, autofill_root,
-                                                tag, **ix);
+      int64 id;
+      if (!MakeNewAutofillProfileSyncNode(write_trans, autofill_root,
+                                          tag, **ix, &id)) {
+        return false;
+      }
       Associate(&tag, id);
     }
     current_profiles->insert(label);
@@ -174,18 +177,18 @@ string16 AutofillModelAssociator::MakeUniqueLabel(
   return string16();
 }
 
-int64 AutofillModelAssociator::MakeNewAutofillProfileSyncNode(
+bool AutofillModelAssociator::MakeNewAutofillProfileSyncNode(
     sync_api::WriteTransaction* trans, const sync_api::BaseNode& autofill_root,
-    const std::string& tag, const AutoFillProfile& profile) {
+    const std::string& tag, const AutoFillProfile& profile, int64* sync_id) {
   sync_api::WriteNode node(trans);
   if (!node.InitUniqueByCreation(syncable::AUTOFILL, autofill_root, tag)) {
     LOG(ERROR) << "Failed to create autofill sync node.";
-    error_handler_->OnUnrecoverableError();
     return false;
   }
   node.SetTitle(UTF8ToWide(tag));
   AutofillChangeProcessor::WriteAutofillProfile(profile, &node);
-  return node.GetId();
+  *sync_id = node.GetId();
+  return true;
 }
 
 
@@ -221,7 +224,6 @@ bool AutofillModelAssociator::AssociateModels() {
 
     sync_api::ReadNode autofill_root(&trans);
     if (!autofill_root.InitByTagLookup(kAutofillTag)) {
-      error_handler_->OnUnrecoverableError();
       LOG(ERROR) << "Server did not create the top-level autofill node. We "
                  << "might be running against an out-of-date server.";
       return false;
@@ -244,7 +246,6 @@ bool AutofillModelAssociator::AssociateModels() {
   // propogated to the ChangeProcessor on this thread.
   if (!SaveChangesToWebData(bundle)) {
     LOG(ERROR) << "Failed to update autofill entries.";
-    error_handler_->OnUnrecoverableError();
     return false;
   }
 
@@ -283,7 +284,6 @@ bool AutofillModelAssociator::TraverseAndAssociateAllSyncNodes(
     sync_api::ReadNode sync_child(write_trans);
     if (!sync_child.InitByIdLookup(sync_child_id)) {
       LOG(ERROR) << "Failed to fetch child node.";
-      error_handler_->OnUnrecoverableError();
       return false;
     }
     const sync_pb::AutofillSpecifics& autofill(
diff --git a/chrome/browser/sync/glue/autofill_model_associator.h b/chrome/browser/sync/glue/autofill_model_associator.h
index 3ae9703..b1869b9 100644
--- a/chrome/browser/sync/glue/autofill_model_associator.h
+++ b/chrome/browser/sync/glue/autofill_model_associator.h
@@ -182,11 +182,12 @@ class AutofillModelAssociator
   // Helper to insert a sync node for the given AutoFillProfile (e.g. in
   // response to encountering a native profile that doesn't exist yet in the
   // cloud).
-  int64 MakeNewAutofillProfileSyncNode(
+  bool MakeNewAutofillProfileSyncNode(
       sync_api::WriteTransaction* trans,
       const sync_api::BaseNode& autofill_root,
       const std::string& tag,
-      const AutoFillProfile& profile);
+      const AutoFillProfile& profile,
+      int64* sync_id);
 
   ProfileSyncService* sync_service_;
   WebDatabase* web_database_;
diff --git a/chrome/browser/sync/glue/bookmark_model_associator.cc b/chrome/browser/sync/glue/bookmark_model_associator.cc
index ab38942..ce40157 100644
--- a/chrome/browser/sync/glue/bookmark_model_associator.cc
+++ b/chrome/browser/sync/glue/bookmark_model_associator.cc
@@ -340,14 +340,12 @@ bool BookmarkModelAssociator::BuildAssociations() {
   // To prime our association, we associate the top-level nodes, Bookmark Bar
   // and Other Bookmarks.
   if (!AssociateTaggedPermanentNode(model->other_node(), kOtherBookmarksTag)) {
-    error_handler_->OnUnrecoverableError();
     LOG(ERROR) << "Server did not create top-level nodes.  Possibly we "
                << "are running against an out-of-date server?";
     return false;
   }
   if (!AssociateTaggedPermanentNode(model->GetBookmarkBarNode(),
                                     kBookmarkBarTag)) {
-    error_handler_->OnUnrecoverableError();
     LOG(ERROR) << "Server did not create top-level nodes.  Possibly we "
                << "are running against an out-of-date server?";
     return false;
@@ -372,7 +370,6 @@ bool BookmarkModelAssociator::BuildAssociations() {
 
     sync_api::ReadNode sync_parent(&trans);
     if (!sync_parent.InitByIdLookup(sync_parent_id)) {
-      error_handler_->OnUnrecoverableError();
       return false;
     }
     // Only folder nodes are pushed on to the stack.
@@ -388,7 +385,6 @@ bool BookmarkModelAssociator::BuildAssociations() {
     while (sync_child_id != sync_api::kInvalidId) {
       sync_api::WriteNode sync_child_node(&trans);
       if (!sync_child_node.InitByIdLookup(sync_child_id)) {
-        error_handler_->OnUnrecoverableError();
         return false;
       }
 
@@ -486,13 +482,11 @@ bool BookmarkModelAssociator::LoadAssociations() {
   int64 bookmark_bar_id;
   if (!GetSyncIdForTaggedNode(kBookmarkBarTag, &bookmark_bar_id)) {
     // We should always be able to find the permanent nodes.
-    error_handler_->OnUnrecoverableError();
     return false;
   }
   int64 other_bookmarks_id;
   if (!GetSyncIdForTaggedNode(kOtherBookmarksTag, &other_bookmarks_id)) {
     // We should always be able to find the permanent nodes.
-    error_handler_->OnUnrecoverableError();
     return false;
   }
 
@@ -518,7 +512,6 @@ bool BookmarkModelAssociator::LoadAssociations() {
     ++sync_node_count;
     sync_api::ReadNode sync_parent(&trans);
     if (!sync_parent.InitByIdLookup(parent_id)) {
-      error_handler_->OnUnrecoverableError();
       return false;
     }
 
@@ -545,7 +538,6 @@ bool BookmarkModelAssociator::LoadAssociations() {
       dfs_stack.push(child_id);
       sync_api::ReadNode child_node(&trans);
       if (!child_node.InitByIdLookup(child_id)) {
-        error_handler_->OnUnrecoverableError();
         return false;
       }
       child_id = child_node.GetSuccessorId();
diff --git a/chrome/browser/sync/glue/preference_model_associator.cc b/chrome/browser/sync/glue/preference_model_associator.cc
index 5021b4b..0adf0b5 100644
--- a/chrome/browser/sync/glue/preference_model_associator.cc
+++ b/chrome/browser/sync/glue/preference_model_associator.cc
@@ -49,7 +49,6 @@ bool PreferenceModelAssociator::AssociateModels() {
 
   int64 root_id;
   if (!GetSyncIdForTaggedNode(kPreferencesTag, &root_id)) {
-    error_handler_->OnUnrecoverableError();
     LOG(ERROR) << "Server did not create the top-level preferences node. We "
                << "might be running against an out-of-date server.";
     return false;
@@ -59,7 +58,6 @@ bool PreferenceModelAssociator::AssociateModels() {
       sync_service()->backend()->GetUserShareHandle());
   sync_api::ReadNode root(&trans);
   if (!root.InitByIdLookup(root_id)) {
-    error_handler_->OnUnrecoverableError();
     LOG(ERROR) << "Server did not create the top-level preferences node. We "
                << "might be running against an out-of-date server.";
     return false;
@@ -85,7 +83,6 @@ bool PreferenceModelAssociator::AssociateModels() {
       if (!value.get()) {
         LOG(ERROR) << "Failed to deserialize preference value: "
                    << reader.GetErrorMessage();
-        error_handler_->OnUnrecoverableError();
         return false;
       }
 
@@ -96,7 +93,6 @@ bool PreferenceModelAssociator::AssociateModels() {
       sync_api::WriteNode node(&trans);
       if (!node.InitUniqueByCreation(syncable::PREFERENCES, root, tag)) {
         LOG(ERROR) << "Failed to create preference sync node.";
-        error_handler_->OnUnrecoverableError();
         return false;
       }
 
@@ -105,7 +101,6 @@ bool PreferenceModelAssociator::AssociateModels() {
       JSONStringValueSerializer json(&serialized);
       if (!json.Serialize(*(pref->GetValue()))) {
         LOG(ERROR) << "Failed to serialize preference value.";
-        error_handler_->OnUnrecoverableError();
         return false;
       }
       sync_pb::PreferenceSpecifics preference;
diff --git a/chrome/browser/sync/glue/theme_model_associator.cc b/chrome/browser/sync/glue/theme_model_associator.cc
index c4f87ce..4681063 100644
--- a/chrome/browser/sync/glue/theme_model_associator.cc
+++ b/chrome/browser/sync/glue/theme_model_associator.cc
@@ -45,7 +45,6 @@ bool ThemeModelAssociator::AssociateModels() {
   sync_api::ReadNode root(&trans);
   if (!root.InitByTagLookup(kThemesTag)) {
     LOG(ERROR) << kNoThemesFolderError;
-    error_handler_->OnUnrecoverableError();
     return false;
   }
 
@@ -67,7 +66,6 @@ bool ThemeModelAssociator::AssociateModels() {
     if (!node.InitUniqueByCreation(syncable::THEMES, root,
                                    kCurrentThemeClientTag)) {
       LOG(ERROR) << "Could not create current theme node.";
-      error_handler_->OnUnrecoverableError();
       return false;
     }
     node.SetIsFolder(false);
@@ -91,7 +89,6 @@ bool ThemeModelAssociator::SyncModelHasUserCreatedNodes(bool* has_nodes) {
       sync_service_->backend()->GetUserShareHandle());
   sync_api::ReadNode root(&trans);
   if (!root.InitByTagLookup(kThemesTag)) {
-    error_handler_->OnUnrecoverableError();
     LOG(ERROR) << kNoThemesFolderError;
     return false;
   }