sync: Introduce ModelNeutralWriteTransactions

Introduce ModelNeutralWriteTransactions that allows changes to entries
only if those changes do not need to be sent back to the model.  In
other words, they have write access to almost everything except
SPECIFICS, UNIQUE_POSITION, PARENT_ID, and other similar fields that
need to be reported back to the model to keep it in sync.

We enforce this restriction by allowing the construction of
ModelNeutralMutableEntries from these transactions, but requiring a full
WriteTransaction to construct regular MutableEntries.

To make it easier to write generic functions, this change also
introduces a common base class called BaseWriteTransaction.  This class
is a parent of both WriteTransaction and ModelNeutralWriteTransaction,
so it can be used in contexts where we're willing to accept either.
This is similar to the way BaseTransaction currently enables generic
functions to work with either ReadTransactions or WriteTransactions.

This patch does not actually instantiate any
ModelNeutralWriteTransactions.  The work of converting existing code to
use this new function will be started in future CLs.

BUG=284672

Review URL: https://codereview.chromium.org/24435002

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

14 files changed, 250 insertions, 89 deletions

diff --git a/sync/sync_core.gypi b/sync/sync_core.gypi
index 9633f5e..29650bf 100644
--- a/sync/sync_core.gypi
+++ b/sync/sync_core.gypi
@@ -148,6 +148,8 @@
     'syncable/syncable-inl.h',
     'syncable/syncable_base_transaction.cc',
     'syncable/syncable_base_transaction.h',
+    'syncable/syncable_base_write_transaction.cc',
+    'syncable/syncable_base_write_transaction.h',
     'syncable/syncable_changes_version.h',
     'syncable/syncable_columns.h',
     'syncable/syncable_delete_journal.cc',
@@ -156,6 +158,8 @@
     'syncable/syncable_enum_conversions.h',
     'syncable/syncable_id.cc',
     'syncable/syncable_id.h',
+    'syncable/syncable_model_neutral_write_transaction.cc',
+    'syncable/syncable_model_neutral_write_transaction.h',
     'syncable/syncable_proto_util.cc',
     'syncable/syncable_proto_util.h',
     'syncable/syncable_read_transaction.cc',
diff --git a/sync/syncable/directory.cc b/sync/syncable/directory.cc
index 9754b88..855d442 100644
--- a/sync/syncable/directory.cc
+++ b/sync/syncable/directory.cc
@@ -394,9 +394,9 @@ bool Directory::InsertEntry(WriteTransaction* trans,
   return true;
 }
 
