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

sync/syncable/mutable_entry.cc

Index: sync/syncable/mutable_entry.cc
diff --git a/sync/syncable/mutable_entry.cc b/sync/syncable/mutable_entry.cc
index 6c13261ba0463d805526d3ba8aef50ae3bad6d76..8e33a67a98f1345cf4729caf8e2dc4c5b0870488 100644
--- a/sync/syncable/mutable_entry.cc
+++ b/sync/syncable/mutable_entry.cc
@@ -122,36 +122,36 @@
 
 void MutableEntry::PutLocalExternalId(int64 value) {
   DCHECK(kernel_);
+  write_transaction()->TrackChangesTo(kernel_);
   if (kernel_->ref(LOCAL_EXTERNAL_ID) != value) {
-    write_transaction()->TrackChangesTo(kernel_);
     ScopedKernelLock lock(dir());
     kernel_->put(LOCAL_EXTERNAL_ID, value);
-    MarkDirty();
+    kernel_->mark_dirty(&dir()->kernel()->dirty_metahandles);
   }
 }
 
 void MutableEntry::PutMtime(base::Time value) {
   DCHECK(kernel_);
+  write_transaction()->TrackChangesTo(kernel_);
   if (kernel_->ref(MTIME) != value) {
-    write_transaction()->TrackChangesTo(kernel_);
     kernel_->put(MTIME, value);
-    MarkDirty();
+    kernel_->mark_dirty(&dir()->kernel()->dirty_metahandles);
   }
 }
 
 void MutableEntry::PutCtime(base::Time value) {
   DCHECK(kernel_);
+  write_transaction()->TrackChangesTo(kernel_);
   if (kernel_->ref(CTIME) != value) {
-    write_transaction()->TrackChangesTo(kernel_);
     kernel_->put(CTIME, value);
-    MarkDirty();
+    kernel_->mark_dirty(&dir()->kernel()->dirty_metahandles);
   }
 }
 
 void MutableEntry::PutParentId(const Id& value) {
   DCHECK(kernel_);
+  write_transaction()->TrackChangesTo(kernel_);
   if (kernel_->ref(PARENT_ID) != value) {
-    write_transaction()->TrackChangesTo(kernel_);
     PutParentIdPropertyOnly(value);
     if (!GetIsDel()) {
       if (!PutPredecessor(Id())) {
@@ -164,20 +164,20 @@
 
 void MutableEntry::PutIsDir(bool value) {
   DCHECK(kernel_);
-  if (kernel_->ref(IS_DIR) != value) {
-    write_transaction()->TrackChangesTo(kernel_);
+  write_transaction()->TrackChangesTo(kernel_);
+  bool old_value = kernel_->ref(IS_DIR);
+  if (old_value != value) {
     kernel_->put(IS_DIR, value);
-    MarkDirty();
+    kernel_->mark_dirty(GetDirtyIndexHelper());
   }
 }
 
 void MutableEntry::PutIsDel(bool value) {
   DCHECK(kernel_);
+  write_transaction()->TrackChangesTo(kernel_);
   if (value == kernel_->ref(IS_DEL)) {
     return;
   }
-
-  write_transaction()->TrackChangesTo(kernel_);
   if (value) {
     // If the server never knew about this item and it's deleted then we don't
     // need to keep it around.  Unsetting IS_UNSYNCED will:
@@ -202,27 +202,28 @@
         &dir()->kernel()->parent_child_index);
 
     kernel_->put(IS_DEL, value);
-    MarkDirty();
+    kernel_->mark_dirty(&dir()->kernel()->dirty_metahandles);
   }
 }
 
 void MutableEntry::PutNonUniqueName(const std::string& value) {
   DCHECK(kernel_);
+  write_transaction()->TrackChangesTo(kernel_);
+
   if (kernel_->ref(NON_UNIQUE_NAME) != value) {
-    write_transaction()->TrackChangesTo(kernel_);
     kernel_->put(NON_UNIQUE_NAME, value);
-    MarkDirty();
+    kernel_->mark_dirty(&dir()->kernel()->dirty_metahandles);
   }
 }
 
 void MutableEntry::PutSpecifics(const sync_pb::EntitySpecifics& value) {
   DCHECK(kernel_);
   CHECK(!value.password().has_client_only_encrypted_data());
+  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(SPECIFICS).SerializeAsString()) {
-    write_transaction()->TrackChangesTo(kernel_);
     // Check for potential sharing - SPECIFICS is often
     // copied from SERVER_SPECIFICS.
     if (serialized_value ==
@@ -231,21 +232,21 @@
     } else {
       kernel_->put(SPECIFICS, value);
     }
-    MarkDirty();
+    kernel_->mark_dirty(&dir()->kernel()->dirty_metahandles);
   }
 }
 
 void MutableEntry::PutUniquePosition(const UniquePosition& value) {
   DCHECK(kernel_);
+  write_transaction()->TrackChangesTo(kernel_);
   if(!kernel_->ref(UNIQUE_POSITION).Equals(value)) {
-    write_transaction()->TrackChangesTo(kernel_);
     // We should never overwrite a valid position with an invalid one.
     DCHECK(value.IsValid());
     ScopedKernelLock lock(dir());
     ScopedParentChildIndexUpdater updater(
         lock, kernel_, &dir()->kernel()->parent_child_index);
     kernel_->put(UNIQUE_POSITION, value);
-    MarkDirty();
+    kernel_->mark_dirty(&dir()->kernel()->dirty_metahandles);
   }
 }
 
@@ -257,7 +258,6 @@
     if (!predecessor.good())
       return false;
     dir()->PutPredecessor(kernel_, predecessor.kernel_);
-    DCHECK(kernel_->is_dirty());
   }
   return true;
 }
@@ -265,10 +265,10 @@
 void MutableEntry::PutAttachmentMetadata(
     const sync_pb::AttachmentMetadata& value) {
   DCHECK(kernel_);
+  write_transaction()->TrackChangesTo(kernel_);
   const std::string& serialized_value = value.SerializeAsString();
   if (serialized_value !=
       kernel_->ref(ATTACHMENT_METADATA).SerializeAsString()) {
-    write_transaction()->TrackChangesTo(kernel_);
     dir()->UpdateAttachmentIndex(GetMetahandle(),
                                  kernel_->ref(ATTACHMENT_METADATA), value);
     // Check for potential sharing - ATTACHMENT_METADATA is often
@@ -279,7 +279,7 @@
     } else {
       kernel_->put(ATTACHMENT_METADATA, value);
     }
-    MarkDirty();
+    kernel_->mark_dirty(&dir()->kernel()->dirty_metahandles);
   }
 }
 
@@ -298,7 +298,7 @@
     record->set_is_on_server(true);
   }
   kernel_->put(ATTACHMENT_METADATA, attachment_metadata);
-  MarkDirty();
+  kernel_->mark_dirty(&dir()->kernel()->dirty_metahandles);
   MarkForSyncing(this);
 }