cc: Fix for synced property main thread updates with MFBA.

cc/base/synced_property.h

Index: cc/base/synced_property.h
diff --git a/cc/base/synced_property.h b/cc/base/synced_property.h
index 7d1e563343ab8553c26930ae690a8b18de929787..5dfc4508fc6fdbc03644dd6668b2f1787de17487 100644
--- a/cc/base/synced_property.h
+++ b/cc/base/synced_property.h
@@ -57,19 +57,20 @@ class SyncedProperty : public base::RefCounted<SyncedProperty<T>> {
   // Returns the latest active tree delta and also makes a note that this value
   // was sent to the main thread.
   typename T::ValueType PullDeltaForMainThread() {
-    sent_delta_ = active_delta_;
-    return active_delta_.get();
+    sent_delta_ = PendingDelta();
+    return sent_delta_.get();
   }
 
   // Push the latest value from the main thread onto pending tree-associated
   // state.  Returns true if this had any effect.
   bool PushFromMainThread(typename T::ValueType main_thread_value) {
-    if (pending_base_.get() == main_thread_value)
-      return false;
+    bool changed = pending_base_.get() != main_thread_value;
 
+    sent_delta_for_pending_tree_ = sent_delta_;
+    sent_delta_ = T::Identity();
     pending_base_ = T(main_thread_value);
 
-    return true;
+    return changed;
   }
 
   // Push the value associated with the pending tree to be the active base
@@ -77,24 +78,24 @@ class SyncedProperty : public base::RefCounted<SyncedProperty<T>> {
   // delta (which will make the delta zero at steady state, or make it contain
   // only the difference since the last send).
   bool PushPendingToActive() {
-    if (active_base_.get() == pending_base_.get() &&
-        sent_delta_.get() == T::Identity().get())
-      return false;
+    bool changed = active_base_.get() != pending_base_.get() ||
+                   active_delta_.get() != PendingDelta().get();
 
     active_base_ = pending_base_;
     active_delta_ = PendingDelta();
-    sent_delta_ = T::Identity();
+    sent_delta_for_pending_tree_ = T::Identity();
     clobber_active_value_ = false;
 
-    return true;
+    return changed;
   }
 
   // This simulates the consequences of the sent value getting committed and
   // activated.  The value sent to the main thread ends up combined with the
   // active value, and the sent_delta is subtracted from the delta.
   void AbortCommit() {
+    pending_base_ = pending_base_.Combine(sent_delta_);
     active_base_ = active_base_.Combine(sent_delta_);
-    active_delta_ = PendingDelta();
+    active_delta_ = active_delta_.InverseCombine(sent_delta_);
     sent_delta_ = T::Identity();
   }
 
@@ -109,7 +110,7 @@ class SyncedProperty : public base::RefCounted<SyncedProperty<T>> {
   T PendingDelta() const {
     if (clobber_active_value_)
       return T::Identity();
-    return active_delta_.InverseCombine(sent_delta_);
+    return active_delta_.InverseCombine(sent_delta_for_pending_tree_);
   }
 
   void set_clobber_active_value() { clobber_active_value_ = true; }
@@ -120,11 +121,14 @@ class SyncedProperty : public base::RefCounted<SyncedProperty<T>> {
   T pending_base_;
   // Value last committed to the active tree (on the last activation).
   T active_base_;
-  // The difference between the active_base_ and the user-perceived value.
+  // The difference between |active_base_| and the user-perceived value.
   T active_delta_;
   // The value sent to the main thread (on the last BeginFrame); this is always
-  // identity outside of the BeginFrame-to-activation interval.
+  // identity outside of the BeginFrame-to-(aborted)commit interval.
   T sent_delta_;

[aelias_OOO_until_Jul13, 2016/06/09 00:46:13]

How about switching the names to: "reflected_delta_in_main_tree_" and
"reflected_delta_in_pending_tree_"?  I think this would better express that
these are tracking duplicated quantities for the purpose of avoiding
double-application.

[sunnyps, 2016/06/09 01:08:56]

Done.

+  // The value that was sent to the main thread for BeginFrame for the current
+  // pending tree.

[aelias_OOO_until_Jul13, 2016/06/09 00:46:13]

Please add comment: "This is always identity outside of the commit-to-activation
interval."

[sunnyps, 2016/06/09 01:08:56]

Done. Also changed some of the other comments.

+  T sent_delta_for_pending_tree_;
   // When true the pending delta is always identity so that it does not change
   // and will clobber the active value on push.
   bool clobber_active_value_;