Communicate MT changes to impl-only scroll offset animations

cc/animation/animation_host.cc

Index: cc/animation/animation_host.cc
diff --git a/cc/animation/animation_host.cc b/cc/animation/animation_host.cc
index 66ebaa482f038abfe391f0a740c934022f1587b3..3f811d42c826946b31877503b64ea0d9b5a3d80c 100644
--- a/cc/animation/animation_host.cc
+++ b/cc/animation/animation_host.cc
@@ -17,6 +17,8 @@
 #include "cc/animation/animation_timeline.h"
 #include "cc/animation/element_animations.h"
 #include "cc/animation/scroll_offset_animation_curve.h"
+#include "cc/animation/scroll_offset_animations.h"
+#include "cc/animation/scroll_offset_animations_impl.h"
 #include "cc/animation/timing_function.h"
 #include "ui/gfx/geometry/box_f.h"
 #include "ui/gfx/geometry/scroll_offset.h"
@@ -36,6 +38,9 @@ AnimationHost::AnimationHost(ThreadInstance thread_instance)
   if (thread_instance_ == ThreadInstance::IMPL)
     scroll_offset_animations_impl_ =
         base::WrapUnique(new ScrollOffsetAnimationsImpl(this));
+  else

[loyso (OOO), 2016/05/11 06:25:44]

Code style: Brackets on multiline if else blocks, AFAIR. if {} else {}

[ymalik, 2016/05/12 22:31:25]

Done.

+    scroll_offset_animations_ =
+        base::WrapUnique(new ScrollOffsetAnimations(this));
 }
 
 AnimationHost::~AnimationHost() {
@@ -148,10 +153,11 @@ void AnimationHost::SetNeedsRebuildPropertyTrees() {
   mutator_host_client_->SetMutatorsNeedRebuildPropertyTrees();
 }
 
-void AnimationHost::PushPropertiesTo(AnimationHost* host_impl) {
+void AnimationHost::PushPropertiesTo(AnimationHost* host_impl,
+                                     base::TimeTicks frame_monotonic_time) {
   PushTimelinesToImplThread(host_impl);
   RemoveTimelinesFromImplThread(host_impl);
-  PushPropertiesToImplThread(host_impl);
+  PushPropertiesToImplThread(host_impl, frame_monotonic_time);
   animation_waiting_for_deletion_ = false;
 }
 
@@ -184,8 +190,10 @@ void AnimationHost::RemoveTimelinesFromImplThread(
   }
 }
 
-void AnimationHost::PushPropertiesToImplThread(AnimationHost* host_impl) {
-  // Firstly, sync all players with impl thread to create ElementAnimations.

[loyso (OOO), 2016/05/11 06:25:44]

Firstly, secondly - it meant the constraint on the order of passes. It was a bit
unclear (bad comments), but no need to delete it at all. I propose to extract
each pass as a separate private function with a name. And the comment that
passes can't be reordered needed.

[ymalik, 2016/05/12 22:31:25]

I only see value in extracting things out if they're used from more than one
place. Coming up with good names is difficult and subjective to the reader's
interpretation. I like to PushPropertiesTo and PushPropertiesToImplThread
convention we have for these classes, and adding more functions makes it
confusing IMO. Updated comment to make order more clear.

+void AnimationHost::PushPropertiesToImplThread(
+    AnimationHost* host_impl,
+    base::TimeTicks frame_monotonic_time) {
+  // Sync all players with impl thread to create ElementAnimations.
   for (auto& kv : id_to_timeline_map_) {
     AnimationTimeline* timeline = kv.second.get();
     AnimationTimeline* timeline_impl =
@@ -194,7 +202,13 @@ void AnimationHost::PushPropertiesToImplThread(AnimationHost* host_impl) {
       timeline->PushPropertiesTo(timeline_impl);
   }
 
-  // Secondly, sync properties for created ElementAnimations.
+  // Update the impl-only scroll offset animations. This needs to happen
+  // before the sync below because it may add new animations that will need to
+  // be pushed to the impl thread.
+  scroll_offset_animations_->PushPropertiesTo(
+      host_impl->scroll_offset_animations_impl_.get(), frame_monotonic_time);
+
+  // Sync properties for created ElementAnimations.
   for (auto& kv : element_to_animations_map_) {
     const auto& element_animations = kv.second;
     auto element_animations_impl =
@@ -533,6 +547,11 @@ bool AnimationHost::ImplOnlyScrollAnimationUpdateTarget(
       element_id, scroll_delta, max_scroll_offset, frame_monotonic_time);
 }
 
+ScrollOffsetAnimations& AnimationHost::scroll_offset_animations() const {
+  DCHECK(scroll_offset_animations_);
+  return *scroll_offset_animations_.get();
+}
+
 void AnimationHost::ScrollAnimationAbort(bool needs_completion) {
   DCHECK(scroll_offset_animations_impl_);
   return scroll_offset_animations_impl_->ScrollAnimationAbort(needs_completion);