I refactored ChangeProcessor so that the common stuff can be reused by other

data types. For ModelAssociator, I just extracted an interface. There's
probably more that can be reused, but I thought we would get to it once we
know more about what kind of associations the other data types will
require. In particular, I didn't use templates because none of the methods that
ProfileSyncService calls on ModelAssociator require a data-type specific type.

I didn't invest too much time refactoring the unit tests, so they're pretty
hacky. I believe the right thing to do would be to test PSS, CP and MA
seperately instead of having a giant PSS test that assumes we only care
about bookmarks.

BUG=29831,29832
TEST=Unit test

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

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

12 files changed, 1058 insertions, 886 deletions

diff --git a/chrome/browser/sync/glue/bookmark_change_processor.cc b/chrome/browser/sync/glue/bookmark_change_processor.cc
new file mode 100644
index 0000000..916669f
--- /dev/null
+++ b/chrome/browser/sync/glue/bookmark_change_processor.cc
@@ -0,0 +1,539 @@
+// Copyright (c) 2006-2009 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 "app/gfx/codec/png_codec.h"
+#include "base/string_util.h"
+#include "chrome/browser/bookmarks/bookmark_utils.h"
+#include "chrome/browser/favicon_service.h"
+#include "chrome/browser/profile.h"
+#include "chrome/browser/sync/profile_sync_service.h"
+#include "third_party/skia/include/core/SkBitmap.h"
+
+namespace browser_sync {
+
+BookmarkChangeProcessor::BookmarkChangeProcessor(
+    UnrecoverableErrorHandler* error_handler)
+    : ChangeProcessor(error_handler),
+      bookmark_model_(NULL),
+      model_associator_(NULL) {
+}
+
+void BookmarkChangeProcessor::StartImpl(Profile* profile) {
+  DCHECK(!bookmark_model_);
+  bookmark_model_ = profile->GetBookmarkModel();
+  DCHECK(bookmark_model_->IsLoaded());
+  bookmark_model_->AddObserver(this);
+}
+
+void BookmarkChangeProcessor::StopImpl() {
+  DCHECK(bookmark_model_);
+  bookmark_model_->RemoveObserver(this);
+  bookmark_model_ = NULL;
+}
+
+void BookmarkChangeProcessor::UpdateSyncNodeProperties(
+    const BookmarkNode* src, BookmarkModel* model, sync_api::WriteNode* dst) {
+  // Set the properties of the item.
+  dst->SetIsFolder(src->is_folder());
+  dst->SetTitle(src->GetTitle());
+  dst->SetURL(src->GetURL());
+  SetSyncNodeFavicon(src, model, dst);
+}
+
+// static
+void BookmarkChangeProcessor::EncodeFavicon(const BookmarkNode* src,
+                                              BookmarkModel* model,
+                                              std::vector<unsigned char>* dst) {
+  const SkBitmap& favicon = model->GetFavIcon(src);
+
+  dst->clear();
+
+  // Check for zero-dimension images.  This can happen if the favicon is
+  // still being loaded.
+  if (favicon.empty())
+    return;
+
+  // Re-encode the BookmarkNode's favicon as a PNG, and pass the data to the
+  // sync subsystem.
+  if (!gfx::PNGCodec::EncodeBGRASkBitmap(favicon, false, dst))
+    return;
+}
+
+void BookmarkChangeProcessor::RemoveOneSyncNode(
+    sync_api::WriteTransaction* trans, const BookmarkNode* node) {
+  sync_api::WriteNode sync_node(trans);
+  if (!model_associator_->InitSyncNodeFromBookmarkId(node->id(), &sync_node)) {
+    error_handler()->OnUnrecoverableError();
+    return;
+  }
+  // This node should have no children.
+  DCHECK(sync_node.GetFirstChildId() == sync_api::kInvalidId);
+  // Remove association and delete the sync node.
+  model_associator_->Disassociate(sync_node.GetId());
+  sync_node.Remove();
+}
+
+void BookmarkChangeProcessor::RemoveSyncNodeHierarchy(
+    const BookmarkNode* topmost) {
+  sync_api::WriteTransaction trans(share_handle());
+
+  // Later logic assumes that |topmost| has been unlinked.
+  DCHECK(!topmost->GetParent());
+
+  // 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->GetParent() || (node->GetParent()->IndexOfChild(node) ==
+      index_stack.top()));
+    if (index == node->GetChildCount()) {
+      // If we've processed all of |node|'s children, delete |node| and move
+      // on to its successor.
+      RemoveOneSyncNode(&trans, node);
+      node = node->GetParent();
+      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->GetChildCount());
+      index_stack.push(index);
+      node = node->GetChild(index);
+      index = 0;
+    }
+  }
+  DCHECK(index_stack.empty());  // Nothing should be left on the stack.
+}
+
+void BookmarkChangeProcessor::BookmarkModelBeingDeleted(
+    BookmarkModel* model) {
+  DCHECK(!running()) << "BookmarkModel deleted while ChangeProcessor running.";
+  bookmark_model_ = NULL;
+}
+
+void BookmarkChangeProcessor::BookmarkNodeAdded(BookmarkModel* model,
+                                                  const BookmarkNode* parent,
+                                                  int index) {
+  DCHECK(running());
+  DCHECK(share_handle());
+
+  // Acquire a scoped write lock via a transaction.
+  sync_api::WriteTransaction trans(share_handle());
+
+  CreateSyncNode(parent, model, index, &trans, model_associator_.get(),
+                 error_handler());
+}
+
+// static
+int64 BookmarkChangeProcessor::CreateSyncNode(const BookmarkNode* parent,
+    BookmarkModel* model, int index, sync_api::WriteTransaction* trans,
+    BookmarkModelAssociator* associator,
+    UnrecoverableErrorHandler* error_handler) {
+  const BookmarkNode* child = parent->GetChild(index);
+  DCHECK(child);
+
+  // Create a WriteNode container to hold the new node.
+  sync_api::WriteNode sync_child(trans);
+
+  // Actually create the node with the appropriate initial position.
+  if (!PlaceSyncNode(CREATE, parent, index, trans, &sync_child, associator,
+                     error_handler)) {
+    LOG(WARNING) << "Sync node creation failed; recovery unlikely";
+    error_handler->OnUnrecoverableError();
+    return sync_api::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) {
+  DCHECK(running());
+  RemoveSyncNodeHierarchy(node);
+}
+
+void BookmarkChangeProcessor::BookmarkNodeChanged(BookmarkModel* model,
+                                                    const BookmarkNode* node) {
+  DCHECK(running());
+  // We shouldn't see changes to the top-level nodes.
+  if (node == model->GetBookmarkBarNode() || node == model->other_node()) {
+    NOTREACHED() << "Saw update to permanent node!";
+    return;
+  }
+
+  // Acquire a scoped write lock via a transaction.
+  sync_api::WriteTransaction trans(share_handle());
+
+  // Lookup the sync node that's associated with |node|.
+  sync_api::WriteNode sync_node(&trans);
+  if (!model_associator_->InitSyncNodeFromBookmarkId(node->id(), &sync_node)) {
+    error_handler()->OnUnrecoverableError();
+    return;
+  }
+
+  UpdateSyncNodeProperties(node, model, &sync_node);
+
+  DCHECK_EQ(sync_node.GetIsFolder(), node->is_folder());
+  DCHECK_EQ(model_associator_->GetBookmarkNodeFromSyncId(
+            sync_node.GetParentId()),
+            node->GetParent());
+  // This node's index should be one more than the predecessor's index.
+  DCHECK_EQ(node->GetParent()->IndexOfChild(node),
+            CalculateBookmarkModelInsertionIndex(node->GetParent(),
+                                                 &sync_node));
+}
+
+
+void BookmarkChangeProcessor::BookmarkNodeMoved(BookmarkModel* model,
+      const BookmarkNode* old_parent, int old_index,
+      const BookmarkNode* new_parent, int new_index) {
+  DCHECK(running());
+  const BookmarkNode* child = new_parent->GetChild(new_index);
+  // We shouldn't see changes to the top-level nodes.
+  if (child == model->GetBookmarkBarNode() || child == model->other_node()) {
+    NOTREACHED() << "Saw update to permanent node!";
+    return;
+  }
+
+  // Acquire a scoped write lock via a transaction.
+  sync_api::WriteTransaction trans(share_handle());
+
+  // Lookup the sync node that's associated with |child|.
+  sync_api::WriteNode sync_node(&trans);
+  if (!model_associator_->InitSyncNodeFromBookmarkId(child->id(), &sync_node)) {
+    error_handler()->OnUnrecoverableError();
+    return;
+  }
+
+  if (!PlaceSyncNode(MOVE, new_parent, new_index, &trans, &sync_node,
+                     model_associator_.get(), error_handler())) {
+    error_handler()->OnUnrecoverableError();
+    return;
+  }
+}
+
+void BookmarkChangeProcessor::BookmarkNodeFavIconLoaded(BookmarkModel* model,
+      const BookmarkNode* node) {
+  DCHECK(running());
+  BookmarkNodeChanged(model, node);
+}
+
+void BookmarkChangeProcessor::BookmarkNodeChildrenReordered(
+    BookmarkModel* model, const BookmarkNode* node) {
+
+  // Acquire a scoped write lock via a transaction.
+  sync_api::WriteTransaction trans(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->GetChildCount(); ++i) {
+    sync_api::WriteNode sync_child(&trans);
+    if (!model_associator_->InitSyncNodeFromBookmarkId(node->GetChild(i)->id(),
+                                                       &sync_child)) {
+      error_handler()->OnUnrecoverableError();
+      return;
+    }
+    DCHECK_EQ(sync_child.GetParentId(),
+              model_associator_->GetSyncIdFromBookmarkId(node->id()));
+
+    if (!PlaceSyncNode(MOVE, node, i, &trans, &sync_child,
+                       model_associator_.get(), error_handler())) {
+      error_handler()->OnUnrecoverableError();
+      return;
+    }
+  }
+}
+
+// static
+bool BookmarkChangeProcessor::PlaceSyncNode(MoveOrCreate operation,
+      const BookmarkNode* parent, int index, sync_api::WriteTransaction* trans,
+      sync_api::WriteNode* dst, BookmarkModelAssociator* associator,
+      UnrecoverableErrorHandler* error_handler) {
+  sync_api::ReadNode sync_parent(trans);
+  if (!associator->InitSyncNodeFromBookmarkId(parent->id(), &sync_parent)) {
+    LOG(WARNING) << "Parent lookup failed";
+    error_handler->OnUnrecoverableError();
+    return false;
+  }
+
+  bool success = false;
+  if (index == 0) {
+    // Insert into first position.
+    success = (operation == CREATE) ? dst->InitByCreation(sync_parent, NULL) :
+                                      dst->SetPosition(sync_parent, NULL);
+    if (success) {
+      DCHECK_EQ(dst->GetParentId(), sync_parent.GetId());
+      DCHECK_EQ(dst->GetId(), sync_parent.GetFirstChildId());
+      DCHECK_EQ(dst->GetPredecessorId(), sync_api::kInvalidId);
+    }
+  } else {
+    // Find the bookmark model predecessor, and insert after it.
+    const BookmarkNode* prev = parent->GetChild(index - 1);
+    sync_api::ReadNode sync_prev(trans);
+    if (!associator->InitSyncNodeFromBookmarkId(prev->id(), &sync_prev)) {
+      LOG(WARNING) << "Predecessor lookup failed";
+      return false;
+    }
+    success = (operation == CREATE) ?
+        dst->InitByCreation(sync_parent, &sync_prev) :
+        dst->SetPosition(sync_parent, &sync_prev);
+    if (success) {
+      DCHECK_EQ(dst->GetParentId(), sync_parent.GetId());
+      DCHECK_EQ(dst->GetPredecessorId(), sync_prev.GetId());
+      DCHECK_EQ(dst->GetId(), sync_prev.GetSuccessorId());
+    }
+  }
+  return success;
+}
+
+// Determine the bookmark model index to which a node must be moved so that
+// predecessor of the node (in the bookmark model) matches the predecessor of
+// |source| (in the sync model).
+// As a precondition, this assumes that the predecessor of |source| has been
+// updated and is already in the correct position in the bookmark model.
+int BookmarkChangeProcessor::CalculateBookmarkModelInsertionIndex(
+    const BookmarkNode* parent,
+    const sync_api::BaseNode* child_info) const {
+  DCHECK(parent);
+  DCHECK(child_info);
+  int64 predecessor_id = child_info->GetPredecessorId();
+  // A return ID of kInvalidId indicates no predecessor.
+  if (predecessor_id == sync_api::kInvalidId)
+    return 0;
+
+  // Otherwise, insert after the predecessor bookmark node.
+  const BookmarkNode* predecessor =
+      model_associator_->GetBookmarkNodeFromSyncId(predecessor_id);
+  DCHECK(predecessor);
+  DCHECK_EQ(predecessor->GetParent(), parent);
+  return parent->IndexOfChild(predecessor) + 1;
+}
+
+// ApplyModelChanges is called by the sync backend after changes have been made
+// to the sync engine's model.  Apply these changes to the browser bookmark
+// model.
+void BookmarkChangeProcessor::ApplyChangesFromSyncModel(
+    const sync_api::BaseTransaction* trans,
+    const sync_api::SyncManager::ChangeRecord* changes,
+    int change_count) {
+  if (!running())
+    return;
+  // A note about ordering.  Sync backend is responsible for ordering the change
+  // records in the following order:
+  //
+  // 1. Deletions, from leaves up to parents.
+  // 2. Existing items with synced parents & predecessors.
+  // 3. New items with synced parents & predecessors.
+  // 4. Items with parents & predecessors in the list.
+  // 5. Repeat #4 until all items are in the list.
+  //
+  // "Predecessor" here means the previous item within a given folder; an item
+  // in the first position is always said to have a synced predecessor.
+  // For the most part, applying these changes in the order given will yield
+  // the correct result.  There is one exception, however: for items that are
+  // moved away from a folder that is being deleted, we will process the delete
+  // before the move.  Since deletions in the bookmark model propagate from
+  // parent to child, we must move them to a temporary location.
+  BookmarkModel* model = bookmark_model_;
+
+  // We are going to make changes to the bookmarks model, but don't want to end
+  // up in a feedback loop, so remove ourselves as an observer while applying
+  // changes.
+  model->RemoveObserver(this);
+
+  // A parent to hold nodes temporarily orphaned by parent deletion.  It is
+  // lazily created inside the loop.
+  const BookmarkNode* foster_parent = NULL;
+  for (int i = 0; i < change_count; ++i) {
+    const BookmarkNode* dst =
+        model_associator_->GetBookmarkNodeFromSyncId(changes[i].id);
+    // Ignore changes to the permanent top-level nodes.  We only care about
+    // their children.
+    if ((dst == model->GetBookmarkBarNode()) || (dst == model->other_node()))
+      continue;
+    if (changes[i].action ==
+        sync_api::SyncManager::ChangeRecord::ACTION_DELETE) {
+      // Deletions should always be at the front of the list.
+      DCHECK(i == 0 || changes[i-1].action == changes[i].action);
+      // Children of a deleted node should not be deleted; they may be
+      // reparented by a later change record.  Move them to a temporary place.
+      DCHECK(dst) << "Could not find node to be deleted";
+      const BookmarkNode* parent = dst->GetParent();
+      if (dst->GetChildCount()) {
+        if (!foster_parent) {
+          foster_parent = model->AddGroup(model->other_node(),
+                                          model->other_node()->GetChildCount(),
+                                          std::wstring());
+        }
+        for (int i = dst->GetChildCount() - 1; i >= 0; --i) {
+          model->Move(dst->GetChild(i), foster_parent,
+                      foster_parent->GetChildCount());
+        }
+      }
+      DCHECK_EQ(dst->GetChildCount(), 0) << "Node being deleted has children";
+      model_associator_->Disassociate(changes[i].id);
+      model->Remove(parent, parent->IndexOfChild(dst));
+      dst = NULL;
+    } else {
+      DCHECK_EQ((changes[i].action ==
+          sync_api::SyncManager::ChangeRecord::ACTION_ADD), (dst == NULL))
+          << "ACTION_ADD should be seen if and only if the node is unknown.";
+
+      sync_api::ReadNode src(trans);
+      if (!src.InitByIdLookup(changes[i].id)) {
+        LOG(ERROR) << "ApplyModelChanges was passed a bad ID";
+        error_handler()->OnUnrecoverableError();
+        return;
+      }
+
+      CreateOrUpdateBookmarkNode(&src, model);
+    }
+  }
+  // Clean up the temporary node.
+  if (foster_parent) {
+    // There should be no nodes left under the foster parent.
+    DCHECK_EQ(foster_parent->GetChildCount(), 0);
+    model->Remove(foster_parent->GetParent(),
+                  foster_parent->GetParent()->IndexOfChild(foster_parent));
+    foster_parent = NULL;
+  }
+
+  // We are now ready to hear about bookmarks changes again.
+  model->AddObserver(this);
+}
+
+// Create a bookmark node corresponding to |src| if one is not already
+// associated with |src|.
+const BookmarkNode* BookmarkChangeProcessor::CreateOrUpdateBookmarkNode(
+    sync_api::BaseNode* src,
+    BookmarkModel* model) {
+  const BookmarkNode* parent =
+      model_associator_->GetBookmarkNodeFromSyncId(src->GetParentId());
+  if (!parent) {
+    DLOG(WARNING) << "Could not find parent of node being added/updated."
+      << " Node title: " << src->GetTitle()
+      << ", parent id = " << src->GetParentId();
+    return NULL;
+  }
+  int index = CalculateBookmarkModelInsertionIndex(parent, src);
+  const BookmarkNode* dst = model_associator_->GetBookmarkNodeFromSyncId(
+      src->GetId());
+  if (!dst) {
+    dst = CreateBookmarkNode(src, parent, model, index);
+    model_associator_->Associate(dst, src->GetId());
+  } else {
+    // URL and is_folder are not expected to change.
+    // TODO(ncarter): Determine if such changes should be legal or not.
+    DCHECK_EQ(src->GetIsFolder(), dst->is_folder());
+
+    // Handle reparenting and/or repositioning.
+    model->Move(dst, parent, index);
+
+    // Handle title update and URL changes due to possible conflict resolution
+    // that can happen if both a local user change and server change occur
+    // within a sufficiently small time interval.
+    const BookmarkNode* old_dst = dst;
+    dst = bookmark_utils::ApplyEditsWithNoGroupChange(model, parent,
+        BookmarkEditor::EditDetails(dst),
+        src->GetTitle(),
+        src->GetIsFolder() ? GURL() : src->GetURL(),
+        NULL);  // NULL because we don't need a BookmarkEditor::Handler.
+    if (dst != old_dst) {  // dst was replaced with a new node with new URL.
+      model_associator_->Disassociate(src->GetId());
+      model_associator_->Associate(dst, src->GetId());
+    }
+    SetBookmarkFavicon(src, dst, model->profile());
+  }
+
+  return dst;
+}
+
+// static
+// Creates a bookmark node under the given parent node from the given sync
+// node. Returns the newly created node.
+const BookmarkNode* BookmarkChangeProcessor::CreateBookmarkNode(
+    sync_api::BaseNode* sync_node,
+    const BookmarkNode* parent,
+    BookmarkModel* model,
+    int index) {
+  DCHECK(parent);
+  DCHECK(index >= 0 && index <= parent->GetChildCount());
+
+  const BookmarkNode* node;
+  if (sync_node->GetIsFolder()) {
+    node = model->AddGroup(parent, index, sync_node->GetTitle());
+  } else {
+    node = model->AddURL(parent, index,
+                         sync_node->GetTitle(), sync_node->GetURL());
+    SetBookmarkFavicon(sync_node, node, model->profile());
+  }
+  return node;
+}
+
+// static
+// Sets the favicon of the given bookmark node from the given sync node.
+bool BookmarkChangeProcessor::SetBookmarkFavicon(
+    sync_api::BaseNode* sync_node,
+    const BookmarkNode* bookmark_node,
+    Profile* profile) {
+  size_t icon_size = 0;
+  const unsigned char* icon_bytes = sync_node->GetFaviconBytes(&icon_size);
+  if (!icon_size || !icon_bytes)
+    return false;
+
+  // Registering a favicon requires that we provide a source URL, but we
+  // don't know where these came from.  Currently we just use the
+  // destination URL, which is not correct, but since the favicon URL
+  // is used as a key in the history's thumbnail DB, this gives us a value
+  // which does not collide with others.
+  GURL fake_icon_url = bookmark_node->GetURL();
+
+  std::vector<unsigned char> icon_bytes_vector(icon_bytes,
+                                               icon_bytes + icon_size);
+
+  HistoryService* history =
+      profile->GetHistoryService(Profile::EXPLICIT_ACCESS);
+  FaviconService* favicon_service =
+      profile->GetFaviconService(Profile::EXPLICIT_ACCESS);
+
+  history->AddPage(bookmark_node->GetURL());
+  favicon_service->SetFavicon(bookmark_node->GetURL(),
+                              fake_icon_url,
+                              icon_bytes_vector);
+
+  return true;
+}
+
+// static
+void BookmarkChangeProcessor::SetSyncNodeFavicon(
+    const BookmarkNode* bookmark_node,
+    BookmarkModel* model,
+    sync_api::WriteNode* sync_node) {
+  std::vector<unsigned char> favicon_bytes;
+  EncodeFavicon(bookmark_node, model, &favicon_bytes);
+  if (!favicon_bytes.empty())
+    sync_node->SetFaviconBytes(&favicon_bytes[0], favicon_bytes.size());
+}
+
+}  // namespace browser_sync
diff --git a/chrome/browser/sync/glue/bookmark_change_processor.h b/chrome/browser/sync/glue/bookmark_change_processor.h
new file mode 100644
index 0000000..496b85f
--- /dev/null
+++ b/chrome/browser/sync/glue/bookmark_change_processor.h
@@ -0,0 +1,172 @@
+// Copyright (c) 2006-2009 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.
+
+#ifndef CHROME_BROWSER_SYNC_GLUE_BOOKMARK_CHANGE_PROCESSOR_H_
+#define CHROME_BROWSER_SYNC_GLUE_BOOKMARK_CHANGE_PROCESSOR_H_
+
+#include <vector>
+
+#include "base/scoped_ptr.h"
+#include "chrome/browser/bookmarks/bookmark_model.h"
+#include "chrome/browser/sync/engine/syncapi.h"
+#include "chrome/browser/sync/glue/change_processor.h"
+#include "chrome/browser/sync/glue/bookmark_model_associator.h"
+#include "chrome/browser/sync/glue/sync_backend_host.h"
+
+class ProfileSyncService;
+
+namespace browser_sync {
+
+// This class is responsible for taking changes from the BookmarkModel
+// and applying them to the sync_api 'syncable' model, and vice versa.
+// All operations and use of this class are from the UI thread.
+// This is currently bookmarks specific.
+class BookmarkChangeProcessor : public BookmarkModelObserver,
+                                 public ChangeProcessor {
+ public:
+  explicit BookmarkChangeProcessor(UnrecoverableErrorHandler* error_handler);
+  virtual ~BookmarkChangeProcessor() {}
+
+  void set_model_associator(BookmarkModelAssociator* model_associator) {
+    model_associator_.reset(model_associator);
+  }
+  virtual ModelAssociator* GetModelAssociator() {
+    return model_associator_.get();
+  }
+
+  // BookmarkModelObserver implementation.
+  // BookmarkModel -> sync_api model change application.
+  virtual void Loaded(BookmarkModel* model) { NOTREACHED(); }
+  virtual void BookmarkModelBeingDeleted(BookmarkModel* model);
+  virtual void BookmarkNodeMoved(BookmarkModel* model,
+                                 const BookmarkNode* old_parent,
+                                 int old_index,
+                                 const BookmarkNode* new_parent,
+                                 int new_index);
+  virtual void BookmarkNodeAdded(BookmarkModel* model,
+                                 const BookmarkNode* parent,
+                                 int index);
+  virtual void BookmarkNodeRemoved(BookmarkModel* model,
+                                   const BookmarkNode* parent,
+                                   int index,
+                                   const BookmarkNode* node);
+  virtual void BookmarkNodeChanged(BookmarkModel* model,
+                                   const BookmarkNode* node);
+  virtual void BookmarkNodeFavIconLoaded(BookmarkModel* model,
+                                         const BookmarkNode* node);
+  virtual void BookmarkNodeChildrenReordered(BookmarkModel* model,
+                                             const BookmarkNode* node);
+
+  // The change processor implementation, responsible for applying changes from
+  // the sync model to the bookmarks model.
+  virtual void ApplyChangesFromSyncModel(
+      const sync_api::BaseTransaction* trans,
+      const sync_api::SyncManager::ChangeRecord* changes,
+      int change_count);
+
+  // The following methods are static and hence may be invoked at any time,
+  // and do not depend on having a running ChangeProcessor.
+  // Creates a bookmark node under the given parent node from the given sync
+  // node. Returns the newly created node.
+  static const BookmarkNode* CreateBookmarkNode(
+      sync_api::BaseNode* sync_node,
+      const BookmarkNode* parent,
+      BookmarkModel* model,
+      int index);
+
+  // Sets the favicon of the given bookmark node from the given sync node.
+  // Returns whether the favicon was set in the bookmark node.
+  // |profile| is the profile that contains the HistoryService and BookmarkModel
+  // for the bookmark in question.
+  static bool SetBookmarkFavicon(sync_api::BaseNode* sync_node,
+                                 const BookmarkNode* bookmark_node,
+                                 Profile* profile);
+
+  // Sets the favicon of the given sync node from the given bookmark node.
+  static void SetSyncNodeFavicon(const BookmarkNode* bookmark_node,
+                                 BookmarkModel* model,
+                                 sync_api::WriteNode* sync_node);
+
+  // Treat the |index|th child of |parent| as a newly added node, and create a
+  // corresponding node in the sync domain using |trans|.  All properties
+  // will be transferred to the new node.  A node corresponding to |parent|
+  // must already exist and be associated for this call to succeed.  Returns
+  // the ID of the just-created node, or if creation fails, kInvalidID.
+  static int64 CreateSyncNode(const BookmarkNode* parent,
+                              BookmarkModel* model,
+                              int index,
+                              sync_api::WriteTransaction* trans,
+                              BookmarkModelAssociator* associator,
+                              UnrecoverableErrorHandler* error_handler);
+
+ protected:
+  virtual void StartImpl(Profile* profile);
+  virtual void StopImpl();
+
+ private:
+  enum MoveOrCreate {
+    MOVE,
+    CREATE,
+  };
+
+  // Create a bookmark node corresponding to |src| if one is not already
+  // associated with |src|.  Returns the node that was created or updated.
+  const BookmarkNode* CreateOrUpdateBookmarkNode(
+      sync_api::BaseNode* src,
+      BookmarkModel* model);
+
+  // Helper function to determine the appropriate insertion index of sync node
+  // |node| under the Bookmark model node |parent|, to make the positions
+  // match up between the two models. This presumes that the predecessor of the
+  // item (in the bookmark model) has already been moved into its appropriate
+  // position.
+  int CalculateBookmarkModelInsertionIndex(
+      const BookmarkNode* parent,
+      const sync_api::BaseNode* node) const;
+
+  // Helper function used to fix the position of a sync node so that it matches
+  // the position of a corresponding bookmark model node. |parent| and
+  // |index| identify the bookmark model position.  |dst| is the node whose
+  // position is to be fixed.  If |operation| is CREATE, treat |dst| as an
+  // uncreated node and set its position via InitByCreation(); otherwise,
+  // |dst| is treated as an existing node, and its position will be set via
+  // SetPosition().  |trans| is the transaction to which |dst| belongs. Returns
+  // false on failure.
+  static bool PlaceSyncNode(MoveOrCreate operation,
+                            const BookmarkNode* parent,
+                            int index,
+                            sync_api::WriteTransaction* trans,
+                            sync_api::WriteNode* dst,
+                            BookmarkModelAssociator* associator,
+                            UnrecoverableErrorHandler* error_handler);
+
+  // Copy properties (but not position) from |src| to |dst|.
+  static void UpdateSyncNodeProperties(const BookmarkNode* src,
+                                       BookmarkModel* model,
+                                       sync_api::WriteNode* dst);
+
+  // Helper function to encode a bookmark's favicon into a PNG byte vector.
+  static void EncodeFavicon(const BookmarkNode* src,
+                            BookmarkModel* model,
+                            std::vector<unsigned char>* dst);
+
+  // Remove the sync node corresponding to |node|.  It shouldn't have
+  // any children.
+  void RemoveOneSyncNode(sync_api::WriteTransaction* trans,
+                         const BookmarkNode* node);
+
+  // Remove all the sync nodes associated with |node| and its children.
+  void RemoveSyncNodeHierarchy(const BookmarkNode* node);
+
+  // The bookmark model we are processing changes from.  Non-NULL when
+  // |running_| is true.
+  BookmarkModel* bookmark_model_;
+  scoped_ptr<BookmarkModelAssociator> model_associator_;
+
+  DISALLOW_COPY_AND_ASSIGN(BookmarkChangeProcessor);
+};
+
+}  // namespace browser_sync
+
+#endif  // CHROME_BROWSER_SYNC_GLUE_BOOKMARK_CHANGE_PROCESSOR_H_
diff --git a/chrome/browser/sync/glue/model_associator.cc b/chrome/browser/sync/glue/bookmark_model_associator.cc
index 1754504..7e220bf 100644
--- a/chrome/browser/sync/glue/model_associator.cc
+++ b/chrome/browser/sync/glue/bookmark_model_associator.cc
@@ -2,7 +2,7 @@
 // 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/model_associator.h"
