Revert of Sync: MutableEntry shouldn't track changes that have no actual impact on the stored data

sync/syncable/model_neutral_mutable_entry.cc

Index: sync/syncable/model_neutral_mutable_entry.cc
diff --git a/sync/syncable/model_neutral_mutable_entry.cc b/sync/syncable/model_neutral_mutable_entry.cc
index 498ba9bf304e6ea8999627260708f95489f6718b..bdf5299defa60eb025ea066a73d053c0f93d7ef0 100644
--- a/sync/syncable/model_neutral_mutable_entry.cc
+++ b/sync/syncable/model_neutral_mutable_entry.cc
@@ -105,65 +105,66 @@
 
 void ModelNeutralMutableEntry::PutBaseVersion(int64 value) {
   DCHECK(kernel_);
+  base_write_transaction_->TrackChangesTo(kernel_);
   if (kernel_->ref(BASE_VERSION) != value) {
-    base_write_transaction_->TrackChangesTo(kernel_);
     kernel_->put(BASE_VERSION, value);
-    MarkDirty();
+    kernel_->mark_dirty(&dir()->kernel()->dirty_metahandles);
   }
 }
 
 void ModelNeutralMutableEntry::PutServerVersion(int64 value) {
   DCHECK(kernel_);
+  base_write_transaction_->TrackChangesTo(kernel_);
   if (kernel_->ref(SERVER_VERSION) != value) {
-    base_write_transaction_->TrackChangesTo(kernel_);
     ScopedKernelLock lock(dir());
     kernel_->put(SERVER_VERSION, value);
-    MarkDirty();
+    kernel_->mark_dirty(&dir()->kernel()->dirty_metahandles);
   }
 }
 
 void ModelNeutralMutableEntry::PutServerMtime(base::Time value) {
   DCHECK(kernel_);
+  base_write_transaction_->TrackChangesTo(kernel_);
   if (kernel_->ref(SERVER_MTIME) != value) {
-    base_write_transaction_->TrackChangesTo(kernel_);
     kernel_->put(SERVER_MTIME, value);
-    MarkDirty();
+    kernel_->mark_dirty(&dir()->kernel()->dirty_metahandles);
   }
 }
 
 void ModelNeutralMutableEntry::PutServerCtime(base::Time value) {
   DCHECK(kernel_);
+  base_write_transaction_->TrackChangesTo(kernel_);
   if (kernel_->ref(SERVER_CTIME) != value) {
-    base_write_transaction_->TrackChangesTo(kernel_);
     kernel_->put(SERVER_CTIME, value);
-    MarkDirty();
+    kernel_->mark_dirty(&dir()->kernel()->dirty_metahandles);
   }
 }
 
 bool ModelNeutralMutableEntry::PutId(const Id& value) {
   DCHECK(kernel_);
+  base_write_transaction_->TrackChangesTo(kernel_);
   if (kernel_->ref(ID) != value) {
-    base_write_transaction_->TrackChangesTo(kernel_);
     if (!dir()->ReindexId(base_write_transaction(), kernel_, value))
       return false;
-    MarkDirty();
+    kernel_->mark_dirty(&dir()->kernel()->dirty_metahandles);
   }
   return true;
 }
 
 void ModelNeutralMutableEntry::PutServerParentId(const Id& value) {
   DCHECK(kernel_);
+  base_write_transaction_->TrackChangesTo(kernel_);
+
   if (kernel_->ref(SERVER_PARENT_ID) != value) {
-    base_write_transaction_->TrackChangesTo(kernel_);
     kernel_->put(SERVER_PARENT_ID, value);
-    MarkDirty();
+    kernel_->mark_dirty(&dir()->kernel()->dirty_metahandles);
   }
 }
 
 bool ModelNeutralMutableEntry::PutIsUnsynced(bool value) {
   DCHECK(kernel_);
+  base_write_transaction_->TrackChangesTo(kernel_);
   if (kernel_->ref(IS_UNSYNCED) != value) {
-  base_write_transaction_->TrackChangesTo(kernel_);
     MetahandleSet* index = &dir()->kernel()->unsynced_metahandles;
 
     ScopedKernelLock lock(dir());
@@ -183,15 +184,15 @@
       }
     }
     kernel_->put(IS_UNSYNCED, value);
