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

cc/base/synced_property.h

 // Copyright 2014 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 CC_BASE_SYNCED_PROPERTY_H_
 #define CC_BASE_SYNCED_PROPERTY_H_
 
 #include "base/memory/ref_counted.h"
 
 namespace cc {
@@ skipping 39 matching lines @@
     return true;
   }
 
   // Returns the difference between the last value that was committed and
   // activated from the main thread, and the current total value.
   typename T::ValueType Delta() const { return active_delta_.get(); }
 
   // 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
   // value.  As part of this, subtract the last sent value from the active tree
   // 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();
   }
 
   // Values as last pushed to the pending or active tree respectively, with no
   // impl-thread delta applied.
   typename T::ValueType PendingBase() const { return pending_base_.get(); }
   typename T::ValueType ActiveBase() const { return active_base_.get(); }
 
   // The new delta we would use if we decide to activate now.  This delta
   // excludes the amount that we expect the main thread to reflect back at the
   // impl thread during the commit.
   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; }
   bool clobber_active_value() const { return clobber_active_value_; }
 
  private:
   // Value last committed to the pending tree.
   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_;
 
   friend class base::RefCounted<SyncedProperty<T>>;
   ~SyncedProperty() {}
 };
 
 // SyncedProperty's delta-based conflict resolution logic makes sense for any
 // mathematical group.  In practice, there are two that are useful:
@@ skipping 41 matching lines @@
     return ScaleGroup(value_ / p.value_);
   }
 
  private:
   float value_;
 };
 
 }  // namespace cc
 
 #endif  // CC_BASE_SYNCED_PROPERTY_H_