+#include "chrome/browser/sync/glue/bookmark_model_associator.h"
 
 #include <stack>
 
@@ -11,6 +11,7 @@
 #include "base/task.h"
 #include "chrome/browser/bookmarks/bookmark_model.h"
 #include "chrome/browser/sync/engine/syncapi.h"
+#include "chrome/browser/sync/glue/bookmark_change_processor.h"
 #include "chrome/browser/sync/profile_sync_service.h"
 
 namespace browser_sync {
@@ -150,29 +151,32 @@ const BookmarkNode* BookmarkNodeIdIndex::Find(int64 id) const {
   return iter == node_index_.end() ? NULL : iter->second;
 }
 
-ModelAssociator::ModelAssociator(ProfileSyncService* sync_service)
+BookmarkModelAssociator::BookmarkModelAssociator(
+    ProfileSyncService* sync_service)
     : sync_service_(sync_service),
-      task_pending_(false) {
+      ALLOW_THIS_IN_INITIALIZER_LIST(persist_associations_(this)) {
   DCHECK(sync_service_);
 }
 
-void ModelAssociator::ClearAll() {
+bool BookmarkModelAssociator::DisassociateModels() {
   id_map_.clear();
   id_map_inverse_.clear();
   dirty_associations_sync_ids_.clear();
+  return true;
 }
 
-int64 ModelAssociator::GetSyncIdFromBookmarkId(int64 node_id) const {
+int64 BookmarkModelAssociator::GetSyncIdFromBookmarkId(int64 node_id) const {
   BookmarkIdToSyncIdMap::const_iterator iter = id_map_.find(node_id);
   return iter == id_map_.end() ? sync_api::kInvalidId : iter->second;
 }
 
-const BookmarkNode* ModelAssociator::GetBookmarkNodeFromSyncId(int64 sync_id) {
+const BookmarkNode* BookmarkModelAssociator::GetBookmarkNodeFromSyncId(
+    int64 sync_id) {
   SyncIdToBookmarkNodeMap::const_iterator iter = id_map_inverse_.find(sync_id);
   return iter == id_map_inverse_.end() ? NULL : iter->second;
 }
 
-bool ModelAssociator::InitSyncNodeFromBookmarkId(
+bool BookmarkModelAssociator::InitSyncNodeFromBookmarkId(
     int64 node_id,
     sync_api::BaseNode* sync_node) {
   DCHECK(sync_node);
@@ -185,7 +189,8 @@ bool ModelAssociator::InitSyncNodeFromBookmarkId(
   return true;
 }
 
-void ModelAssociator::Associate(const BookmarkNode* node, int64 sync_id) {
+void BookmarkModelAssociator::Associate(const BookmarkNode* node,
+                                        int64 sync_id) {
   int64 node_id = node->id();
   DCHECK_NE(sync_id, sync_api::kInvalidId);
   DCHECK(id_map_.find(node_id) == id_map_.end());
@@ -196,7 +201,7 @@ void ModelAssociator::Associate(const BookmarkNode* node, int64 sync_id) {
   PostPersistAssociationsTask();
 }
 
-void ModelAssociator::Disassociate(int64 sync_id) {
+void BookmarkModelAssociator::Disassociate(int64 sync_id) {
   SyncIdToBookmarkNodeMap::iterator iter = id_map_inverse_.find(sync_id);
   if (iter == id_map_inverse_.end())
     return;
@@ -205,16 +210,16 @@ void ModelAssociator::Disassociate(int64 sync_id) {
   dirty_associations_sync_ids_.erase(sync_id);
 }
 
-bool ModelAssociator::BookmarkModelHasUserCreatedNodes() const {
+bool BookmarkModelAssociator::ChromeModelHasUserCreatedNodes() {
   BookmarkModel* model = sync_service_->profile()->GetBookmarkModel();
   DCHECK(model->IsLoaded());
   return model->GetBookmarkBarNode()->GetChildCount() > 0 ||
          model->other_node()->GetChildCount() > 0;
 }
 
-bool ModelAssociator::SyncModelHasUserCreatedNodes() {
+bool BookmarkModelAssociator::SyncModelHasUserCreatedNodes() {
   int64 bookmark_bar_sync_id;
-  if (!GetSyncIdForTaggedNode(kBookmarkBarTag,&bookmark_bar_sync_id)) {
+  if (!GetSyncIdForTaggedNode(kBookmarkBarTag, &bookmark_bar_sync_id)) {
     sync_service_->OnUnrecoverableError();
     return false;
   }
@@ -245,8 +250,9 @@ bool ModelAssociator::SyncModelHasUserCreatedNodes() {
          other_bookmarks_node.GetFirstChildId() != sync_api::kInvalidId;
 }
 
-bool ModelAssociator::NodesMatch(const BookmarkNode* bookmark,
-                                 const sync_api::BaseNode* sync_node) const {
+bool BookmarkModelAssociator::NodesMatch(const BookmarkNode* bookmark,
+                                          const sync_api::BaseNode* sync_node)
+    const {
   if (bookmark->GetTitle() != sync_node->GetTitle())
     return false;
   if (bookmark->is_folder() != sync_node->GetIsFolder())
@@ -261,7 +267,7 @@ bool ModelAssociator::NodesMatch(const BookmarkNode* bookmark,
   return true;
 }
 
-bool ModelAssociator::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 = GetSyncIdFromBookmarkId(permanent_node->id());
@@ -274,8 +280,8 @@ bool ModelAssociator::AssociateTaggedPermanentNode(
   return true;
 }
 
-bool ModelAssociator::GetSyncIdForTaggedNode(const std::string& tag,
-                                             int64* sync_id) {
+bool BookmarkModelAssociator::GetSyncIdForTaggedNode(const std::string& tag,
+                                                      int64* sync_id) {
   sync_api::ReadTransaction trans(
       sync_service_->backend()->GetUserShareHandle());
   sync_api::ReadNode sync_node(&trans);
@@ -285,20 +291,20 @@ bool ModelAssociator::GetSyncIdForTaggedNode(const std::string& tag,
   return true;
 }
 
-bool ModelAssociator::AssociateModels() {
+bool BookmarkModelAssociator::AssociateModels() {
   // Try to load model associations from persisted associations first. If that
   // succeeds, we don't need to run the complex model matching algorithm.
   if (LoadAssociations())
     return true;
 
-  ClearAll();
+  DisassociateModels();
 
   // We couldn't load model associations from persisted associations. So build
   // them.
   return BuildAssociations();
 }
 
-bool ModelAssociator::BuildAssociations() {
+bool BookmarkModelAssociator::BuildAssociations() {
   // Algorithm description:
   // Match up the roots and recursively do the following:
   // * For each sync node for the current sync parent node, find the best
@@ -379,15 +385,15 @@ bool ModelAssociator::BuildAssociations() {
       if (child_node) {
         model->Move(child_node, parent_node, index);
         // Set the favicon for bookmark node from sync node or vice versa.
-        if (ChangeProcessor::SetBookmarkFavicon(&sync_child_node,
+        if (BookmarkChangeProcessor::SetBookmarkFavicon(&sync_child_node,
             child_node, sync_service_->profile())) {
-          ChangeProcessor::SetSyncNodeFavicon(child_node, model,
-                                              &sync_child_node);
+          BookmarkChangeProcessor::SetSyncNodeFavicon(child_node, model,
+                                                       &sync_child_node);
         }
       } else {
         // Create a new bookmark node for the sync node.
-        child_node = ChangeProcessor::CreateBookmarkNode(&sync_child_node,
-            parent_node, model, index);
+        child_node = BookmarkChangeProcessor::CreateBookmarkNode(
+            &sync_child_node, parent_node, model, index);
       }
       Associate(child_node, sync_child_id);
       if (sync_child_node.GetIsFolder())
@@ -403,8 +409,8 @@ bool ModelAssociator::BuildAssociations() {
     // So the children starting from index in the parent bookmark node are the
     // ones that are not present in the parent sync node. So create them.
     for (int i = index; i < parent_node->GetChildCount(); ++i) {
-      sync_child_id = ChangeProcessor::CreateSyncNode(parent_node, model, i,
-          &trans, this, sync_service_);
+      sync_child_id = BookmarkChangeProcessor::CreateSyncNode(parent_node,
+          model, i, &trans, this, sync_service_);
       if (parent_node->GetChild(i)->is_folder())
         dfs_stack.push(sync_child_id);
     }
@@ -412,20 +418,17 @@ bool ModelAssociator::BuildAssociations() {
   return true;
 }
 
-void ModelAssociator::PostPersistAssociationsTask() {
+void BookmarkModelAssociator::PostPersistAssociationsTask() {
   // No need to post a task if a task is already pending.
-  if (task_pending_)
+  if (!persist_associations_.empty())
     return;
-  task_pending_ = true;
   MessageLoop::current()->PostTask(
       FROM_HERE,
-      NewRunnableMethod(this, &ModelAssociator::PersistAssociations));
+      persist_associations_.NewRunnableMethod(
+          &BookmarkModelAssociator::PersistAssociations));
 }
 
-void ModelAssociator::PersistAssociations() {
-  DCHECK(task_pending_);
-  task_pending_ = false;
-
+void BookmarkModelAssociator::PersistAssociations() {
   // If there are no dirty associations we have nothing to do. We handle this
   // explicity instead of letting the for loop do it to avoid creating a write
   // transaction in this case.
@@ -456,7 +459,7 @@ void ModelAssociator::PersistAssociations() {
   dirty_associations_sync_ids_.clear();
 }
 
-bool ModelAssociator::LoadAssociations() {
+bool BookmarkModelAssociator::LoadAssociations() {
   BookmarkModel* model = sync_service_->profile()->GetBookmarkModel();
   DCHECK(model->IsLoaded());
   // If the bookmarks changed externally, our previous associations may not be
diff --git a/chrome/browser/sync/glue/bookmark_model_associator.h b/chrome/browser/sync/glue/bookmark_model_associator.h
new file mode 100644
index 0000000..d722367
--- /dev/null
+++ b/chrome/browser/sync/glue/bookmark_model_associator.h
@@ -0,0 +1,135 @@
+// Copyright (c) 2006-2009 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.
+
+#ifndef CHROME_BROWSER_SYNC_GLUE_BOOKMARK_MODEL_ASSOCIATOR_H_
+#define CHROME_BROWSER_SYNC_GLUE_BOOKMARK_MODEL_ASSOCIATOR_H_
+
+#include <map>
+#include <set>
+#include <string>
+
+#include "base/basictypes.h"
+#include "base/task.h"
+#include "chrome/browser/sync/glue/model_associator.h"
+
+class BookmarkNode;
+
+namespace sync_api {
+class BaseNode;
+class BaseTransaction;
+class ReadNode;
+}
+
+class ProfileSyncService;
+
+namespace browser_sync {
+
+class BookmarkChangeProcessor;
+
+// Contains all model association related logic:
+// * Algorithm to associate bookmark model and sync model.
+// * Methods to get a bookmark node for a given sync node and vice versa.
+// * Persisting model associations and loading them back.
+class BookmarkModelAssociator : public ModelAssociator {
+ public:
+  explicit BookmarkModelAssociator(ProfileSyncService* sync_service);
+  virtual ~BookmarkModelAssociator() { }
+
+  // AssociateModels iterates through both the sync and the browser
+  // bookmark model, looking for matched pairs of items.  For any pairs it
+  // finds, it will call AssociateSyncID.  For any unmatched items,
+  // MergeAndAssociateModels will try to repair the match, e.g. by adding a new
+  // node.  After successful completion, the models should be identical and
+  // corresponding. Returns true on success.  On failure of this step, we
+  // should abort the sync operation and report an error to the user.
+  virtual bool AssociateModels();
+
+  // Clears all associations.
+  virtual bool DisassociateModels();
+
+  // Returns whether the sync model has nodes other than the permanent tagged
+  // nodes.
+  virtual bool SyncModelHasUserCreatedNodes();
+
+  // Returns whether the bookmark model has user created nodes or not. That is,
+  // whether there are nodes in the bookmark model except the bookmark bar and
+  // other bookmarks.
+  virtual bool ChromeModelHasUserCreatedNodes();
+
+  // Returns sync id for the given bookmark node id.
+  // Returns sync_api::kInvalidId if the sync node is not found for the given
+  // bookmark node id.
+  int64 GetSyncIdFromBookmarkId(int64 node_id) const;
+
+  // Returns the bookmark node for the given sync id.
+  // Returns NULL if no bookmark node is found for the given sync id.
+  const BookmarkNode* GetBookmarkNodeFromSyncId(int64 sync_id);
+
+  // Initializes the given sync node from the given bookmark node id.
+  // Returns false if no sync node was found for the given bookmark node id or
+  // if the initialization of sync node fails.
+  bool InitSyncNodeFromBookmarkId(int64 node_id, sync_api::BaseNode* sync_node);
+
+  // Associates the given bookmark node with the given sync id.
+  void Associate(const BookmarkNode* node, int64 sync_id);
+  // Remove the association that corresponds to the given sync id.
+  void Disassociate(int64 sync_id);
+
+ protected:
+  // Stores the id of the node with the given tag in |sync_id|.
+  // Returns of that node was found successfully.
+  // Tests override this.
+   virtual bool GetSyncIdForTaggedNode(const std::string& tag, int64* sync_id);
+
+  // Returns sync service instance.
+  ProfileSyncService* sync_service() { return sync_service_; }
+
+ private:
+  typedef std::map<int64, int64> BookmarkIdToSyncIdMap;
+  typedef std::map<int64, const BookmarkNode*> SyncIdToBookmarkNodeMap;
+  typedef std::set<int64> DirtyAssociationsSyncIds;
+
+  // Posts a task to persist dirty associations.
+  void PostPersistAssociationsTask();
+  // Persists all dirty associations.
+  void PersistAssociations();
+
+  // Loads the persisted associations into in-memory maps.
+  // If the persisted associations are out-of-date due to some reason, returns
+  // false; otherwise returns true.
+  bool LoadAssociations();
+
+  // Matches up the bookmark model and the sync model to build model
+  // associations.
+  bool BuildAssociations();
+
+  // Associate a top-level node of the bookmark model with a permanent node in
+  // the sync domain.  Such permanent nodes are identified by a tag that is
+  // well known to the server and the client, and is unique within a particular
+  // user's share.  For example, "other_bookmarks" is the tag for the Other
+  // Bookmarks folder.  The sync nodes are server-created.
+  bool AssociateTaggedPermanentNode(const BookmarkNode* permanent_node,
+                                    const std::string& tag);
+
+  // Compare the properties of a pair of nodes from either domain.
+  bool NodesMatch(const BookmarkNode* bookmark,
+                  const sync_api::BaseNode* sync_node) const;
+
+  ProfileSyncService* sync_service_;
+  BookmarkIdToSyncIdMap id_map_;
+  SyncIdToBookmarkNodeMap id_map_inverse_;
+  // Stores sync ids for dirty associations.
+  DirtyAssociationsSyncIds dirty_associations_sync_ids_;
+
+  // Used to post PersistAssociation tasks to the current message loop and
+  // guarantees no invocations can occur if |this| has been deleted. (This
+  // allows this class to be non-refcounted).
+  ScopedRunnableMethodFactory<BookmarkModelAssociator> persist_associations_;
+
+  DISALLOW_COPY_AND_ASSIGN(BookmarkModelAssociator);
+};
+
+}  // namespace browser_sync
+
+#endif  // CHROME_BROWSER_SYNC_GLUE_BOOKMARK_MODEL_ASSOCIATOR_H_
diff --git a/chrome/browser/sync/glue/change_processor.cc b/chrome/browser/sync/glue/change_processor.cc
index 6b4e9f7..0c608b8 100644
--- a/chrome/browser/sync/glue/change_processor.cc
+++ b/chrome/browser/sync/glue/change_processor.cc
@@ -1,542 +1,30 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2006-2009 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/change_processor.h"
-
-#include <stack>
-
-#include "app/gfx/codec/png_codec.h"
-#include "base/string_util.h"
-#include "chrome/browser/bookmarks/bookmark_utils.h"
-#include "chrome/browser/favicon_service.h"
 #include "chrome/browser/profile.h"
-#include "chrome/browser/sync/profile_sync_service.h"
-#include "third_party/skia/include/core/SkBitmap.h"
 
 namespace browser_sync {
 
-void ChangeProcessor::Start(BookmarkModel* model, sync_api::UserShare* handle) {
-  DCHECK(error_handler_ && model_associator_);
-  DCHECK(!share_handle_ && !bookmark_model_);
-  share_handle_ = handle;
-  bookmark_model_ = model;
-  DCHECK(model->IsLoaded());
-  bookmark_model_->AddObserver(this);
+ChangeProcessor::~ChangeProcessor() {
+  Stop();
+}
+
+void ChangeProcessor::Start(Profile* profile,
+                            sync_api::UserShare* share_handle) {
+  DCHECK(error_handler_ && !share_handle_);
+  share_handle_ = share_handle;
+  StartImpl(profile);
   running_ = true;
 }
 
 void ChangeProcessor::Stop() {
   if (!running_)
     return;
-  DCHECK(bookmark_model_);
-  bookmark_model_->RemoveObserver(this);
-  bookmark_model_ = NULL;
+  StopImpl();
   share_handle_ = NULL;
-  model_associator_ = NULL;
   running_ = false;
 }
 
-void ChangeProcessor::UpdateSyncNodeProperties(const BookmarkNode* src,
-                                               BookmarkModel* model,
-                                               sync_api::WriteNode* dst) {
-  // Set the properties of the item.
-  dst->SetIsFolder(src->is_folder());
-  dst->SetTitle(src->GetTitle());
-  dst->SetURL(src->GetURL());
-  SetSyncNodeFavicon(src, model, dst);
-}
-
-// static
-void ChangeProcessor::EncodeFavicon(const BookmarkNode* src,
-                                    BookmarkModel* model,
-                                    std::vector<unsigned char>* dst) {
-  const SkBitmap& favicon = model->GetFavIcon(src);
-
-  dst->clear();
-
-  // Check for zero-dimension images.  This can happen if the favicon is
-  // still being loaded.
-  if (favicon.empty())
-    return;
-
-  // Re-encode the BookmarkNode's favicon as a PNG, and pass the data to the
-  // sync subsystem.
-  if (!gfx::PNGCodec::EncodeBGRASkBitmap(favicon, false, dst))
-    return;
-}
-
-void ChangeProcessor::RemoveOneSyncNode(sync_api::WriteTransaction* trans,
-                                        const BookmarkNode* node) {
-  sync_api::WriteNode sync_node(trans);
-  if (!model_associator_->InitSyncNodeFromBookmarkId(node->id(), &sync_node)) {
-    error_handler_->OnUnrecoverableError();
-    return;
-  }
-  // This node should have no children.
-  DCHECK(sync_node.GetFirstChildId() == sync_api::kInvalidId);
-  // Remove association and delete the sync node.
-  model_associator_->Disassociate(sync_node.GetId());
-  sync_node.Remove();
-}
-
-void ChangeProcessor::RemoveSyncNodeHierarchy(const BookmarkNode* topmost) {
-  sync_api::WriteTransaction trans(share_handle_);
-
-  // Later logic assumes that |topmost| has been unlinked.
-  DCHECK(!topmost->GetParent());
-
-  // 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->GetParent() || (node->GetParent()->IndexOfChild(node) ==
-      index_stack.top()));
-    if (index == node->GetChildCount()) {
-      // If we've processed all of |node|'s children, delete |node| and move
-      // on to its successor.
-      RemoveOneSyncNode(&trans, node);
-      node = node->GetParent();
-      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->GetChildCount());
-      index_stack.push(index);
-      node = node->GetChild(index);
-      index = 0;
-    }
-  }
-  DCHECK(index_stack.empty());  // Nothing should be left on the stack.
-}
-
-void ChangeProcessor::BookmarkModelBeingDeleted(BookmarkModel* model) {
-  DCHECK(!running_) << "BookmarkModel deleted while ChangeProcessor running.";
-  bookmark_model_ = NULL;
-}
-
-void ChangeProcessor::BookmarkNodeAdded(BookmarkModel* model,
-                                        const BookmarkNode* parent,
-                                        int index) {
-  DCHECK(running_);
-  DCHECK(share_handle_);
-
-  // Acquire a scoped write lock via a transaction.
-  sync_api::WriteTransaction trans(share_handle_);
-
-  CreateSyncNode(parent, model, index, &trans, model_associator_,
-                 error_handler_);
-}
-
-// static
-int64 ChangeProcessor::CreateSyncNode(const BookmarkNode* parent,
-    BookmarkModel* model, int index, sync_api::WriteTransaction* trans,
-    ModelAssociator* associator, UnrecoverableErrorHandler* error_handler) {
-  const BookmarkNode* child = parent->GetChild(index);
-  DCHECK(child);
-
-  // Create a WriteNode container to hold the new node.
-  sync_api::WriteNode sync_child(trans);
-
-  // Actually create the node with the appropriate initial position.
-  if (!PlaceSyncNode(CREATE, parent, index, trans, &sync_child, associator,
-                     error_handler)) {
-    LOG(WARNING) << "Sync node creation failed; recovery unlikely";
-    error_handler->OnUnrecoverableError();
-    return sync_api::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 ChangeProcessor::BookmarkNodeRemoved(BookmarkModel* model,
-                                          const BookmarkNode* parent,
-                                          int index,
-                                          const BookmarkNode* node) {
-  DCHECK(running_);
-  RemoveSyncNodeHierarchy(node);
-}
-
-void ChangeProcessor::BookmarkNodeChanged(BookmarkModel* model,
-                                          const BookmarkNode* node) {
-  DCHECK(running_);
-  // We shouldn't see changes to the top-level nodes.
-  if (node == model->GetBookmarkBarNode() || node == model->other_node()) {
-    NOTREACHED() << "Saw update to permanent node!";
-    return;
-  }
-
-  // Acquire a scoped write lock via a transaction.
-  sync_api::WriteTransaction trans(share_handle_);
-
-  // Lookup the sync node that's associated with |node|.
-  sync_api::WriteNode sync_node(&trans);
-  if (!model_associator_->InitSyncNodeFromBookmarkId(node->id(), &sync_node)) {
-    error_handler_->OnUnrecoverableError();
-    return;
-  }
-
-  UpdateSyncNodeProperties(node, model, &sync_node);
-
-  DCHECK_EQ(sync_node.GetIsFolder(), node->is_folder());
-  DCHECK_EQ(model_associator_->GetBookmarkNodeFromSyncId(
-            sync_node.GetParentId()),
-            node->GetParent());
-  // This node's index should be one more than the predecessor's index.
-  DCHECK_EQ(node->GetParent()->IndexOfChild(node),
-            CalculateBookmarkModelInsertionIndex(node->GetParent(),
-                                                 &sync_node));
-}
-
-
-void ChangeProcessor::BookmarkNodeMoved(BookmarkModel* model,
-                                        const BookmarkNode* old_parent,
-                                        int old_index,
-                                        const BookmarkNode* new_parent,
-                                        int new_index) {
-  DCHECK(running_);
-  const BookmarkNode* child = new_parent->GetChild(new_index);
-  // We shouldn't see changes to the top-level nodes.
-  if (child == model->GetBookmarkBarNode() || child == model->other_node()) {
-    NOTREACHED() << "Saw update to permanent node!";
-    return;
-  }
-
-  // Acquire a scoped write lock via a transaction.
-  sync_api::WriteTransaction trans(share_handle_);
-
-  // Lookup the sync node that's associated with |child|.
-  sync_api::WriteNode sync_node(&trans);
-  if (!model_associator_->InitSyncNodeFromBookmarkId(child->id(), &sync_node)) {
-    error_handler_->OnUnrecoverableError();
-    return;
-  }
-
-  if (!PlaceSyncNode(MOVE, new_parent, new_index, &trans, &sync_node,
-                     model_associator_, error_handler_)) {
-    error_handler_->OnUnrecoverableError();
-    return;
-  }
-}
-
-void ChangeProcessor::BookmarkNodeFavIconLoaded(BookmarkModel* model,
-                                                const BookmarkNode* node) {
-  DCHECK(running_);
-  BookmarkNodeChanged(model, node);
-}
-
-void ChangeProcessor::BookmarkNodeChildrenReordered(
-    BookmarkModel* model, const BookmarkNode* node) {
-
-  // Acquire a scoped write lock via a transaction.
-  sync_api::WriteTransaction trans(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->GetChildCount(); ++i) {
-    sync_api::WriteNode sync_child(&trans);
-    if (!model_associator_->InitSyncNodeFromBookmarkId(node->GetChild(i)->id(),
-                                                       &sync_child)) {
-      error_handler_->OnUnrecoverableError();
-      return;
-    }
-    DCHECK_EQ(sync_child.GetParentId(),
-              model_associator_->GetSyncIdFromBookmarkId(node->id()));
-
-    if (!PlaceSyncNode(MOVE, node, i, &trans, &sync_child, model_associator_,
-                       error_handler_)) {
-      error_handler_->OnUnrecoverableError();
-      return;
-    }
-  }
-}
-
-// static
-bool ChangeProcessor::PlaceSyncNode(MoveOrCreate operation,
-                                    const BookmarkNode* parent,
-                                    int index,
-                                    sync_api::WriteTransaction* trans,
-                                    sync_api::WriteNode* dst,
-                                    ModelAssociator* associator,
-                                    UnrecoverableErrorHandler* error_handler) {
-  sync_api::ReadNode sync_parent(trans);
-  if (!associator->InitSyncNodeFromBookmarkId(parent->id(), &sync_parent)) {
-    LOG(WARNING) << "Parent lookup failed";
-    error_handler->OnUnrecoverableError();
-    return false;
-  }
-
-  bool success = false;
-  if (index == 0) {
-    // Insert into first position.
-    success = (operation == CREATE) ? dst->InitByCreation(sync_parent, NULL) :
-                                      dst->SetPosition(sync_parent, NULL);
-    if (success) {
-      DCHECK_EQ(dst->GetParentId(), sync_parent.GetId());
-      DCHECK_EQ(dst->GetId(), sync_parent.GetFirstChildId());
-      DCHECK_EQ(dst->GetPredecessorId(), sync_api::kInvalidId);
-    }
-  } else {
-    // Find the bookmark model predecessor, and insert after it.
-    const BookmarkNode* prev = parent->GetChild(index - 1);
-    sync_api::ReadNode sync_prev(trans);
-    if (!associator->InitSyncNodeFromBookmarkId(prev->id(), &sync_prev)) {
-      LOG(WARNING) << "Predecessor lookup failed";
-      return false;
-    }
-    success = (operation == CREATE) ?
-        dst->InitByCreation(sync_parent, &sync_prev) :
-        dst->SetPosition(sync_parent, &sync_prev);
-    if (success) {
-      DCHECK_EQ(dst->GetParentId(), sync_parent.GetId());
-      DCHECK_EQ(dst->GetPredecessorId(), sync_prev.GetId());
-      DCHECK_EQ(dst->GetId(), sync_prev.GetSuccessorId());
-    }
-  }
-  return success;
-}
-
-// Determine the bookmark model index to which a node must be moved so that
-// predecessor of the node (in the bookmark model) matches the predecessor of
-// |source| (in the sync model).
-// As a precondition, this assumes that the predecessor of |source| has been
-// updated and is already in the correct position in the bookmark model.
-int ChangeProcessor::CalculateBookmarkModelInsertionIndex(
-    const BookmarkNode* parent,
-    const sync_api::BaseNode* child_info) const {
-  DCHECK(parent);
-  DCHECK(child_info);
-  int64 predecessor_id = child_info->GetPredecessorId();
-  // A return ID of kInvalidId indicates no predecessor.
-  if (predecessor_id == sync_api::kInvalidId)
-    return 0;
-
-  // Otherwise, insert after the predecessor bookmark node.
-  const BookmarkNode* predecessor =
-      model_associator_->GetBookmarkNodeFromSyncId(predecessor_id);
-  DCHECK(predecessor);
-  DCHECK_EQ(predecessor->GetParent(), parent);
-  return parent->IndexOfChild(predecessor) + 1;
-}
-
-// ApplyModelChanges is called by the sync backend after changes have been made
-// to the sync engine's model.  Apply these changes to the browser bookmark
-// model.
-void ChangeProcessor::ApplyChangesFromSyncModel(
-    const sync_api::BaseTransaction* trans,
-    const sync_api::SyncManager::ChangeRecord* changes,
-    int change_count) {
-  if (!running_)
-    return;
-  // A note about ordering.  Sync backend is responsible for ordering the change
-  // records in the following order:
-  //
-  // 1. Deletions, from leaves up to parents.
-  // 2. Existing items with synced parents & predecessors.
-  // 3. New items with synced parents & predecessors.
-  // 4. Items with parents & predecessors in the list.
-  // 5. Repeat #4 until all items are in the list.
-  //
-  // "Predecessor" here means the previous item within a given folder; an item
-  // in the first position is always said to have a synced predecessor.
-  // For the most part, applying these changes in the order given will yield
-  // the correct result.  There is one exception, however: for items that are
-  // moved away from a folder that is being deleted, we will process the delete
-  // before the move.  Since deletions in the bookmark model propagate from
-  // parent to child, we must move them to a temporary location.
-  BookmarkModel* model = bookmark_model_;
-
-  // We are going to make changes to the bookmarks model, but don't want to end
-  // up in a feedback loop, so remove ourselves as an observer while applying
-  // changes.
-  model->RemoveObserver(this);
-
-  // A parent to hold nodes temporarily orphaned by parent deletion.  It is
-  // lazily created inside the loop.
-  const BookmarkNode* foster_parent = NULL;
-  for (int i = 0; i < change_count; ++i) {
-    const BookmarkNode* dst =
-        model_associator_->GetBookmarkNodeFromSyncId(changes[i].id);
-    // Ignore changes to the permanent top-level nodes.  We only care about
-    // their children.
-    if ((dst == model->GetBookmarkBarNode()) || (dst == model->other_node()))
-      continue;
-    if (changes[i].action ==
-        sync_api::SyncManager::ChangeRecord::ACTION_DELETE) {
-      // Deletions should always be at the front of the list.
-      DCHECK(i == 0 || changes[i-1].action == changes[i].action);
-      // Children of a deleted node should not be deleted; they may be
-      // reparented by a later change record.  Move them to a temporary place.
-      DCHECK(dst) << "Could not find node to be deleted";
-      const BookmarkNode* parent = dst->GetParent();
-      if (dst->GetChildCount()) {
-        if (!foster_parent) {
-          foster_parent = model->AddGroup(model->other_node(),
-                                          model->other_node()->GetChildCount(),
-                                          std::wstring());
-        }
-        for (int i = dst->GetChildCount() - 1; i >= 0; --i) {
-          model->Move(dst->GetChild(i), foster_parent,
-                      foster_parent->GetChildCount());
-        }
-      }
-      DCHECK_EQ(dst->GetChildCount(), 0) << "Node being deleted has children";
-      model_associator_->Disassociate(changes[i].id);
-      model->Remove(parent, parent->IndexOfChild(dst));
-      dst = NULL;
-    } else {
-      DCHECK_EQ((changes[i].action ==
-          sync_api::SyncManager::ChangeRecord::ACTION_ADD), (dst == NULL))
-          << "ACTION_ADD should be seen if and only if the node is unknown.";
-
-      sync_api::ReadNode src(trans);
-      if (!src.InitByIdLookup(changes[i].id)) {
-        LOG(ERROR) << "ApplyModelChanges was passed a bad ID";
-        error_handler_->OnUnrecoverableError();
-        return;
-      }
-
-      CreateOrUpdateBookmarkNode(&src, model);
-    }
-  }
-  // Clean up the temporary node.
-  if (foster_parent) {
-    // There should be no nodes left under the foster parent.
-    DCHECK_EQ(foster_parent->GetChildCount(), 0);
-    model->Remove(foster_parent->GetParent(),
-                  foster_parent->GetParent()->IndexOfChild(foster_parent));
-    foster_parent = NULL;
-  }
-
-  // We are now ready to hear about bookmarks changes again.
-  model->AddObserver(this);
-}
-
-// Create a bookmark node corresponding to |src| if one is not already
-// associated with |src|.
-const BookmarkNode* ChangeProcessor::CreateOrUpdateBookmarkNode(
-    sync_api::BaseNode* src,
-    BookmarkModel* model) {
-  const BookmarkNode* parent =
-      model_associator_->GetBookmarkNodeFromSyncId(src->GetParentId());
-  if (!parent) {
-    DLOG(WARNING) << "Could not find parent of node being added/updated."
-      << " Node title: " << src->GetTitle()
-      << ", parent id = " << src->GetParentId();
-    return NULL;
-  }
-  int index = CalculateBookmarkModelInsertionIndex(parent, src);
-  const BookmarkNode* dst = model_associator_->GetBookmarkNodeFromSyncId(
-      src->GetId());
-  if (!dst) {
-    dst = CreateBookmarkNode(src, parent, model, index);
-    model_associator_->Associate(dst, src->GetId());
-  } else {
-    // URL and is_folder are not expected to change.
-    // TODO(ncarter): Determine if such changes should be legal or not.
-    DCHECK_EQ(src->GetIsFolder(), dst->is_folder());
-
-    // Handle reparenting and/or repositioning.
-    model->Move(dst, parent, index);
-
-    // Handle title update and URL changes due to possible conflict resolution
-    // that can happen if both a local user change and server change occur
-    // within a sufficiently small time interval.
-    const BookmarkNode* old_dst = dst;
-    dst = bookmark_utils::ApplyEditsWithNoGroupChange(model, parent,
-        BookmarkEditor::EditDetails(dst),
-        src->GetTitle(),
-        src->GetIsFolder() ? GURL() : src->GetURL(),
-        NULL);  // NULL because we don't need a BookmarkEditor::Handler.
-    if (dst != old_dst) {  // dst was replaced with a new node with new URL.
-      model_associator_->Disassociate(src->GetId());
-      model_associator_->Associate(dst, src->GetId());
-    }
-    SetBookmarkFavicon(src, dst, model->profile());
-  }
-
-  return dst;
-}
-
-// static
-// Creates a bookmark node under the given parent node from the given sync
-// node. Returns the newly created node.
-const BookmarkNode* ChangeProcessor::CreateBookmarkNode(
-    sync_api::BaseNode* sync_node,
-    const BookmarkNode* parent,
-    BookmarkModel* model,
-    int index) {
-  DCHECK(parent);
-  DCHECK(index >= 0 && index <= parent->GetChildCount());
-
-  const BookmarkNode* node;
-  if (sync_node->GetIsFolder()) {
-    node = model->AddGroup(parent, index, sync_node->GetTitle());
-  } else {
-    node = model->AddURL(parent, index,
-                         sync_node->GetTitle(), sync_node->GetURL());
-    SetBookmarkFavicon(sync_node, node, model->profile());
-  }
-  return node;
-}
-
-// static
-// Sets the favicon of the given bookmark node from the given sync node.
-bool ChangeProcessor::SetBookmarkFavicon(
-    sync_api::BaseNode* sync_node,
-    const BookmarkNode* bookmark_node,
-    Profile* profile) {
-  size_t icon_size = 0;
-  const unsigned char* icon_bytes = sync_node->GetFaviconBytes(&icon_size);
-  if (!icon_size || !icon_bytes)
-    return false;
-
-  // Registering a favicon requires that we provide a source URL, but we
-  // don't know where these came from.  Currently we just use the
-  // destination URL, which is not correct, but since the favicon URL
-  // is used as a key in the history's thumbnail DB, this gives us a value
-  // which does not collide with others.
-  GURL fake_icon_url = bookmark_node->GetURL();
-
-  std::vector<unsigned char> icon_bytes_vector(icon_bytes,
-                                               icon_bytes + icon_size);
-
-  HistoryService* history =
-      profile->GetHistoryService(Profile::EXPLICIT_ACCESS);
-  FaviconService* favicon_service =
-      profile->GetFaviconService(Profile::EXPLICIT_ACCESS);
-
-  history->AddPage(bookmark_node->GetURL());
-  favicon_service->SetFavicon(bookmark_node->GetURL(),
-                              fake_icon_url,
-                              icon_bytes_vector);
-
-  return true;
-}
-
-// static
-void ChangeProcessor::SetSyncNodeFavicon(
-    const BookmarkNode* bookmark_node,
-    BookmarkModel* model,
-    sync_api::WriteNode* sync_node) {
-  std::vector<unsigned char> favicon_bytes;
-  EncodeFavicon(bookmark_node, model, &favicon_bytes);
-  if (!favicon_bytes.empty())
-    sync_node->SetFaviconBytes(&favicon_bytes[0], favicon_bytes.size());
-}
-
 }  // namespace browser_sync
diff --git a/chrome/browser/sync/glue/change_processor.h b/chrome/browser/sync/glue/change_processor.h
index fe7719b..2537636 100644
--- a/chrome/browser/sync/glue/change_processor.h
+++ b/chrome/browser/sync/glue/change_processor.h
@@ -1,181 +1,68 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2006-2009 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.
 
 #ifndef CHROME_BROWSER_SYNC_GLUE_CHANGE_PROCESSOR_H_
 #define CHROME_BROWSER_SYNC_GLUE_CHANGE_PROCESSOR_H_
 
-#include "chrome/browser/bookmarks/bookmark_model.h"
 #include "chrome/browser/sync/engine/syncapi.h"
-#include "chrome/browser/sync/glue/model_associator.h"
 #include "chrome/browser/sync/glue/sync_backend_host.h"
 
-class ProfileSyncService;
+class Profile;
 
 namespace browser_sync {
 
+class ModelAssociator;
 class UnrecoverableErrorHandler;
 
-// This class is responsible for taking changes from the BookmarkModel
-// and applying them to the sync_api 'syncable' model, and vice versa.
-// All operations and use of this class are from the UI thread.
-// This is currently bookmarks specific.
-class ChangeProcessor : public BookmarkModelObserver,
-                        public ChangeProcessingInterface {
+// An interface used to apply changes from the sync model to the browser's
+// native model.  This does not currently distinguish between model data types.
+class ChangeProcessor {
  public:
   explicit ChangeProcessor(UnrecoverableErrorHandler* error_handler)
-      : running_(false), error_handler_(error_handler),
-        bookmark_model_(NULL), share_handle_(NULL), model_associator_(NULL) {}
-  virtual ~ChangeProcessor() { Stop(); }
+      : running_(false), error_handler_(error_handler), share_handle_(NULL) {}
+  virtual ~ChangeProcessor();
 
   // Call when the processor should accept changes from either provided model
-  // and apply them to the other.  Both the BookmarkModel and sync_api are
+  // and apply them to the other.  Both the chrome model and sync_api are
   // expected to be initialized and loaded.  You must have set a valid
   // ModelAssociator and UnrecoverableErrorHandler before using this method,
   // and the two models should be associated w.r.t the ModelAssociator provided.
   // It is safe to call Start again after previously Stop()ing the processor.
-  void Start(BookmarkModel* model, sync_api::UserShare* handle);
-
-  // Call when processing changes between models is no longer safe / needed.
+  // Subclasses can extract their associated chrome model from |profile| in
+  // |StartImpl|.
+  void Start(Profile* profile, sync_api::UserShare* share_handle);
   void Stop();
-
   bool IsRunning() const { return running_; }
 
-  // Injectors for the components required to Start the processor.
-  void set_model_associator(ModelAssociator* associator) {
-    model_associator_ = associator;
-  }
-
-  // BookmarkModelObserver implementation.
-  // BookmarkModel -> sync_api model change application.
-  virtual void Loaded(BookmarkModel* model) { NOTREACHED(); }
-  virtual void BookmarkModelBeingDeleted(BookmarkModel* model);
-  virtual void BookmarkNodeMoved(BookmarkModel* model,
-                                 const BookmarkNode* old_parent,
-                                 int old_index,
-                                 const BookmarkNode* new_parent,
-                                 int new_index);
-  virtual void BookmarkNodeAdded(BookmarkModel* model,
-                                 const BookmarkNode* parent,
-                                 int index);
-  virtual void BookmarkNodeRemoved(BookmarkModel* model,
-                                   const BookmarkNode* parent,
-                                   int index,
-                                   const BookmarkNode* node);
-  virtual void BookmarkNodeChanged(BookmarkModel* model,
-                                   const BookmarkNode* node);
-  virtual void BookmarkNodeFavIconLoaded(BookmarkModel* model,
-                                         const BookmarkNode* node);
-  virtual void BookmarkNodeChildrenReordered(BookmarkModel* model,
-                                             const BookmarkNode* node);
-
-  // sync_api model -> BookmarkModel change application.
+  virtual ModelAssociator* GetModelAssociator() = 0;
+
+  // Changes have been applied to the backend model and are ready to be
+  // applied to the frontend model. See syncapi.h for detailed instructions on
+  // how to interpret and process |changes|.
   virtual void ApplyChangesFromSyncModel(
       const sync_api::BaseTransaction* trans,
       const sync_api::SyncManager::ChangeRecord* changes,
-      int change_count);
-
-  // The following methods are static and hence may be invoked at any time,
-  // and do not depend on having a running ChangeProcessor.
-  // Creates a bookmark node under the given parent node from the given sync
-  // node. Returns the newly created node.
-  static const BookmarkNode* CreateBookmarkNode(
-      sync_api::BaseNode* sync_node,
-      const BookmarkNode* parent,
-      BookmarkModel* model,
-      int index);
-
-  // Sets the favicon of the given bookmark node from the given sync node.
-  // Returns whether the favicon was set in the bookmark node.
-  // |profile| is the profile that contains the HistoryService and BookmarkModel
-  // for the bookmark in question.
-  static bool SetBookmarkFavicon(sync_api::BaseNode* sync_node,
-                                 const BookmarkNode* bookmark_node,
-                                 Profile* profile);
-
-  // Sets the favicon of the given sync node from the given bookmark node.
-  static void SetSyncNodeFavicon(const BookmarkNode* bookmark_node,
-                                 BookmarkModel* model,
-                                 sync_api::WriteNode* sync_node);
-
-  // Treat the |index|th child of |parent| as a newly added node, and create a
-  // corresponding node in the sync domain using |trans|.  All properties
-  // will be transferred to the new node.  A node corresponding to |parent|
-  // must already exist and be associated for this call to succeed.  Returns
-  // the ID of the just-created node, or if creation fails, kInvalidID.
-  static int64 CreateSyncNode(const BookmarkNode* parent,
-                              BookmarkModel* model,
-                              int index,
-                              sync_api::WriteTransaction* trans,
-                              ModelAssociator* associator,
-                              UnrecoverableErrorHandler* error_handler);
+      int change_count) = 0;
 
- private:
-  enum MoveOrCreate {
-    MOVE,
-    CREATE,
-  };
-
-  // Create a bookmark node corresponding to |src| if one is not already
-  // associated with |src|.  Returns the node that was created or updated.
-  const BookmarkNode* CreateOrUpdateBookmarkNode(
-      sync_api::BaseNode* src,
-      BookmarkModel* model);
-
-  // Helper function to determine the appropriate insertion index of sync node
-  // |node| under the Bookmark model node |parent|, to make the positions
-  // match up between the two models. This presumes that the predecessor of the
-  // item (in the bookmark model) has already been moved into its appropriate
-  // position.
-  int CalculateBookmarkModelInsertionIndex(
-      const BookmarkNode* parent,
-      const sync_api::BaseNode* node) const;
-
-  // Helper function used to fix the position of a sync node so that it matches
-  // the position of a corresponding bookmark model node. |parent| and
-  // |index| identify the bookmark model position.  |dst| is the node whose
-  // position is to be fixed.  If |operation| is CREATE, treat |dst| as an
-  // uncreated node and set its position via InitByCreation(); otherwise,
-  // |dst| is treated as an existing node, and its position will be set via
-  // SetPosition().  |trans| is the transaction to which |dst| belongs. Returns
-  // false on failure.
-  static bool PlaceSyncNode(MoveOrCreate operation,
-                            const BookmarkNode* parent,
-                            int index,
-                            sync_api::WriteTransaction* trans,
-                            sync_api::WriteNode* dst,
-                            ModelAssociator* associator,
-                            UnrecoverableErrorHandler* error_handler);
-
-  // Copy properties (but not position) from |src| to |dst|.
-  static void UpdateSyncNodeProperties(const BookmarkNode* src,
-                                       BookmarkModel* model,
-                                       sync_api::WriteNode* dst);
-
-  // Helper function to encode a bookmark's favicon into a PNG byte vector.
-  static void EncodeFavicon(const BookmarkNode* src,
-                            BookmarkModel* model,
-                            std::vector<unsigned char>* dst);
-
-  // Remove the sync node corresponding to |node|.  It shouldn't have
-  // any children.
-  void RemoveOneSyncNode(sync_api::WriteTransaction* trans,
-                         const BookmarkNode* node);
-
-  // Remove all the sync nodes associated with |node| and its children.
-  void RemoveSyncNodeHierarchy(const BookmarkNode* node);
+ protected:
+  // These methods are invoked by Start() and Stop() to do
+  // implementation-specific work.
+  virtual void StartImpl(Profile* profile) = 0;
+  virtual void StopImpl() = 0;
 
+  bool running() { return running_; }
+  UnrecoverableErrorHandler* error_handler() { return error_handler_; }
+  sync_api::UserShare* share_handle() { return share_handle_; }
+
+ private:
   bool running_;  // True if we have been told it is safe to process changes.
   UnrecoverableErrorHandler* error_handler_;  // Guaranteed to outlive us.
 
-  // The two models we are processing changes between.  Non-NULL when
-  // |running_| is true.
-  BookmarkModel* bookmark_model_;
+  // The sync model we are processing changes from. Non-NULL when |running_| is
+  // true.
   sync_api::UserShare* share_handle_;
 
-  // The two models should be associated according to this ModelAssociator.
-  scoped_refptr<ModelAssociator> model_associator_;
-
   DISALLOW_COPY_AND_ASSIGN(ChangeProcessor);
 };
 
diff --git a/chrome/browser/sync/glue/model_associator.h b/chrome/browser/sync/glue/model_associator.h
index e7d5c32..7e09be2 100644
--- a/chrome/browser/sync/glue/model_associator.h
+++ b/chrome/browser/sync/glue/model_associator.h
@@ -5,131 +5,31 @@
 #ifndef CHROME_BROWSER_SYNC_GLUE_MODEL_ASSOCIATOR_H_
 #define CHROME_BROWSER_SYNC_GLUE_MODEL_ASSOCIATOR_H_
 
-#include <map>
-#include <set>
-#include <string>
-
-#include "base/basictypes.h"
-#include "base/ref_counted.h"
-#include "base/scoped_ptr.h"
-
-class BookmarkNode;
-
-namespace sync_api {
-class BaseNode;
-class BaseTransaction;
-class ReadNode;
-}
-
-class ProfileSyncService;
+#include "chrome/browser/sync/engine/syncapi.h"
 
 namespace browser_sync {
 
-class ChangeProcessor;
-
-// Contains all model association related logic:
-// * Algorithm to associate bookmark model and sync model.
-// * Methods to get a bookmark node for a given sync node and vice versa.
-// * Persisting model associations and loading them back.
-class ModelAssociator
-    : public base::RefCountedThreadSafe<ModelAssociator> {
+// Contains all model association related logic to associate the chrome model
+// with the sync model.
+class ModelAssociator {
  public:
-  explicit ModelAssociator(ProfileSyncService* sync_service);
-
-  // Clears all associations.
-  void ClearAll();
+  virtual ~ModelAssociator() {}
 
-  // Returns sync id for the given bookmark node id.
-  // Returns sync_api::kInvalidId if the sync node is not found for the given
-  // bookmark node id.
-  int64 GetSyncIdFromBookmarkId(int64 node_id) const;
-
-  // Returns the bookmark node for the given sync id.
-  // Returns NULL if no bookmark node is found for the given sync id.
-  const BookmarkNode* GetBookmarkNodeFromSyncId(int64 sync_id);
-
-  // Initializes the given sync node from the given bookmark node id.
-  // Returns false if no sync node was found for the given bookmark node id or
-  // if the initialization of sync node fails.
-  bool InitSyncNodeFromBookmarkId(int64 node_id, sync_api::BaseNode* sync_node);
-
-  // Associates the given bookmark node with the given sync id.
-  void Associate(const BookmarkNode* node, int64 sync_id);
-  // Remove the association that corresponds to the given sync id.
-  void Disassociate(int64 sync_id);
+  // Iterates through both the sync and the chrome model looking for matched
+  // pairs of items. After successful completion, the models should be identical
+  // and corresponding. Returns true on success. On failure of this step, we
+  // should abort the sync operation and report an error to the user.
+  virtual bool AssociateModels() = 0;
 
-  // Returns whether the bookmark model has user created nodes or not. That is,
-  // whether there are nodes in the bookmark model except the bookmark bar and
-  // other bookmarks.
-  bool BookmarkModelHasUserCreatedNodes() const;
+  // Clears all the associations between the chrome and sync models.
+  virtual bool DisassociateModels() = 0;
 
   // Returns whether the sync model has nodes other than the permanent tagged
   // nodes.
-  bool SyncModelHasUserCreatedNodes();
-
-  // AssociateModels iterates through both the sync and the browser
-  // bookmark model, looking for matched pairs of items.  For any pairs it
-  // finds, it will call AssociateSyncID.  For any unmatched items,
-  // MergeAndAssociateModels will try to repair the match, e.g. by adding a new
-  // node.  After successful completion, the models should be identical and
-  // corresponding. Returns true on success.  On failure of this step, we
-  // should abort the sync operation and report an error to the user.
-  bool AssociateModels();
-
- protected:
-  friend class base::RefCountedThreadSafe<ModelAssociator>;
-  virtual ~ModelAssociator() { }
-
-  // Stores the id of the node with the given tag in |sync_id|.
-  // Returns of that node was found successfully.
-  // Tests override this.
-   virtual bool GetSyncIdForTaggedNode(const std::string& tag, int64* sync_id);
-
-  // Returns sync service instance.
-  ProfileSyncService* sync_service() { return sync_service_; }
-
- private:
-  typedef std::map<int64, int64> BookmarkIdToSyncIdMap;
-  typedef std::map<int64, const BookmarkNode*> SyncIdToBookmarkNodeMap;
-  typedef std::set<int64> DirtyAssociationsSyncIds;
-
-  // Posts a task to persist dirty associations.
-  void PostPersistAssociationsTask();
-  // Persists all dirty associations.
-  void PersistAssociations();
-
-  // Loads the persisted associations into in-memory maps.
-  // If the persisted associations are out-of-date due to some reason, returns
-  // false; otherwise returns true.
-  bool LoadAssociations();
-
-  // Matches up the bookmark model and the sync model to build model
-  // associations.
-  bool BuildAssociations();
-
-  // Associate a top-level node of the bookmark model with a permanent node in
-  // the sync domain.  Such permanent nodes are identified by a tag that is
-  // well known to the server and the client, and is unique within a particular
-  // user's share.  For example, "other_bookmarks" is the tag for the Other
-  // Bookmarks folder.  The sync nodes are server-created.
-  bool AssociateTaggedPermanentNode(const BookmarkNode* permanent_node,
-                                    const std::string& tag);
-
-  // Compare the properties of a pair of nodes from either domain.
-  bool NodesMatch(const BookmarkNode* bookmark,
-                  const sync_api::BaseNode* sync_node) const;
-
-  ProfileSyncService* sync_service_;
-  BookmarkIdToSyncIdMap id_map_;
-  SyncIdToBookmarkNodeMap id_map_inverse_;
-  // Stores sync ids for dirty associations.
-  DirtyAssociationsSyncIds dirty_associations_sync_ids_;
-
-  // Indicates whether there is already a pending task to persist dirty model
-  // associations.
-  bool task_pending_;
+  virtual bool SyncModelHasUserCreatedNodes() = 0;
 
-  DISALLOW_COPY_AND_ASSIGN(ModelAssociator);
+  // Returns whether the chrome model has user-created nodes or not.
+  virtual bool ChromeModelHasUserCreatedNodes() = 0;
 };
 
 }  // namespace browser_sync
diff --git a/chrome/browser/sync/glue/sync_backend_host.cc b/chrome/browser/sync/glue/sync_backend_host.cc
index d6917cd..8bff80c 100644
--- a/chrome/browser/sync/glue/sync_backend_host.cc
+++ b/chrome/browser/sync/glue/sync_backend_host.cc
@@ -25,12 +25,12 @@ namespace browser_sync {
 
 SyncBackendHost::SyncBackendHost(SyncFrontend* frontend,
                                  const FilePath& profile_path,
-                                 ChangeProcessingInterface* processor)
+                                 std::set<ChangeProcessor*> processors)
     : core_thread_("Chrome_SyncCoreThread"),
       frontend_loop_(MessageLoop::current()),
       bookmark_model_worker_(NULL),
       frontend_(frontend),
-      processor_(processor),
+      processors_(processors),
       sync_data_folder_path_(profile_path.Append(kSyncDataFolderName)),
       last_auth_error_(AuthError::None()) {
   core_ = new Core(this);
@@ -243,7 +243,12 @@ void SyncBackendHost::Core::OnChangesApplied(
     DLOG(WARNING) << "Could not update bookmark model from non-UI thread";
     return;
   }
-  host_->processor_->ApplyChangesFromSyncModel(trans, changes, change_count);
+  for (std::set<ChangeProcessor*>::const_iterator it =
+       host_->processors_.begin(); it != host_->processors_.end(); ++it) {
+    // TODO(sync): Filter per data-type here and apply only the relevant
+    // changes for each processor.
+    (*it)->ApplyChangesFromSyncModel(trans, changes, change_count);
+  }
 }
 
 void SyncBackendHost::Core::OnSyncCycleCompleted() {
diff --git a/chrome/browser/sync/glue/sync_backend_host.h b/chrome/browser/sync/glue/sync_backend_host.h
index 80696eb..59ce96c 100644
--- a/chrome/browser/sync/glue/sync_backend_host.h
+++ b/chrome/browser/sync/glue/sync_backend_host.h
@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_SYNC_GLUE_SYNC_BACKEND_HOST_H_
 #define CHROME_BROWSER_SYNC_GLUE_SYNC_BACKEND_HOST_H_
 
+#include <set>
 #include <string>
 
 #include "base/file_path.h"
@@ -21,6 +22,8 @@
 
 namespace browser_sync {
 
+class ChangeProcessor;
+
 // SyncFrontend is the interface used by SyncBackendHost to communicate with
 // the entity that created it and, presumably, is interested in sync-related
 // activity.
@@ -49,21 +52,6 @@ class SyncFrontend {
   DISALLOW_COPY_AND_ASSIGN(SyncFrontend);
 };
 
-// An interface used to apply changes from the sync model to the browser's
-// native model.  This does not currently distinguish between model data types.
-class ChangeProcessingInterface {
- public:
-  // Changes have been applied to the backend model and are ready to be
-  // applied to the frontend model. See syncapi.h for detailed instructions on
-  // how to interpret and process |changes|.
-  virtual void ApplyChangesFromSyncModel(
-      const sync_api::BaseTransaction* trans,
-      const sync_api::SyncManager::ChangeRecord* changes,
-      int change_count) = 0;
- protected:
-  virtual ~ChangeProcessingInterface() { }
-};
-
 // A UI-thread safe API into the sync backend that "hosts" the top-level
 // syncapi element, the SyncManager, on its own thread. This class handles
 // dispatch of potentially blocking calls to appropriate threads and ensures
@@ -79,7 +67,7 @@ class SyncBackendHost {
   // it used to call the constructor), and push changes from sync_api through
   // |processor|.
   SyncBackendHost(SyncFrontend* frontend, const FilePath& profile_path,
-                  ChangeProcessingInterface* processor);
+                  std::set<ChangeProcessor*> processor);
   ~SyncBackendHost();
 
   // Called on |frontend_loop_| to kick off asynchronous initialization.
@@ -282,7 +270,8 @@ class SyncBackendHost {
   // The frontend which we serve (and are owned by).
   SyncFrontend* frontend_;
 
-  ChangeProcessingInterface* processor_;  // Guaranteed to outlive us.
+  // The change processors that handle the different data types.
+  std::set<ChangeProcessor*> processors_;
 
   // Path of the folder that stores the sync data files.
   FilePath sync_data_folder_path_;
diff --git a/chrome/browser/sync/profile_sync_service.cc b/chrome/browser/sync/profile_sync_service.cc
index 37539ef..be160fd 100644
--- a/chrome/browser/sync/profile_sync_service.cc
+++ b/chrome/browser/sync/profile_sync_service.cc
@@ -21,6 +21,7 @@
 #include "chrome/browser/history/history_types.h"
 #include "chrome/browser/profile.h"
 #include "chrome/browser/sync/engine/syncapi.h"
+#include "chrome/browser/sync/glue/bookmark_change_processor.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/notification_service.h"
 #include "chrome/common/notification_type.h"
@@ -31,8 +32,9 @@
 #include "net/base/cookie_monster.h"
 #include "views/window/window.h"
 
+using browser_sync::BookmarkChangeProcessor;
+using browser_sync::BookmarkModelAssociator;
 using browser_sync::ChangeProcessor;
-using browser_sync::ModelAssociator;
 using browser_sync::SyncBackendHost;
 
 typedef GoogleServiceAuthError AuthError;
@@ -49,17 +51,22 @@ ProfileSyncService::ProfileSyncService(Profile* profile)
       is_auth_in_progress_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(wizard_(this)),
       unrecoverable_error_detected_(false) {
-  change_processor_.reset(new ChangeProcessor(this));
+  BookmarkChangeProcessor* processor = new BookmarkChangeProcessor(this);
+  change_processors_.insert(processor);
+  // TODO: Move this back to StartUp
+  BookmarkModelAssociator* associator = new BookmarkModelAssociator(this);
+  processor->set_model_associator(associator);
 }
 
 ProfileSyncService::~ProfileSyncService() {
   Shutdown(false);
+  STLDeleteElements(&change_processors_);
 }
 
-void ProfileSyncService::set_model_associator(
-    browser_sync::ModelAssociator* associator) {
-  model_associator_ = associator;
-  change_processor_->set_model_associator(associator);
+void ProfileSyncService::set_change_processor(
+    browser_sync::ChangeProcessor* change_processor) {
+  change_processors_.clear();
+  change_processors_.insert(change_processor);
 }
 
 void ProfileSyncService::Initialize() {
@@ -153,15 +160,11 @@ void ProfileSyncService::StartUp() {
       profile_->GetPrefs()->GetInt64(prefs::kSyncLastSyncedTime));
 
   backend_.reset(new SyncBackendHost(this, profile_->GetPath(),
-                                     change_processor_.get()));
+                                     change_processors_));
 
   registrar_.Add(this, NotificationType::BOOKMARK_MODEL_LOADED,
                  Source<Profile>(profile_));
 
-  // Create new model association manager and change processor.
-  model_associator_ = new ModelAssociator(this);
-  change_processor_->set_model_associator(model_associator_);
-
   InitializeBackend();
 }
 
@@ -171,13 +174,16 @@ void ProfileSyncService::Shutdown(bool sync_disabled) {
   if (backend_.get())
     backend_->Shutdown(sync_disabled);
 
-  change_processor_->Stop();
+  for (std::set<ChangeProcessor*>::const_iterator it =
+       change_processors_.begin(); it != change_processors_.end(); ++it) {
+    (*it)->Stop();
+  }
   backend_.reset();
 
   // Clear all associations and throw away the association manager instance.
-  if (model_associator_.get()) {
-    model_associator_->ClearAll();
-    model_associator_ = NULL;
+  for (std::set<ChangeProcessor*>::const_iterator it =
+       change_processors_.begin(); it != change_processors_.end(); ++it) {
+    (*it)->GetModelAssociator()->DisassociateModels();
   }
 
   // Clear various flags.
@@ -222,8 +228,13 @@ bool ProfileSyncService::MergeAndSyncAcceptanceNeeded() const {
   if (profile_->GetPrefs()->GetBoolean(prefs::kSyncHasSetupCompleted))
     return false;
 
-  return model_associator_->BookmarkModelHasUserCreatedNodes() &&
-         model_associator_->SyncModelHasUserCreatedNodes();
+  for (std::set<ChangeProcessor*>::const_iterator it =
+       change_processors_.begin(); it != change_processors_.end(); ++it) {
+    if ((*it)->GetModelAssociator()->ChromeModelHasUserCreatedNodes() &&
+        (*it)->GetModelAssociator()->SyncModelHasUserCreatedNodes())
+      return true;
+  }
+  return false;
 }
 
 bool ProfileSyncService::HasSyncSetupCompleted() const {
@@ -247,7 +258,10 @@ void ProfileSyncService::UpdateLastSyncedTime() {
 // to do as little work as possible, to avoid further corruption or crashes.
 void ProfileSyncService::OnUnrecoverableError() {
   unrecoverable_error_detected_ = true;
-  change_processor_->Stop();
+  for (std::set<ChangeProcessor*>::const_iterator it =
+       change_processors_.begin(); it != change_processors_.end(); ++it) {
+    (*it)->Stop();
+  }
 
   // Tell the wizard so it can inform the user only if it is already open.
   wizard_.Step(SyncSetupWizard::FATAL_ERROR);
@@ -375,8 +389,17 @@ void ProfileSyncService::OnUserSubmittedAuth(
 
 void ProfileSyncService::OnUserAcceptedMergeAndSync() {
   base::TimeTicks start_time = base::TimeTicks::Now();
-  bool not_first_run = model_associator_->SyncModelHasUserCreatedNodes();
-  bool merge_success = model_associator_->AssociateModels();
+  bool not_first_run = false;
+  bool merge_success = true;
+  for (std::set<ChangeProcessor*>::const_iterator it =
+       change_processors_.begin(); it != change_processors_.end(); ++it) {
+    not_first_run |= (*it)->GetModelAssociator()->
+        SyncModelHasUserCreatedNodes();
+    // TODO(sync): Figure out what do to when a single associator fails.
+    // http://crbug.com/30038
+    merge_success &= (*it)->GetModelAssociator()->
+        AssociateModels();
+  }
   UMA_HISTOGRAM_MEDIUM_TIMES("Sync.UserPerceivedBookmarkAssociation",
                              base::TimeTicks::Now() - start_time);
   if (!merge_success) {
@@ -388,8 +411,10 @@ void ProfileSyncService::OnUserAcceptedMergeAndSync() {
   wizard_.Step(not_first_run ? SyncSetupWizard::DONE :
                SyncSetupWizard::DONE_FIRST_TIME);
 
-  change_processor_->Start(profile_->GetBookmarkModel(),
-                           backend_->GetUserShareHandle());
+  for (std::set<ChangeProcessor*>::const_iterator it =
+       change_processors_.begin(); it != change_processors_.end(); ++it) {
+    (*it)->Start(profile(), backend_->GetUserShareHandle());
+  }
   FOR_EACH_OBSERVER(Observer, observers_, OnStateChanged());
 }
 
@@ -406,7 +431,10 @@ void ProfileSyncService::OnUserCancelledDialog() {
 void ProfileSyncService::StartProcessingChangesIfReady() {
   BookmarkModel* model = profile_->GetBookmarkModel();
 
-  DCHECK(!change_processor_->IsRunning());
+  for (std::set<ChangeProcessor*>::const_iterator it =
+       change_processors_.begin(); it != change_processors_.end(); ++it) {
+    DCHECK(!(*it)->IsRunning());
+  }
 
   // First check if the subsystems are ready.  We can't proceed until they
   // both have finished loading.
@@ -424,8 +452,15 @@ void ProfileSyncService::StartProcessingChangesIfReady() {
 
   // We're ready to merge the models.
   base::TimeTicks start_time = base::TimeTicks::Now();
-  bool not_first_run = model_associator_->SyncModelHasUserCreatedNodes();
-  bool merge_success = model_associator_->AssociateModels();
+  bool not_first_run = false;
+  bool merge_success = true;
+  for (std::set<ChangeProcessor*>::const_iterator it =
+       change_processors_.begin(); it != change_processors_.end(); ++it) {
+    not_first_run |= (*it)->GetModelAssociator()->
+        SyncModelHasUserCreatedNodes();
+    merge_success &= (*it)->GetModelAssociator()->
+        AssociateModels();
+  }
   UMA_HISTOGRAM_TIMES("Sync.BookmarkAssociationTime",
                       base::TimeTicks::Now() - start_time);
   if (!merge_success) {
@@ -437,8 +472,10 @@ void ProfileSyncService::StartProcessingChangesIfReady() {
   wizard_.Step(not_first_run ? SyncSetupWizard::DONE :
                SyncSetupWizard::DONE_FIRST_TIME);
 
-  change_processor_->Start(profile_->GetBookmarkModel(),
-                           backend_->GetUserShareHandle());
+  for (std::set<ChangeProcessor*>::const_iterator it =
+       change_processors_.begin(); it != change_processors_.end(); ++it) {
+    (*it)->Start(profile(), backend_->GetUserShareHandle());
+  }
   FOR_EACH_OBSERVER(Observer, observers_, OnStateChanged());
 }
 
@@ -469,5 +506,9 @@ bool ProfileSyncService::ShouldPushChanges() {
   // True only after all bootstrapping has succeeded: the bookmark model is
   // loaded, the sync backend is initialized, the two domains are
   // consistent with one another, and no unrecoverable error has transpired.
-  return change_processor_->IsRunning();
+  for (std::set<ChangeProcessor*>::const_iterator it =
+       change_processors_.begin(); it != change_processors_.end(); ++it) {
+    if (!(*it)->IsRunning()) return false;
+  }
+  return true;
 }
diff --git a/chrome/browser/sync/profile_sync_service.h b/chrome/browser/sync/profile_sync_service.h
index 59bc2e5..7a0dab08 100644
--- a/chrome/browser/sync/profile_sync_service.h
+++ b/chrome/browser/sync/profile_sync_service.h
@@ -28,7 +28,6 @@ class MessageLoop;
 class Profile;
 
 namespace browser_sync {
-class ModelAssociator;
 
 class UnrecoverableErrorHandler {
  public:
@@ -231,7 +230,7 @@ class ProfileSyncService : public NotificationObserver,
   virtual void InitializeBackend();
 
   // Tests need this.
-  void set_model_associator(browser_sync::ModelAssociator* associator);
+  void set_change_processor(browser_sync::ChangeProcessor* change_processor);
 
   // We keep track of the last auth error observed so we can cover up the first
   // "expected" auth failure from observers.
@@ -274,9 +273,6 @@ class ProfileSyncService : public NotificationObserver,
   // This specifies where to find the sync server.
   GURL sync_service_url_;
 
-  // Model association manager instance.
-  scoped_refptr<browser_sync::ModelAssociator> model_associator_;
-
   // The last time we detected a successful transition from SYNCING state.
   // Our backend notifies us whenever we should take a new snapshot.
   base::Time last_synced_time_;
@@ -285,7 +281,8 @@ class ProfileSyncService : public NotificationObserver,
   // other threads.
   scoped_ptr<browser_sync::SyncBackendHost> backend_;
 
-  scoped_ptr<browser_sync::ChangeProcessor> change_processor_;
+  // The change processors that handle the different data types.
+  std::set<browser_sync::ChangeProcessor*> change_processors_;
 
   NotificationRegistrar registrar_;
 
diff --git a/chrome/browser/sync/profile_sync_service_unittest.cc b/chrome/browser/sync/profile_sync_service_unittest.cc
index aa87866..ea575a9 100644
--- a/chrome/browser/sync/profile_sync_service_unittest.cc
+++ b/chrome/browser/sync/profile_sync_service_unittest.cc
@@ -15,7 +15,8 @@
 #include "chrome/browser/chrome_thread.h"
 #include "chrome/browser/profile.h"
 #include "chrome/browser/sync/engine/syncapi.h"
-#include "chrome/browser/sync/glue/model_associator.h"
+#include "chrome/browser/sync/glue/bookmark_change_processor.h"
+#include "chrome/browser/sync/glue/bookmark_model_associator.h"
 #include "chrome/browser/sync/glue/sync_backend_host.h"
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "chrome/common/chrome_switches.h"
@@ -23,15 +24,17 @@
 #include "chrome/test/sync/test_http_bridge_factory.h"
 
 using std::vector;
-using browser_sync::ChangeProcessingInterface;
+using browser_sync::BookmarkChangeProcessor;
+using browser_sync::BookmarkModelAssociator;
+using browser_sync::ChangeProcessor;
 using browser_sync::ModelAssociator;
 using browser_sync::SyncBackendHost;
 using browser_sync::TestHttpBridgeFactory;
 
-class TestModelAssociator : public ModelAssociator {
+class TestModelAssociator : public BookmarkModelAssociator {
  public:
   explicit TestModelAssociator(ProfileSyncService* service)
-      : ModelAssociator(service) {
+      : BookmarkModelAssociator(service) {
   }
 
   virtual bool GetSyncIdForTaggedNode(const std::string& tag, int64* sync_id) {
@@ -92,7 +95,10 @@ class TestProfileSyncService : public ProfileSyncService {
   }
 
   virtual void InitializeBackend() {
-    set_model_associator(new TestModelAssociator(this));
+    change_processor_ = new BookmarkChangeProcessor(this);
+    model_associator_ = new TestModelAssociator(this);
+    change_processor_->set_model_associator(model_associator_);
+    set_change_processor(change_processor_);
     TestHttpBridgeFactory* factory = new TestHttpBridgeFactory();
     TestHttpBridgeFactory* factory2 = new TestHttpBridgeFactory();
     backend()->InitializeForTestMode(L"testuser", factory, factory2);
@@ -112,6 +118,16 @@ class TestProfileSyncService : public ProfileSyncService {
     // Never show the dialog.
     return false;
   }
+
+  BookmarkChangeProcessor* change_processor() {
+    return change_processor_;
+  }
+  BookmarkModelAssociator* model_associator() {
+    return model_associator_;
+  }
+
+  BookmarkChangeProcessor* change_processor_;
+  BookmarkModelAssociator* model_associator_;
 };
 
 // FakeServerChange constructs a list of sync_api::ChangeRecords while modifying
@@ -228,7 +244,7 @@ class FakeServerChange {
   }
 
   // Pass the fake change list to |service|.
-  void ApplyPendingChanges(browser_sync::ChangeProcessingInterface* processor) {
+  void ApplyPendingChanges(browser_sync::ChangeProcessor* processor) {
     processor->ApplyChangesFromSyncModel(trans_,
         changes_.size() ? &changes_[0] : NULL, changes_.size());
   }
@@ -279,14 +295,14 @@ class ProfileSyncServiceTest : public testing::Test {
     MessageLoop::current()->RunAllPending();
   }
 
-  ModelAssociator* associator() {
+  BookmarkModelAssociator* associator() {
     DCHECK(service_.get());
     return service_->model_associator_;
   }
 
-  ChangeProcessingInterface* change_processor() {
+  BookmarkChangeProcessor* change_processor() {
     DCHECK(service_.get());
-    return service_->change_processor_.get();
+    return service_->change_processor_;
   }
 
   void StartSyncService() {
@@ -468,7 +484,7 @@ class ProfileSyncServiceTest : public testing::Test {
   ChromeThread ui_thread_;
   ChromeThread file_thread_;
 
-  scoped_ptr<ProfileSyncService> service_;
+  scoped_ptr<TestProfileSyncService> service_;
   scoped_ptr<TestingProfile> profile_;
   BookmarkModel* model_;
 };
@@ -922,7 +938,7 @@ static TestData kF2Children[] = {
   { L"f2u1", "http://www.f2u1.com/" },
 };
 
-static TestData kOtherBookmarksChildren[] = {
+static TestData kOtherBookmarkChildren[] = {
   { L"f3", NULL },
   { L"u4", "http://www.u4.com/" },
   { L"u3", "http://www.u3.com/" },
@@ -1002,8 +1018,8 @@ void ProfileSyncServiceTestWithData::WriteTestDataToBookmarkModel() {
 
   const BookmarkNode* other_bookmarks_node = model_->other_node();
   PopulateFromTestData(other_bookmarks_node,
-                       kOtherBookmarksChildren,
-                       arraysize(kOtherBookmarksChildren));
+                       kOtherBookmarkChildren,
+                       arraysize(kOtherBookmarkChildren));
 
   ASSERT_GE(other_bookmarks_node->GetChildCount(), 6);
   const BookmarkNode* f3_node = other_bookmarks_node->GetChild(0);
@@ -1032,8 +1048,8 @@ void ProfileSyncServiceTestWithData::ExpectBookmarkModelMatchesTestData() {
 
   const BookmarkNode* other_bookmarks_node = model_->other_node();
   CompareWithTestData(other_bookmarks_node,
-                      kOtherBookmarksChildren,
-                      arraysize(kOtherBookmarksChildren));
+                      kOtherBookmarkChildren,
+                      arraysize(kOtherBookmarkChildren));
 
   ASSERT_GE(other_bookmarks_node->GetChildCount(), 6);
   const BookmarkNode* f3_node = other_bookmarks_node->GetChild(0);