Communicate MT changes to impl-only scroll offset animations

cc/animation/animation_host.cc

 // Copyright 2015 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.
 
 #include "cc/animation/animation_host.h"
 
 #include <algorithm>
 
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/trace_event/trace_event.h"
 #include "base/trace_event/trace_event_argument.h"
 #include "cc/animation/animation_delegate.h"
 #include "cc/animation/animation_events.h"
 #include "cc/animation/animation_id_provider.h"
 #include "cc/animation/animation_player.h"
 #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"
 
 namespace cc {
 
 std::unique_ptr<AnimationHost> AnimationHost::Create(
     ThreadInstance thread_instance) {
   return base::WrapUnique(new AnimationHost(thread_instance));
 }
 
 AnimationHost::AnimationHost(ThreadInstance thread_instance)
     : mutator_host_client_(nullptr),
       thread_instance_(thread_instance),
       supports_scroll_animations_(false),
       animation_waiting_for_deletion_(false) {
   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() {
   scroll_offset_animations_impl_ = nullptr;
 
   ClearTimelines();
   DCHECK(!mutator_host_client());
   DCHECK(element_to_animations_map_.empty());
 }
 
@@ skipping 92 matching lines @@
 void AnimationHost::SetNeedsCommit() {
   DCHECK(mutator_host_client_);
   mutator_host_client_->SetMutatorsNeedCommit();
 }
 
 void AnimationHost::SetNeedsRebuildPropertyTrees() {
   DCHECK(mutator_host_client_);
   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;
 }
 
 void AnimationHost::PushTimelinesToImplThread(AnimationHost* host_impl) const {
   for (auto& kv : id_to_timeline_map_) {
     auto& timeline = kv.second;
     AnimationTimeline* timeline_impl =
         host_impl->GetTimelineById(timeline->id());
     if (timeline_impl)
       continue;
@@ skipping 12 matching lines @@
     auto& timeline_impl = it->second;
     if (timeline_impl->is_impl_only() || GetTimelineById(timeline_impl->id())) {
       ++it;
     } else {
       host_impl->EraseTimeline(it->second);
       it = timelines_impl.erase(it);
     }
   }
 }
 
-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 =
         host_impl->GetTimelineById(timeline->id());
     if (timeline_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 =
         host_impl->GetElementAnimationsForElementId(kv.first);
     if (element_animations_impl)
       element_animations->PushPropertiesTo(std::move(element_animations_impl));
   }
 }
 
 scoped_refptr<ElementAnimations>
@@ skipping 318 matching lines @@
 bool AnimationHost::ImplOnlyScrollAnimationUpdateTarget(
     ElementId element_id,
     const gfx::Vector2dF& scroll_delta,
     const gfx::ScrollOffset& max_scroll_offset,
     base::TimeTicks frame_monotonic_time) {
   DCHECK(scroll_offset_animations_impl_);
   return scroll_offset_animations_impl_->ScrollAnimationUpdateTarget(
       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);
 }
 
 void AnimationHost::DidActivateElementAnimations(
     ElementAnimations* element_animations) {
   DCHECK(element_animations->element_id());
   active_element_to_animations_map_[element_animations->element_id()] =
       element_animations;
@@ skipping 27 matching lines @@
 const AnimationHost::ElementToAnimationsMap&
 AnimationHost::all_element_animations_for_testing() const {
   return element_to_animations_map_;
 }
 
 void AnimationHost::OnAnimationWaitingForDeletion() {
   animation_waiting_for_deletion_ = true;
 }
 
 }  // namespace cc