4 files changed, 68 insertions, 54 deletions

diff --git a/sync/internal_api/public/write_transaction.h b/sync/internal_api/public/write_transaction.h
index df27cad..8c11570 100644
--- a/sync/internal_api/public/write_transaction.h
+++ b/sync/internal_api/public/write_transaction.h
@@ -30,6 +30,12 @@ class WriteTransaction : public BaseTransaction {
   // Start a new read/write transaction.
   WriteTransaction(const tracked_objects::Location& from_here,
                    UserShare* share);
+  // |transaction_version| stores updated model and nodes version if model
+  // is changed by the transaction, or syncer::syncable::kInvalidTransaction
+  // if not after transaction is closed. This constructor is used for model
+  // types that support embassy data.
+  WriteTransaction(const tracked_objects::Location& from_here,
+                   UserShare* share, int64* transaction_version);
   virtual ~WriteTransaction();
 
   // Provide access to the syncable transaction from the API WriteNode.
@@ -40,7 +46,7 @@ class WriteTransaction : public BaseTransaction {
   WriteTransaction() {}
 
   void SetTransaction(syncable::WriteTransaction* trans) {
-      transaction_ = trans;
+    transaction_ = trans;
   }
 
  private:
diff --git a/sync/internal_api/sync_manager_impl.cc b/sync/internal_api/sync_manager_impl.cc
index 37c1a01..f7abea4 100644
--- a/sync/internal_api/sync_manager_impl.cc
+++ b/sync/internal_api/sync_manager_impl.cc
@@ -270,14 +270,6 @@ bool SyncManagerImpl::VisiblePropertiesDiffer(
   return false;
 }
 
-bool SyncManagerImpl::ChangeBuffersAreEmpty() {
-  for (int i = 0; i < MODEL_TYPE_COUNT; ++i) {
-    if (!change_buffers_[i].IsEmpty())
-      return false;
-  }
-  return true;
-}
-
 void SyncManagerImpl::ThrowUnrecoverableError() {
   DCHECK(thread_checker_.CalledOnValidThread());
   ReadTransaction trans(FROM_HERE, GetUserShare());
@@ -788,7 +780,7 @@ SyncManagerImpl::HandleTransactionEndingChangeEvent(
   // This notification happens immediately before a syncable WriteTransaction
   // falls out of scope. It happens while the channel mutex is still held,
   // and while the transaction mutex is held, so it cannot be re-entrant.
-  if (!change_delegate_ || ChangeBuffersAreEmpty())
+  if (!change_delegate_ || change_records_.empty())
     return ModelTypeSet();
 
   // This will continue the WriteTransaction using a read only wrapper.
@@ -798,40 +790,28 @@ SyncManagerImpl::HandleTransactionEndingChangeEvent(
   ReadTransaction read_trans(GetUserShare(), trans);
 
   ModelTypeSet models_with_changes;
-  for (int i = FIRST_REAL_MODEL_TYPE; i < MODEL_TYPE_COUNT; ++i) {
-    const ModelType type = ModelTypeFromInt(i);
-    if (change_buffers_[type].IsEmpty())
-      continue;
-
-    ImmutableChangeRecordList ordered_changes;
-    // TODO(akalin): Propagate up the error further (see
-    // http://crbug.com/100907).
-    CHECK(change_buffers_[type].GetAllChangesInTreeOrder(&read_trans,
-                                                         &ordered_changes));
-    if (!ordered_changes.Get().empty()) {
-      // Increment transaction version so that change processor can read
-      // updated value and set it in native model after changes are applied.
-      trans->directory()->IncrementTransactionVersion(type);
-
-      change_delegate_->
-          OnChangesApplied(type,
-                           trans->directory()->GetTransactionVersion(type),
-                           &read_trans, ordered_changes);
-      change_observer_.Call(FROM_HERE,
-          &SyncManager::ChangeObserver::OnChangesApplied,
-          type, write_transaction_info.Get().id, ordered_changes);
-      models_with_changes.Put(type);
-    }
-    change_buffers_[i].Clear();
+  for (ChangeRecordMap::const_iterator it = change_records_.begin();
+      it != change_records_.end(); ++it) {
+    DCHECK(!it->second.Get().empty());
+    ModelType type = ModelTypeFromInt(it->first);
+    change_delegate_->
+        OnChangesApplied(type, trans->directory()->GetTransactionVersion(type),
+                         &read_trans, it->second);
+    change_observer_.Call(FROM_HERE,
+        &SyncManager::ChangeObserver::OnChangesApplied,
+        type, write_transaction_info.Get().id, it->second);
+    models_with_changes.Put(type);
   }
+  change_records_.clear();
   return models_with_changes;
 }
 
 void SyncManagerImpl::HandleCalculateChangesChangeEventFromSyncApi(
     const ImmutableWriteTransactionInfo& write_transaction_info,
-    syncable::BaseTransaction* trans) {
+    syncable::BaseTransaction* trans,
+    std::vector<int64>* entries_changed) {
   // We have been notified about a user action changing a sync model.
-  LOG_IF(WARNING, !ChangeBuffersAreEmpty()) <<
+  LOG_IF(WARNING, !change_records_.empty()) <<
       "CALCULATE_CHANGES called with unapplied old changes.";
 
   // The mutated model type, or UNSPECIFIED if nothing was mutated.
@@ -855,6 +835,7 @@ void SyncManagerImpl::HandleCalculateChangesChangeEventFromSyncApi(
     // Found real mutation.
     if (model_type != UNSPECIFIED) {
       mutated_model_types.Put(model_type);
+      entries_changed->push_back(it->second.mutated.ref(syncable::META_HANDLE));
     }
   }
 
@@ -903,12 +884,15 @@ void SyncManagerImpl::SetExtraChangeRecordData(int64 id,
 
 void SyncManagerImpl::HandleCalculateChangesChangeEventFromSyncer(
     const ImmutableWriteTransactionInfo& write_transaction_info,
-    syncable::BaseTransaction* trans) {
+    syncable::BaseTransaction* trans,
+    std::vector<int64>* entries_changed) {
   // We only expect one notification per sync step, so change_buffers_ should
   // contain no pending entries.
-  LOG_IF(WARNING, !ChangeBuffersAreEmpty()) <<
+  LOG_IF(WARNING, !change_records_.empty()) <<
       "CALCULATE_CHANGES called with unapplied old changes.";
 
+  ChangeReorderBuffer change_buffers[MODEL_TYPE_COUNT];
+
   Cryptographer* crypto = directory()->GetCryptographer(trans);
   const syncable::ImmutableEntryKernelMutationMap& mutations =
       write_transaction_info.Get().mutations;
@@ -925,18 +909,31 @@ void SyncManagerImpl::HandleCalculateChangesChangeEventFromSyncer(
 
     int64 handle = it->first;
     if (exists_now && !existed_before)
-      change_buffers_[type].PushAddedItem(handle);
+      change_buffers[type].PushAddedItem(handle);
     else if (!exists_now && existed_before)
-      change_buffers_[type].PushDeletedItem(handle);
+      change_buffers[type].PushDeletedItem(handle);
     else if (exists_now && existed_before &&
              VisiblePropertiesDiffer(it->second, crypto)) {
-      change_buffers_[type].PushUpdatedItem(
+      change_buffers[type].PushUpdatedItem(
           handle, VisiblePositionsDiffer(it->second));
     }
 
-    SetExtraChangeRecordData(handle, type, &change_buffers_[type], crypto,
+    SetExtraChangeRecordData(handle, type, &change_buffers[type], crypto,
                              it->second.original, existed_before, exists_now);
   }
+
+  ReadTransaction read_trans(GetUserShare(), trans);
+  for (int i = FIRST_REAL_MODEL_TYPE; i < MODEL_TYPE_COUNT; ++i) {
+    if (!change_buffers[i].IsEmpty()) {
+      if (change_buffers[i].GetAllChangesInTreeOrder(&read_trans,
+                                                     &(change_records_[i]))) {
+        for (size_t j = 0; j < change_records_[i].Get().size(); ++j)
+          entries_changed->push_back((change_records_[i].Get())[j].id);
+      }
+      if (change_records_[i].Get().empty())
+        change_records_.erase(i);
+    }
+  }
 }
 
 TimeDelta SyncManagerImpl::GetNudgeDelayTimeDelta(
diff --git a/sync/internal_api/sync_manager_impl.h b/sync/internal_api/sync_manager_impl.h
index 6037e8c..6accdab 100644
--- a/sync/internal_api/sync_manager_impl.h
+++ b/sync/internal_api/sync_manager_impl.h
@@ -165,10 +165,12 @@ class SyncManagerImpl :
       syncable::BaseTransaction* trans) OVERRIDE;
   virtual void HandleCalculateChangesChangeEventFromSyncApi(
       const syncable::ImmutableWriteTransactionInfo& write_transaction_info,
-      syncable::BaseTransaction* trans) OVERRIDE;
+      syncable::BaseTransaction* trans,
+      std::vector<int64>* entries_changed) OVERRIDE;
   virtual void HandleCalculateChangesChangeEventFromSyncer(
       const syncable::ImmutableWriteTransactionInfo& write_transaction_info,
-      syncable::BaseTransaction* trans) OVERRIDE;
+      syncable::BaseTransaction* trans,
+      std::vector<int64>* entries_changed) OVERRIDE;
 
   // InvalidationHandler implementation.
   virtual void OnInvalidatorStateChange(InvalidatorState state) OVERRIDE;
@@ -227,8 +229,6 @@ class SyncManagerImpl :
       const syncable::EntryKernelMutation& mutation,
       Cryptographer* cryptographer) const;
 
-  bool ChangeBuffersAreEmpty();
-
   // Open the directory named with username_for_share
   bool OpenDirectory();
 
@@ -335,13 +335,14 @@ class SyncManagerImpl :
   // sync components.
   AllStatus allstatus_;
 
-  // Each element of this array is a store of change records produced by
-  // HandleChangeEvent during the CALCULATE_CHANGES step.  The changes are
-  // segregated by model type, and are stored here to be processed and
-  // forwarded to the observer slightly later, at the TRANSACTION_ENDING
-  // step by HandleTransactionEndingChangeEvent. The list is cleared in the
-  // TRANSACTION_COMPLETE step by HandleTransactionCompleteChangeEvent.
-  ChangeReorderBuffer change_buffers_[MODEL_TYPE_COUNT];
+  // Each element of this map is a store of change records produced by
+  // HandleChangeEventFromSyncer during the CALCULATE_CHANGES step. The changes
+  // are grouped by model type, and are stored here in tree order to be
+  // forwarded to the observer slightly later, at the TRANSACTION_ENDING step
+  // by HandleTransactionEndingChangeEvent. The list is cleared after observer
+  // finishes processing.
+  typedef std::map<int, ImmutableChangeRecordList> ChangeRecordMap;
+  ChangeRecordMap change_records_;
 
   SyncManager::ChangeDelegate* change_delegate_;
 
diff --git a/sync/internal_api/write_transaction.cc b/sync/internal_api/write_transaction.cc
index fe11adf..cf0f121 100644
--- a/sync/internal_api/write_transaction.cc
+++ b/sync/internal_api/write_transaction.cc
@@ -18,6 +18,16 @@ WriteTransaction::WriteTransaction(const tracked_objects::Location& from_here,
                                                 share->directory.get());
 }
 
+WriteTransaction::WriteTransaction(const tracked_objects::Location& from_here,
+                                   UserShare* share,
+                                   int64* new_model_version)
+    : BaseTransaction(share),
+      transaction_(NULL) {
+  transaction_ = new syncable::WriteTransaction(from_here,
+                                                share->directory.get(),
+                                                new_model_version);
+}
+
 WriteTransaction::~WriteTransaction() {
   delete transaction_;
 }