-bool Directory::ReindexId(WriteTransaction* trans,
-                         EntryKernel* const entry,
-                         const Id& new_id) {
+bool Directory::ReindexId(BaseWriteTransaction* trans,
+                          EntryKernel* const entry,
+                          const Id& new_id) {
   ScopedKernelLock lock(this);
   if (NULL != GetEntryById(new_id, &lock))
     return false;
@@ -413,7 +413,7 @@ bool Directory::ReindexId(WriteTransaction* trans,
   return true;
 }
 
-bool Directory::ReindexParentId(WriteTransaction* trans,
+bool Directory::ReindexParentId(BaseWriteTransaction* trans,
                                 EntryKernel* const entry,
                                 const Id& new_parent_id) {
   ScopedKernelLock lock(this);
@@ -957,26 +957,24 @@ void Directory::CollectMetaHandleCounts(
 
 bool Directory::CheckInvariantsOnTransactionClose(
     syncable::BaseTransaction* trans,
-    const EntryKernelMutationMap& mutations) {
+    const MetahandleSet& modified_handles) {
   // NOTE: The trans may be in the process of being destructed.  Be careful if
   // you wish to call any of its virtual methods.
-  MetahandleSet handles;
-
   switch (invariant_check_level_) {
-  case FULL_DB_VERIFICATION:
-    GetAllMetaHandles(trans, &handles);
-    break;
-  case VERIFY_CHANGES:
-    for (EntryKernelMutationMap::const_iterator i = mutations.begin();
-         i != mutations.end(); ++i) {
-      handles.insert(i->first);
+    case FULL_DB_VERIFICATION: {
+      MetahandleSet all_handles;
+      GetAllMetaHandles(trans, &all_handles);
+      return CheckTreeInvariants(trans, all_handles);
+    }
+    case VERIFY_CHANGES: {
+      return CheckTreeInvariants(trans, modified_handles);
+    }
+    case OFF: {
+      return true;
     }
-    break;
-  case OFF:
-    break;
   }
-
-  return CheckTreeInvariants(trans, handles);
+  NOTREACHED();
+  return false;
 }
 
 bool Directory::FullyCheckTreeInvariants(syncable::BaseTransaction* trans) {
diff --git a/sync/syncable/directory.h b/sync/syncable/directory.h
index 6de3a47..5a07322 100644
--- a/sync/syncable/directory.h
+++ b/sync/syncable/directory.h
@@ -32,6 +32,7 @@ class UnrecoverableErrorHandler;
 namespace syncable {
 
 class BaseTransaction;
+class BaseWriteTransaction;
 class DirectoryChangeDelegate;
 class DirectoryBackingStore;
 class NigoriHandler;
@@ -341,7 +342,7 @@ class SYNC_EXPORT Directory {
   // and may be used in release code.
   bool CheckInvariantsOnTransactionClose(
       syncable::BaseTransaction* trans,
-      const EntryKernelMutationMap& mutations);
+      const MetahandleSet& modified_handles);
 
   // Forces a full check of the directory.  This operation may be slow and
   // should not be invoked outside of tests.
@@ -377,9 +378,9 @@ class SYNC_EXPORT Directory {
   EntryKernel* GetEntryByServerTag(const std::string& tag);
   virtual EntryKernel* GetEntryByClientTag(const std::string& tag);
   EntryKernel* GetRootEntry();
-  bool ReindexId(WriteTransaction* trans, EntryKernel* const entry,
+  bool ReindexId(BaseWriteTransaction* trans, EntryKernel* const entry,
                  const Id& new_id);
-  bool ReindexParentId(WriteTransaction* trans, EntryKernel* const entry,
+  bool ReindexParentId(BaseWriteTransaction* trans, EntryKernel* const entry,
                        const Id& new_parent_id);
   void ClearDirtyMetahandles();
 
diff --git a/sync/syncable/metahandle_set.h b/sync/syncable/metahandle_set.h
index 0522825..5b4e425 100644
--- a/sync/syncable/metahandle_set.h
+++ b/sync/syncable/metahandle_set.h
@@ -5,6 +5,8 @@
 #ifndef SYNC_SYNCABLE_METAHANDLE_SET_
 #define SYNC_SYNCABLE_METAHANDLE_SET_
 
+#include <set>
+
 #include "base/basictypes.h"
 
 namespace syncer {
diff --git a/sync/syncable/model_neutral_mutable_entry.cc b/sync/syncable/model_neutral_mutable_entry.cc
index 3d019e6..9b054ef 100644
--- a/sync/syncable/model_neutral_mutable_entry.cc
+++ b/sync/syncable/model_neutral_mutable_entry.cc
@@ -19,28 +19,28 @@ namespace syncer {
 namespace syncable {
 
 ModelNeutralMutableEntry::ModelNeutralMutableEntry(
-    WriteTransaction* trans, GetById, const Id& id)
-    : Entry(trans, GET_BY_ID, id), write_transaction_(trans) {
+    BaseWriteTransaction* trans, GetById, const Id& id)
+    : Entry(trans, GET_BY_ID, id), base_write_transaction_(trans) {
 }
 
 ModelNeutralMutableEntry::ModelNeutralMutableEntry(
-    WriteTransaction* trans, GetByHandle, int64 metahandle)
-    : Entry(trans, GET_BY_HANDLE, metahandle), write_transaction_(trans) {
+    BaseWriteTransaction* trans, GetByHandle, int64 metahandle)
+    : Entry(trans, GET_BY_HANDLE, metahandle), base_write_transaction_(trans) {
 }
 
 ModelNeutralMutableEntry::ModelNeutralMutableEntry(
-    WriteTransaction* trans, GetByClientTag, const std::string& tag)
-    : Entry(trans, GET_BY_CLIENT_TAG, tag), write_transaction_(trans) {
+    BaseWriteTransaction* trans, GetByClientTag, const std::string& tag)
+    : Entry(trans, GET_BY_CLIENT_TAG, tag), base_write_transaction_(trans) {
 }
 
 ModelNeutralMutableEntry::ModelNeutralMutableEntry(
-    WriteTransaction* trans, GetByServerTag, const string& tag)
-    : Entry(trans, GET_BY_SERVER_TAG, tag), write_transaction_(trans) {
+    BaseWriteTransaction* trans, GetByServerTag, const string& tag)
+    : Entry(trans, GET_BY_SERVER_TAG, tag), base_write_transaction_(trans) {
 }
 
 void ModelNeutralMutableEntry::PutBaseVersion(int64 value) {
   DCHECK(kernel_);
-  write_transaction_->SaveOriginal(kernel_);
+  base_write_transaction_->TrackChangesTo(kernel_);
   if (kernel_->ref(BASE_VERSION) != value) {
     kernel_->put(BASE_VERSION, value);
     kernel_->mark_dirty(&dir()->kernel_->dirty_metahandles);
@@ -49,7 +49,7 @@ void ModelNeutralMutableEntry::PutBaseVersion(int64 value) {
 
 void ModelNeutralMutableEntry::PutServerVersion(int64 value) {
   DCHECK(kernel_);
-  write_transaction_->SaveOriginal(kernel_);
+  base_write_transaction_->TrackChangesTo(kernel_);
   if (kernel_->ref(SERVER_VERSION) != value) {
     ScopedKernelLock lock(dir());
     kernel_->put(SERVER_VERSION, value);
@@ -59,7 +59,7 @@ void ModelNeutralMutableEntry::PutServerVersion(int64 value) {
 
 void ModelNeutralMutableEntry::PutServerMtime(base::Time value) {
   DCHECK(kernel_);
-  write_transaction_->SaveOriginal(kernel_);
+  base_write_transaction_->TrackChangesTo(kernel_);
   if (kernel_->ref(SERVER_MTIME) != value) {
     kernel_->put(SERVER_MTIME, value);
     kernel_->mark_dirty(&dir()->kernel_->dirty_metahandles);
@@ -68,7 +68,7 @@ void ModelNeutralMutableEntry::PutServerMtime(base::Time value) {
 
 void ModelNeutralMutableEntry::PutServerCtime(base::Time value) {
   DCHECK(kernel_);
-  write_transaction_->SaveOriginal(kernel_);
+  base_write_transaction_->TrackChangesTo(kernel_);
   if (kernel_->ref(SERVER_CTIME) != value) {
     kernel_->put(SERVER_CTIME, value);
     kernel_->mark_dirty(&dir()->kernel_->dirty_metahandles);
@@ -77,9 +77,9 @@ void ModelNeutralMutableEntry::PutServerCtime(base::Time value) {
 
 bool ModelNeutralMutableEntry::PutId(const Id& value) {
   DCHECK(kernel_);
-  write_transaction_->SaveOriginal(kernel_);
+  base_write_transaction_->TrackChangesTo(kernel_);
   if (kernel_->ref(ID) != value) {
-    if (!dir()->ReindexId(write_transaction(), kernel_, value))
+    if (!dir()->ReindexId(base_write_transaction(), kernel_, value))
       return false;
     kernel_->mark_dirty(&dir()->kernel_->dirty_metahandles);
   }
@@ -88,7 +88,7 @@ bool ModelNeutralMutableEntry::PutId(const Id& value) {
 
 void ModelNeutralMutableEntry::PutServerParentId(const Id& value) {
   DCHECK(kernel_);
-  write_transaction_->SaveOriginal(kernel_);
+  base_write_transaction_->TrackChangesTo(kernel_);
 
   if (kernel_->ref(SERVER_PARENT_ID) != value) {
     kernel_->put(SERVER_PARENT_ID, value);
@@ -98,7 +98,7 @@ void ModelNeutralMutableEntry::PutServerParentId(const Id& value) {
 
 bool ModelNeutralMutableEntry::PutIsUnsynced(bool value) {
   DCHECK(kernel_);
-  write_transaction_->SaveOriginal(kernel_);
+  base_write_transaction_->TrackChangesTo(kernel_);
   if (kernel_->ref(IS_UNSYNCED) != value) {
     MetahandleSet* index = &dir()->kernel_->unsynced_metahandles;
 
@@ -107,14 +107,14 @@ bool ModelNeutralMutableEntry::PutIsUnsynced(bool value) {
       if (!SyncAssert(index->insert(kernel_->ref(META_HANDLE)).second,
                       FROM_HERE,
                       "Could not insert",
-                      write_transaction())) {
+                      base_write_transaction())) {
         return false;
       }
     } else {
       if (!SyncAssert(1U == index->erase(kernel_->ref(META_HANDLE)),
                       FROM_HERE,
                       "Entry Not succesfully erased",
-                      write_transaction())) {
+                      base_write_transaction())) {
         return false;
       }
     }
@@ -126,7 +126,7 @@ bool ModelNeutralMutableEntry::PutIsUnsynced(bool value) {
 
 bool ModelNeutralMutableEntry::PutIsUnappliedUpdate(bool value) {
   DCHECK(kernel_);
-  write_transaction_->SaveOriginal(kernel_);
+  base_write_transaction_->TrackChangesTo(kernel_);
   if (kernel_->ref(IS_UNAPPLIED_UPDATE) != value) {
     // Use kernel_->GetServerModelType() instead of
     // GetServerModelType() as we may trigger some DCHECKs in the
@@ -139,14 +139,14 @@ bool ModelNeutralMutableEntry::PutIsUnappliedUpdate(bool value) {
       if (!SyncAssert(index->insert(kernel_->ref(META_HANDLE)).second,
                       FROM_HERE,
                       "Could not insert",
-                      write_transaction())) {
+                      base_write_transaction())) {
         return false;
       }
     } else {
       if (!SyncAssert(1U == index->erase(kernel_->ref(META_HANDLE)),
                       FROM_HERE,
                       "Entry Not succesfully erased",
-                      write_transaction())) {
+                      base_write_transaction())) {
         return false;
       }
     }
@@ -158,7 +158,7 @@ bool ModelNeutralMutableEntry::PutIsUnappliedUpdate(bool value) {
 
 void ModelNeutralMutableEntry::PutServerIsDir(bool value) {
   DCHECK(kernel_);
-  write_transaction_->SaveOriginal(kernel_);
+  base_write_transaction_->TrackChangesTo(kernel_);
   bool old_value = kernel_->ref(SERVER_IS_DIR);
   if (old_value != value) {
     kernel_->put(SERVER_IS_DIR, value);
@@ -168,7 +168,7 @@ void ModelNeutralMutableEntry::PutServerIsDir(bool value) {
 
 void ModelNeutralMutableEntry::PutServerIsDel(bool value) {
   DCHECK(kernel_);
-  write_transaction_->SaveOriginal(kernel_);
+  base_write_transaction_->TrackChangesTo(kernel_);
   bool old_value = kernel_->ref(SERVER_IS_DEL);
   if (old_value != value) {
     kernel_->put(SERVER_IS_DEL, value);
@@ -181,13 +181,13 @@ void ModelNeutralMutableEntry::PutServerIsDel(bool value) {
   // UpdateDeleteJournalForServerDelete() checks for SERVER_IS_DEL, it has
   // to be called on sync thread.
   dir()->delete_journal()->UpdateDeleteJournalForServerDelete(
-      write_transaction(), old_value, *kernel_);
+      base_write_transaction(), old_value, *kernel_);
 }
 
 void ModelNeutralMutableEntry::PutServerNonUniqueName(
     const std::string& value) {
   DCHECK(kernel_);
-  write_transaction_->SaveOriginal(kernel_);
+  base_write_transaction_->TrackChangesTo(kernel_);
 
   if (kernel_->ref(SERVER_NON_UNIQUE_NAME) != value) {
     kernel_->put(SERVER_NON_UNIQUE_NAME, value);
@@ -200,7 +200,7 @@ bool ModelNeutralMutableEntry::PutUniqueServerTag(const string& new_tag) {
     return true;
   }
 
-  write_transaction_->SaveOriginal(kernel_);
+  base_write_transaction_->TrackChangesTo(kernel_);
   ScopedKernelLock lock(dir());
   // Make sure your new value is not in there already.
   if (dir()->kernel_->server_tags_map.find(new_tag) !=
@@ -224,7 +224,7 @@ bool ModelNeutralMutableEntry::PutUniqueClientTag(const string& new_tag) {
     return true;
   }
 
-  write_transaction_->SaveOriginal(kernel_);
+  base_write_transaction_->TrackChangesTo(kernel_);
   ScopedKernelLock lock(dir());
   // Make sure your new value is not in there already.
   if (dir()->kernel_->client_tags_map.find(new_tag) !=
@@ -270,7 +270,7 @@ void ModelNeutralMutableEntry::PutServerSpecifics(
     const sync_pb::EntitySpecifics& value) {
   DCHECK(kernel_);
   CHECK(!value.password().has_client_only_encrypted_data());
-  write_transaction_->SaveOriginal(kernel_);
+  base_write_transaction_->TrackChangesTo(kernel_);
   // TODO(ncarter): This is unfortunately heavyweight.  Can we do
   // better?
   if (kernel_->ref(SERVER_SPECIFICS).SerializeAsString() !=
@@ -304,7 +304,7 @@ void ModelNeutralMutableEntry::PutBaseServerSpecifics(
     const sync_pb::EntitySpecifics& value) {
   DCHECK(kernel_);
   CHECK(!value.password().has_client_only_encrypted_data());
-  write_transaction_->SaveOriginal(kernel_);
+  base_write_transaction_->TrackChangesTo(kernel_);
   // TODO(ncarter): This is unfortunately heavyweight.  Can we do
   // better?
   if (kernel_->ref(BASE_SERVER_SPECIFICS).SerializeAsString()
@@ -317,7 +317,7 @@ void ModelNeutralMutableEntry::PutBaseServerSpecifics(
 void ModelNeutralMutableEntry::PutServerUniquePosition(
     const UniquePosition& value) {
   DCHECK(kernel_);
-  write_transaction_->SaveOriginal(kernel_);
+  base_write_transaction_->TrackChangesTo(kernel_);
   if(!kernel_->ref(SERVER_UNIQUE_POSITION).Equals(value)) {
     // We should never overwrite a valid position with an invalid one.
     DCHECK(value.IsValid());
@@ -332,8 +332,8 @@ void ModelNeutralMutableEntry::PutSyncing(bool value) {
 }
 
 void ModelNeutralMutableEntry::PutParentIdPropertyOnly(const Id& parent_id) {
-  write_transaction_->SaveOriginal(kernel_);
-  dir()->ReindexParentId(write_transaction(), kernel_, parent_id);
+  base_write_transaction_->TrackChangesTo(kernel_);
+  dir()->ReindexParentId(base_write_transaction(), kernel_, parent_id);
   kernel_->mark_dirty(&dir()->kernel_->dirty_metahandles);
 }
 
@@ -343,8 +343,8 @@ void ModelNeutralMutableEntry::UpdateTransactionVersion(int64 value) {
   kernel_->mark_dirty(&(dir()->kernel_->dirty_metahandles));
 }
 
-ModelNeutralMutableEntry::ModelNeutralMutableEntry(WriteTransaction* trans)
-  : Entry(trans), write_transaction_(trans) {}
+ModelNeutralMutableEntry::ModelNeutralMutableEntry(BaseWriteTransaction* trans)
+  : Entry(trans), base_write_transaction_(trans) {}
 
 MetahandleSet* ModelNeutralMutableEntry::GetDirtyIndexHelper() {
   return &dir()->kernel_->dirty_metahandles;
diff --git a/sync/syncable/model_neutral_mutable_entry.h b/sync/syncable/model_neutral_mutable_entry.h
index 0d15fc9..c9f6c8b 100644
--- a/sync/syncable/model_neutral_mutable_entry.h
+++ b/sync/syncable/model_neutral_mutable_entry.h
@@ -14,7 +14,7 @@ class WriteNode;
 
 namespace syncable {
 
-class WriteTransaction;
+class BaseWriteTransaction;
 
 // This Entry includes all the operations one can safely perform on the sync
 // thread.  In particular, it does not expose setters to make changes that need
@@ -23,19 +23,19 @@ class WriteTransaction;
 // entry.
 class SYNC_EXPORT_PRIVATE ModelNeutralMutableEntry : public Entry {
  public:
-  ModelNeutralMutableEntry(WriteTransaction* trans, GetByHandle, int64);
-  ModelNeutralMutableEntry(WriteTransaction* trans, GetById, const Id&);
+  ModelNeutralMutableEntry(BaseWriteTransaction* trans, GetByHandle, int64);
+  ModelNeutralMutableEntry(BaseWriteTransaction* trans, GetById, const Id&);
   ModelNeutralMutableEntry(
-      WriteTransaction* trans,
+      BaseWriteTransaction* trans,
       GetByClientTag,
       const std::string& tag);
   ModelNeutralMutableEntry(
-      WriteTransaction* trans,
+      BaseWriteTransaction* trans,
       GetByServerTag,
       const std::string& tag);
 
-  inline WriteTransaction* write_transaction() const {
-    return write_transaction_;
+  inline BaseWriteTransaction* base_write_transaction() const {
+    return base_write_transaction_;
   }
 
   // Non-model-changing setters.  These setters will change properties internal
@@ -84,7 +84,7 @@ class SYNC_EXPORT_PRIVATE ModelNeutralMutableEntry : public Entry {
   void UpdateTransactionVersion(int64 version);
 
  protected:
-  explicit ModelNeutralMutableEntry(WriteTransaction* trans);
+  explicit ModelNeutralMutableEntry(BaseWriteTransaction* trans);
 
   syncable::MetahandleSet* GetDirtyIndexHelper();
 
@@ -98,7 +98,7 @@ class SYNC_EXPORT_PRIVATE ModelNeutralMutableEntry : public Entry {
   // Kind of redundant. We should reduce the number of pointers
   // floating around if at all possible. Could we store this in Directory?
   // Scope: Set on construction, never changed after that.
-  WriteTransaction* const write_transaction_;
+  BaseWriteTransaction* const base_write_transaction_;
 
   DISALLOW_COPY_AND_ASSIGN(ModelNeutralMutableEntry);
 };
diff --git a/sync/syncable/mutable_entry.cc b/sync/syncable/mutable_entry.cc
index 6dcf484..a4fadd2 100644
--- a/sync/syncable/mutable_entry.cc
+++ b/sync/syncable/mutable_entry.cc
@@ -47,7 +47,7 @@ void MutableEntry::Init(WriteTransaction* trans,
 
   // Because this entry is new, it was originally deleted.
   kernel->put(IS_DEL, true);
-  trans->SaveOriginal(kernel.get());
+  trans->TrackChangesTo(kernel.get());
   kernel->put(IS_DEL, false);
 
   // Now swap the pointers.
@@ -59,7 +59,7 @@ MutableEntry::MutableEntry(WriteTransaction* trans,
                            ModelType model_type,
                            const Id& parent_id,
                            const string& name)
-    : ModelNeutralMutableEntry(trans) {
+    : ModelNeutralMutableEntry(trans), write_transaction_(trans) {
   Init(trans, model_type, parent_id, name);
   // We need to have a valid position ready before we can index the item.
   if (model_type == BOOKMARKS) {
@@ -78,7 +78,7 @@ MutableEntry::MutableEntry(WriteTransaction* trans,
 
 MutableEntry::MutableEntry(WriteTransaction* trans, CreateNewUpdateItem,
                            const Id& id)
-    : ModelNeutralMutableEntry(trans) {
+    : ModelNeutralMutableEntry(trans), write_transaction_(trans) {
   Entry same_id(trans, GET_BY_ID, id);
   kernel_ = NULL;
   if (same_id.good()) {
@@ -95,33 +95,37 @@ MutableEntry::MutableEntry(WriteTransaction* trans, CreateNewUpdateItem,
   if (!trans->directory()->InsertEntry(trans, kernel.get())) {
     return;  // Failed inserting.
   }
-  trans->SaveOriginal(kernel.get());
+  trans->TrackChangesTo(kernel.get());
 
   kernel_ = kernel.release();
 }
 
 MutableEntry::MutableEntry(WriteTransaction* trans, GetById, const Id& id)
-    : ModelNeutralMutableEntry(trans, GET_BY_ID, id) {
+    : ModelNeutralMutableEntry(trans, GET_BY_ID, id),
+      write_transaction_(trans) {
 }
 
 MutableEntry::MutableEntry(WriteTransaction* trans, GetByHandle,
                            int64 metahandle)
-    : ModelNeutralMutableEntry(trans, GET_BY_HANDLE, metahandle) {
+    : ModelNeutralMutableEntry(trans, GET_BY_HANDLE, metahandle),
+      write_transaction_(trans) {
 }
 
 MutableEntry::MutableEntry(WriteTransaction* trans, GetByClientTag,
                            const std::string& tag)
-    : ModelNeutralMutableEntry(trans, GET_BY_CLIENT_TAG, tag) {
+    : ModelNeutralMutableEntry(trans, GET_BY_CLIENT_TAG, tag),
+      write_transaction_(trans) {
 }
 
 MutableEntry::MutableEntry(WriteTransaction* trans, GetByServerTag,
                            const string& tag)
-    : ModelNeutralMutableEntry(trans, GET_BY_SERVER_TAG, tag) {
+    : ModelNeutralMutableEntry(trans, GET_BY_SERVER_TAG, tag),
+      write_transaction_(trans) {
 }
 
 void MutableEntry::PutLocalExternalId(int64 value) {
   DCHECK(kernel_);
-  write_transaction()->SaveOriginal(kernel_);
+  write_transaction()->TrackChangesTo(kernel_);
   if (kernel_->ref(LOCAL_EXTERNAL_ID) != value) {
     ScopedKernelLock lock(dir());
     kernel_->put(LOCAL_EXTERNAL_ID, value);
@@ -131,7 +135,7 @@ void MutableEntry::PutLocalExternalId(int64 value) {
 
 void MutableEntry::PutMtime(base::Time value) {
   DCHECK(kernel_);
-  write_transaction()->SaveOriginal(kernel_);
+  write_transaction()->TrackChangesTo(kernel_);
   if (kernel_->ref(MTIME) != value) {
     kernel_->put(MTIME, value);
     kernel_->mark_dirty(&dir()->kernel_->dirty_metahandles);
@@ -140,7 +144,7 @@ void MutableEntry::PutMtime(base::Time value) {
 
 void MutableEntry::PutCtime(base::Time value) {
   DCHECK(kernel_);
-  write_transaction()->SaveOriginal(kernel_);
+  write_transaction()->TrackChangesTo(kernel_);
   if (kernel_->ref(CTIME) != value) {
     kernel_->put(CTIME, value);
     kernel_->mark_dirty(&dir()->kernel_->dirty_metahandles);
@@ -149,7 +153,7 @@ void MutableEntry::PutCtime(base::Time value) {
 
 void MutableEntry::PutParentId(const Id& value) {
   DCHECK(kernel_);
-  write_transaction()->SaveOriginal(kernel_);
+  write_transaction()->TrackChangesTo(kernel_);
   if (kernel_->ref(PARENT_ID) != value) {
     PutParentIdPropertyOnly(value);
     if (!GetIsDel()) {
@@ -163,7 +167,7 @@ void MutableEntry::PutParentId(const Id& value) {
 
 void MutableEntry::PutIsDir(bool value) {
   DCHECK(kernel_);
-  write_transaction()->SaveOriginal(kernel_);
+  write_transaction()->TrackChangesTo(kernel_);
   bool old_value = kernel_->ref(IS_DIR);
   if (old_value != value) {
     kernel_->put(IS_DIR, value);
@@ -173,7 +177,7 @@ void MutableEntry::PutIsDir(bool value) {
 
 void MutableEntry::PutIsDel(bool value) {
   DCHECK(kernel_);
-  write_transaction()->SaveOriginal(kernel_);
+  write_transaction()->TrackChangesTo(kernel_);
   if (value == kernel_->ref(IS_DEL)) {
     return;
   }
@@ -205,7 +209,7 @@ void MutableEntry::PutIsDel(bool value) {
 
 void MutableEntry::PutNonUniqueName(const std::string& value) {
   DCHECK(kernel_);
-  write_transaction()->SaveOriginal(kernel_);
+  write_transaction()->TrackChangesTo(kernel_);
 
   if (kernel_->ref(NON_UNIQUE_NAME) != value) {
     kernel_->put(NON_UNIQUE_NAME, value);
@@ -216,7 +220,7 @@ void MutableEntry::PutNonUniqueName(const std::string& value) {
 void MutableEntry::PutSpecifics(const sync_pb::EntitySpecifics& value) {
   DCHECK(kernel_);
   CHECK(!value.password().has_client_only_encrypted_data());
-  write_transaction()->SaveOriginal(kernel_);
+  write_transaction()->TrackChangesTo(kernel_);
   // TODO(ncarter): This is unfortunately heavyweight.  Can we do
   // better?
   if (kernel_->ref(SPECIFICS).SerializeAsString() !=
@@ -228,7 +232,7 @@ void MutableEntry::PutSpecifics(const sync_pb::EntitySpecifics& value) {
 
 void MutableEntry::PutUniquePosition(const UniquePosition& value) {
   DCHECK(kernel_);
-  write_transaction()->SaveOriginal(kernel_);
+  write_transaction()->TrackChangesTo(kernel_);
   if(!kernel_->ref(UNIQUE_POSITION).Equals(value)) {
     // We should never overwrite a valid position with an invalid one.
     DCHECK(value.IsValid());
diff --git a/sync/syncable/mutable_entry.h b/sync/syncable/mutable_entry.h
index f575c22..0b72bcc 100644
--- a/sync/syncable/mutable_entry.h
+++ b/sync/syncable/mutable_entry.h
@@ -41,6 +41,10 @@ class SYNC_EXPORT_PRIVATE MutableEntry : public ModelNeutralMutableEntry {
   MutableEntry(WriteTransaction* trans, GetByClientTag, const std::string& tag);
   MutableEntry(WriteTransaction* trans, GetByServerTag, const std::string& tag);
 
+  inline WriteTransaction* write_transaction() const {
+    return write_transaction_;
+  }
+
   // Model-changing setters.  These setters make user-visible changes that will
   // need to be communicated either to the local model or the sync server.
   void PutLocalExternalId(int64 value);
@@ -58,6 +62,14 @@ class SYNC_EXPORT_PRIVATE MutableEntry : public ModelNeutralMutableEntry {
   // and fails if |predecessor_id| does not identify a sibling.  Pass the root
   // ID to put the node in first position.
   bool PutPredecessor(const Id& predecessor_id);
+
+ private:
+  // Kind of redundant. We should reduce the number of pointers
+  // floating around if at all possible. Could we store this in Directory?
+  // Scope: Set on construction, never changed after that.
+  WriteTransaction* const write_transaction_;
+
+  DISALLOW_COPY_AND_ASSIGN(MutableEntry);
 };
 
 // This function sets only the flags needed to get this entry to sync.
diff --git a/sync/syncable/syncable_base_write_transaction.cc b/sync/syncable/syncable_base_write_transaction.cc
new file mode 100644
index 0000000..a575c69
--- /dev/null
+++ b/sync/syncable/syncable_base_write_transaction.cc
@@ -0,0 +1,22 @@
+// Copyright 2013 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 "sync/syncable/syncable_base_write_transaction.h"
+
+namespace syncer {
+namespace syncable {
+
+BaseWriteTransaction::BaseWriteTransaction(
+    const tracked_objects::Location location,
+    const char* name,
+    WriterTag writer,
+    Directory* directory)
+    : BaseTransaction(location, name, writer, directory) {
+}
+
+BaseWriteTransaction::~BaseWriteTransaction() {}
+
+}  // namespace syncable
+}  // namespace syncer
+
diff --git a/sync/syncable/syncable_base_write_transaction.h b/sync/syncable/syncable_base_write_transaction.h
new file mode 100644
index 0000000..8ea91a1
--- /dev/null
+++ b/sync/syncable/syncable_base_write_transaction.h
@@ -0,0 +1,35 @@
+// Copyright 2013 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 SYNC_SYNCABLE_SYNCABLE_BASE_WRITE_TRANSACTION_H_
+#define SYNC_SYNCABLE_SYNCABLE_BASE_WRITE_TRANSACTION_H_
+
+#include "sync/base/sync_export.h"
+#include "sync/syncable/syncable_base_transaction.h"
+
+namespace syncer {
+namespace syncable {
+
+// A base class shared by both ModelNeutralWriteTransaction and
+// WriteTransaction.
+class SYNC_EXPORT BaseWriteTransaction : public BaseTransaction {
+ public:
+  virtual void TrackChangesTo(const EntryKernel* entry) = 0;
+
+ protected:
+  BaseWriteTransaction(
+      const tracked_objects::Location location,
+      const char* name,
+      WriterTag writer,
+      Directory* directory);
+  virtual ~BaseWriteTransaction();
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(BaseWriteTransaction);
+};
+
+}  // namespace syncable
+}  // namespace syncer
+
+#endif  // SYNC_SYNCABLE_SYNCABLE_BASE_WRITE_TRANSACTION_H_
diff --git a/sync/syncable/syncable_model_neutral_write_transaction.cc b/sync/syncable/syncable_model_neutral_write_transaction.cc
new file mode 100644
index 0000000..9aaf740
--- /dev/null
+++ b/sync/syncable/syncable_model_neutral_write_transaction.cc
@@ -0,0 +1,33 @@
+// Copyright 2013 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 "sync/syncable/syncable_model_neutral_write_transaction.h"
+
+#include "sync/syncable/directory.h"
+
+namespace syncer {
+namespace syncable {
+
+ModelNeutralWriteTransaction::ModelNeutralWriteTransaction(
+    const tracked_objects::Location& location,
+    WriterTag writer, Directory* directory)
+    : BaseWriteTransaction(location,
+                           "ModelNeutralWriteTransaction",
+                           writer,
+                           directory) {
+  Lock();
+}
+
+ModelNeutralWriteTransaction::~ModelNeutralWriteTransaction() {
+  directory()->CheckInvariantsOnTransactionClose(this, modified_handles_);
+  HandleUnrecoverableErrorIfSet();
+  Unlock();
+}
+
+void ModelNeutralWriteTransaction::TrackChangesTo(const EntryKernel* entry) {
+  modified_handles_.insert(entry->ref(META_HANDLE));
+}
+
+}  // namespace syncer
+}  // namespace syncable
diff --git a/sync/syncable/syncable_model_neutral_write_transaction.h b/sync/syncable/syncable_model_neutral_write_transaction.h
new file mode 100644
index 0000000..f96725e
--- /dev/null
+++ b/sync/syncable/syncable_model_neutral_write_transaction.h
@@ -0,0 +1,44 @@
+// Copyright 2013 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 SYNC_SYNCABLE_SYNCABLE_MODEL_NEUTRAL_WRITE_TRANSACTION_H_
+#define SYNC_SYNCABLE_SYNCABLE_MODEL_NEUTRAL_WRITE_TRANSACTION_H_
+
+#include "sync/base/sync_export.h"
+#include "sync/syncable/metahandle_set.h"
+#include "sync/syncable/syncable_base_write_transaction.h"
+
+namespace syncer {
+namespace syncable {
+
+// A transaction used to instantiate Entries or ModelNeutralMutableEntries.
+//
+// This allows it to be used when making changes to sync entity properties that
+// do not need to be kept in sync with the associated native model.
+//
+// This class differs internally from WriteTransactions in that it does a less
+// good job of tracking and reporting on changes to the entries modified within
+// its scope.  This is because its changes do not need to be reported to the
+// DirectoryChangeDelegate.
+class SYNC_EXPORT_PRIVATE ModelNeutralWriteTransaction
+    : public BaseWriteTransaction {
+ public:
+  ModelNeutralWriteTransaction(
+      const tracked_objects::Location& location,
+      WriterTag writer,
+      Directory* directory);
+  virtual ~ModelNeutralWriteTransaction();
+
+  virtual void TrackChangesTo(const EntryKernel* entry) OVERRIDE;
+
+ private:
+  MetahandleSet modified_handles_;
+
+  DISALLOW_COPY_AND_ASSIGN(ModelNeutralWriteTransaction);
+};
+
+}  // namespace syncable
+}  // namespace syncer
+
+#endif  // SYNC_SYNCABLE_SYNCABLE_MODEL_NEUTRAL_WRITE_TRANSACTION_H_
diff --git a/sync/syncable/syncable_write_transaction.cc b/sync/syncable/syncable_write_transaction.cc
index 057b258..d97ff67 100644
--- a/sync/syncable/syncable_write_transaction.cc
+++ b/sync/syncable/syncable_write_transaction.cc
@@ -17,7 +17,7 @@ const int64 kInvalidTransactionVersion = -1;
 
 WriteTransaction::WriteTransaction(const tracked_objects::Location& location,
                                    WriterTag writer, Directory* directory)
-    : BaseTransaction(location, "WriteTransaction", writer, directory),
+    : BaseWriteTransaction(location, "WriteTransaction", writer, directory),
       transaction_version_(NULL) {
   Lock();
 }
@@ -25,14 +25,14 @@ WriteTransaction::WriteTransaction(const tracked_objects::Location& location,
 WriteTransaction::WriteTransaction(const tracked_objects::Location& location,
                                    Directory* directory,
                                    int64* transaction_version)
-    : BaseTransaction(location, "WriteTransaction", SYNCAPI, directory),
+    : BaseWriteTransaction(location, "WriteTransaction", SYNCAPI, directory),
       transaction_version_(transaction_version) {
   Lock();
   if (transaction_version_)
     *transaction_version_ = kInvalidTransactionVersion;
 }
 
-void WriteTransaction::SaveOriginal(const EntryKernel* entry) {
+void WriteTransaction::TrackChangesTo(const EntryKernel* entry) {
   if (!entry) {
     return;
   }
@@ -147,7 +147,13 @@ void WriteTransaction::UpdateTransactionVersion(
 
 WriteTransaction::~WriteTransaction() {
   const ImmutableEntryKernelMutationMap& mutations = RecordMutations();
-  directory()->CheckInvariantsOnTransactionClose(this, mutations.Get());
+
+  MetahandleSet modified_handles;
+  for (EntryKernelMutationMap::const_iterator i = mutations.Get().begin();
+       i != mutations.Get().end(); ++i) {
+    modified_handles.insert(i->first);
+  }
+  directory()->CheckInvariantsOnTransactionClose(this, modified_handles);
 
   // |CheckTreeInvariants| could have thrown an unrecoverable error.
   if (unrecoverable_error_set_) {
diff --git a/sync/syncable/syncable_write_transaction.h b/sync/syncable/syncable_write_transaction.h
index 0debaa5..4d16aca 100644
--- a/sync/syncable/syncable_write_transaction.h
+++ b/sync/syncable/syncable_write_transaction.h
@@ -7,7 +7,7 @@
 
 #include "sync/base/sync_export.h"
 #include "sync/syncable/entry_kernel.h"
-#include "sync/syncable/syncable_base_transaction.h"
+#include "sync/syncable/syncable_base_write_transaction.h"
 
 namespace syncer {
 namespace syncable {
@@ -15,7 +15,7 @@ namespace syncable {
 SYNC_EXPORT extern const int64 kInvalidTransactionVersion;
 
 // Locks db in constructor, unlocks in destructor.
-class SYNC_EXPORT WriteTransaction : public BaseTransaction {
+class SYNC_EXPORT WriteTransaction : public BaseWriteTransaction {
  public:
   WriteTransaction(const tracked_objects::Location& from_here,
                    WriterTag writer, Directory* directory);
@@ -30,7 +30,7 @@ class SYNC_EXPORT WriteTransaction : public BaseTransaction {
 
   virtual ~WriteTransaction();
 
-  void SaveOriginal(const EntryKernel* entry);
+  virtual void TrackChangesTo(const EntryKernel* entry) OVERRIDE;
 
  protected:
   // Overridden by tests.