-    MarkDirty();
+    kernel_->mark_dirty(&dir()->kernel()->dirty_metahandles);
   }
   return true;
 }
 
 bool ModelNeutralMutableEntry::PutIsUnappliedUpdate(bool value) {
   DCHECK(kernel_);
+  base_write_transaction_->TrackChangesTo(kernel_);
   if (kernel_->ref(IS_UNAPPLIED_UPDATE) != value) {
-    base_write_transaction_->TrackChangesTo(kernel_);
     // Use kernel_->GetServerModelType() instead of
     // GetServerModelType() as we may trigger some DCHECKs in the
     // latter.
@@ -215,27 +216,28 @@
       }
     }
     kernel_->put(IS_UNAPPLIED_UPDATE, value);
-    MarkDirty();
+    kernel_->mark_dirty(&dir()->kernel()->dirty_metahandles);
   }
   return true;
 }
 
 void ModelNeutralMutableEntry::PutServerIsDir(bool value) {
   DCHECK(kernel_);
-  if (kernel_->ref(SERVER_IS_DIR) != value) {
-    base_write_transaction_->TrackChangesTo(kernel_);
+  base_write_transaction_->TrackChangesTo(kernel_);
+  bool old_value = kernel_->ref(SERVER_IS_DIR);
+  if (old_value != value) {
     kernel_->put(SERVER_IS_DIR, value);
-    MarkDirty();
+    kernel_->mark_dirty(GetDirtyIndexHelper());
   }
 }
 
 void ModelNeutralMutableEntry::PutServerIsDel(bool value) {
   DCHECK(kernel_);
+  base_write_transaction_->TrackChangesTo(kernel_);
   bool old_value = kernel_->ref(SERVER_IS_DEL);
   if (old_value != value) {
-    base_write_transaction_->TrackChangesTo(kernel_);
     kernel_->put(SERVER_IS_DEL, value);
-    MarkDirty();
+    kernel_->mark_dirty(GetDirtyIndexHelper());
   }
 
   if (!value || kernel_->ref(IS_UNAPPLIED_UPDATE)) {
@@ -257,10 +259,11 @@
 void ModelNeutralMutableEntry::PutServerNonUniqueName(
     const std::string& value) {
   DCHECK(kernel_);
+  base_write_transaction_->TrackChangesTo(kernel_);
+
   if (kernel_->ref(SERVER_NON_UNIQUE_NAME) != value) {
-    base_write_transaction_->TrackChangesTo(kernel_);
     kernel_->put(SERVER_NON_UNIQUE_NAME, value);
-    MarkDirty();
+    kernel_->mark_dirty(&dir()->kernel()->dirty_metahandles);
   }
 }
 
@@ -280,7 +283,7 @@
   dir()->kernel()->server_tags_map.erase(
       kernel_->ref(UNIQUE_SERVER_TAG));
   kernel_->put(UNIQUE_SERVER_TAG, new_tag);
-  MarkDirty();
+  kernel_->mark_dirty(&dir()->kernel()->dirty_metahandles);
   if (!new_tag.empty()) {
     dir()->kernel()->server_tags_map[new_tag] = kernel_;
   }
@@ -304,7 +307,7 @@
   dir()->kernel()->client_tags_map.erase(
       kernel_->ref(UNIQUE_CLIENT_TAG));
   kernel_->put(UNIQUE_CLIENT_TAG, new_tag);
-  MarkDirty();
+  kernel_->mark_dirty(&dir()->kernel()->dirty_metahandles);
   if (!new_tag.empty()) {
     dir()->kernel()->client_tags_map[new_tag] = kernel_;
   }
@@ -319,8 +322,6 @@
     NOTREACHED();
     return;
   }
-
-  //TODO(stanisc): Does this need a call to TrackChangesTo?
 
   if (!kernel_->ref(UNIQUE_BOOKMARK_TAG).empty() &&
       tag != kernel_->ref(UNIQUE_BOOKMARK_TAG)) {
@@ -334,18 +335,18 @@
   }
 
   kernel_->put(UNIQUE_BOOKMARK_TAG, tag);
-  MarkDirty();
+  kernel_->mark_dirty(&dir()->kernel()->dirty_metahandles);
 }
 
 void ModelNeutralMutableEntry::PutServerSpecifics(
     const sync_pb::EntitySpecifics& value) {
   DCHECK(kernel_);
   CHECK(!value.password().has_client_only_encrypted_data());
+  base_write_transaction_->TrackChangesTo(kernel_);
   // TODO(ncarter): This is unfortunately heavyweight.  Can we do
   // better?
   const std::string& serialized_value = value.SerializeAsString();
   if (serialized_value != kernel_->ref(SERVER_SPECIFICS).SerializeAsString()) {
-    base_write_transaction_->TrackChangesTo(kernel_);
     if (kernel_->ref(IS_UNAPPLIED_UPDATE)) {
       // Remove ourselves from unapplied_update_metahandles with our
       // old server type.
@@ -364,7 +365,7 @@
     } else {
       kernel_->put(SERVER_SPECIFICS, value);
     }
-    MarkDirty();
+    kernel_->mark_dirty(&dir()->kernel()->dirty_metahandles);
 
     if (kernel_->ref(IS_UNAPPLIED_UPDATE)) {
       // Add ourselves back into unapplied_update_metahandles with our
@@ -381,12 +382,12 @@
     const sync_pb::EntitySpecifics& value) {
   DCHECK(kernel_);
   CHECK(!value.password().has_client_only_encrypted_data());
+  base_write_transaction_->TrackChangesTo(kernel_);
   // TODO(ncarter): This is unfortunately heavyweight.  Can we do
   // better?
   const std::string& serialized_value = value.SerializeAsString();
   if (serialized_value !=
       kernel_->ref(BASE_SERVER_SPECIFICS).SerializeAsString()) {
-    base_write_transaction_->TrackChangesTo(kernel_);
     // Check for potential sharing - BASE_SERVER_SPECIFICS is often
     // copied from SERVER_SPECIFICS.
     if (serialized_value ==
@@ -395,30 +396,30 @@
     } else {
       kernel_->put(BASE_SERVER_SPECIFICS, value);
     }
-    MarkDirty();
+    kernel_->mark_dirty(&dir()->kernel()->dirty_metahandles);
   }
 }
 
 void ModelNeutralMutableEntry::PutServerUniquePosition(
     const UniquePosition& value) {
   DCHECK(kernel_);
+  base_write_transaction_->TrackChangesTo(kernel_);
   if(!kernel_->ref(SERVER_UNIQUE_POSITION).Equals(value)) {
-    base_write_transaction_->TrackChangesTo(kernel_);
     // We should never overwrite a valid position with an invalid one.
     DCHECK(value.IsValid());
     ScopedKernelLock lock(dir());
     kernel_->put(SERVER_UNIQUE_POSITION, value);
-    MarkDirty();
+    kernel_->mark_dirty(&dir()->kernel()->dirty_metahandles);
   }
 }
 
 void ModelNeutralMutableEntry::PutServerAttachmentMetadata(
     const sync_pb::AttachmentMetadata& value) {
   DCHECK(kernel_);
+  base_write_transaction_->TrackChangesTo(kernel_);
   const std::string& serialized_value = value.SerializeAsString();
   if (serialized_value !=
       kernel_->ref(SERVER_ATTACHMENT_METADATA).SerializeAsString()) {
-    base_write_transaction_->TrackChangesTo(kernel_);
     // Check for potential sharing - SERVER_ATTACHMENT_METADATA is often
     // copied from ATTACHMENT_METADATA.
     if (serialized_value ==
@@ -427,7 +428,7 @@
     } else {
       kernel_->put(SERVER_ATTACHMENT_METADATA, value);
     }
-    MarkDirty();
+    kernel_->mark_dirty(&dir()->kernel()->dirty_metahandles);
   }
 }
 
@@ -443,19 +444,20 @@
 void ModelNeutralMutableEntry::PutParentIdPropertyOnly(const Id& parent_id) {
   base_write_transaction_->TrackChangesTo(kernel_);
   dir()->ReindexParentId(base_write_transaction(), kernel_, parent_id);
-  MarkDirty();
+  kernel_->mark_dirty(&dir()->kernel()->dirty_metahandles);
 }
 
 void ModelNeutralMutableEntry::UpdateTransactionVersion(int64 value) {
+  ScopedKernelLock lock(dir());
   kernel_->put(TRANSACTION_VERSION, value);
-  MarkDirty();
+  kernel_->mark_dirty(&(dir()->kernel()->dirty_metahandles));
 }
 
 ModelNeutralMutableEntry::ModelNeutralMutableEntry(BaseWriteTransaction* trans)
   : Entry(trans), base_write_transaction_(trans) {}
 
-void ModelNeutralMutableEntry::MarkDirty() {
-  kernel_->mark_dirty(&dir()->kernel()->dirty_metahandles);
+MetahandleSet* ModelNeutralMutableEntry::GetDirtyIndexHelper() {
+  return &dir()->kernel()->dirty_metahandles;
 }
 
 }  // namespace syncable