From e4e0bcc0f7cc2ba281200c7d1907c64fba9d4f3e Mon Sep 17 00:00:00 2001
From: loyso <loyso@chromium.org>
Date: Sun, 20 Dec 2015 22:37:23 -0800
Subject: CC Animations: Fix the case when main-thread player changes layer_id.

We didn't compare layer ids.

BUG=394777
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Review URL: https://codereview.chromium.org/1539233002

Cr-Commit-Position: refs/heads/master@{#366338}
---
 cc/animation/animation_player.cc           | 11 ++++------
 cc/animation/animation_player_unittest.cc  | 34 ++++++++++++++++++++++++++++++
 cc/test/animation_timelines_test_common.cc |  9 +++++++-
 cc/test/animation_timelines_test_common.h  |  6 +++++-
 4 files changed, 51 insertions(+), 9 deletions(-)

diff --git a/cc/animation/animation_player.cc b/cc/animation/animation_player.cc
index c237aa7..dee19ea 100644
--- a/cc/animation/animation_player.cc
+++ b/cc/animation/animation_player.cc
@@ -154,15 +154,12 @@ void AnimationPlayer::RemoveAnimation(int animation_id) {
 }
 
 void AnimationPlayer::PushPropertiesTo(AnimationPlayer* player_impl) {
-  if (!element_animations_) {
-    if (player_impl->element_animations())
+  if (layer_id_ != player_impl->layer_id()) {
+    if (player_impl->layer_id())
       player_impl->DetachLayer();
-    return;
+    if (layer_id_)
+      player_impl->AttachLayer(layer_id_);
   }
-
-  DCHECK(layer_id_);
-  if (!player_impl->element_animations())
-    player_impl->AttachLayer(layer_id_);
 }
 
 void AnimationPlayer::NotifyAnimationStarted(
diff --git a/cc/animation/animation_player_unittest.cc b/cc/animation/animation_player_unittest.cc
index 466964f..d8030b3 100644
--- a/cc/animation/animation_player_unittest.cc
+++ b/cc/animation/animation_player_unittest.cc
@@ -332,5 +332,39 @@ TEST_F(AnimationPlayerTest, AddRemoveAnimationCausesSetNeedsCommit) {
   client_.set_mutators_need_commit(false);
 }
 
+// If main-thread player switches to another layer within one frame then
+// impl-thread player must be switched as well.
+TEST_F(AnimationPlayerTest, SwitchToLayer) {
+  host_->AddAnimationTimeline(timeline_);
+  timeline_->AttachPlayer(player_);
+  player_->AttachLayer(layer_id_);
+
+  host_->PushPropertiesTo(host_impl_);
+
+  GetImplTimelineAndPlayerByID();
+
+  EXPECT_EQ(player_, GetPlayerForLayerId(layer_id_));
+  EXPECT_TRUE(player_->element_animations());
+  EXPECT_EQ(player_->layer_id(), layer_id_);
+
+  EXPECT_EQ(player_impl_, GetImplPlayerForLayerId(layer_id_));
+  EXPECT_TRUE(player_impl_->element_animations());
+  EXPECT_EQ(player_impl_->layer_id(), layer_id_);
+
+  const int new_layer_id = NextTestLayerId();
+  player_->DetachLayer();
+  player_->AttachLayer(new_layer_id);
+
+  EXPECT_EQ(player_, GetPlayerForLayerId(new_layer_id));
+  EXPECT_TRUE(player_->element_animations());
+  EXPECT_EQ(player_->layer_id(), new_layer_id);
+
+  host_->PushPropertiesTo(host_impl_);
+
+  EXPECT_EQ(player_impl_, GetImplPlayerForLayerId(new_layer_id));
+  EXPECT_TRUE(player_impl_->element_animations());
+  EXPECT_EQ(player_impl_->layer_id(), new_layer_id);
+}
+
 }  // namespace
 }  // namespace cc
diff --git a/cc/test/animation_timelines_test_common.cc b/cc/test/animation_timelines_test_common.cc
index 9212da6..6df5bd1 100644
--- a/cc/test/animation_timelines_test_common.cc
+++ b/cc/test/animation_timelines_test_common.cc
@@ -197,9 +197,11 @@ AnimationTimelinesTest::AnimationTimelinesTest()
       host_impl_(nullptr),
       timeline_id_(AnimationIdProvider::NextTimelineId()),
       player_id_(AnimationIdProvider::NextPlayerId()),
-      layer_id_(1) {
+      next_test_layer_id_(0) {
   host_ = client_.host();
   host_impl_ = client_impl_.host();
+
+  layer_id_ = NextTestLayerId();
 }
 
 AnimationTimelinesTest::~AnimationTimelinesTest() {
@@ -258,4 +260,9 @@ AnimationPlayer* AnimationTimelinesTest::GetImplPlayerForLayerId(int layer_id) {
                             : nullptr;
 }
 
+int AnimationTimelinesTest::NextTestLayerId() {
+  next_test_layer_id_++;
+  return next_test_layer_id_;
+}
+
 }  // namespace cc
diff --git a/cc/test/animation_timelines_test_common.h b/cc/test/animation_timelines_test_common.h
index 8b3996b..518929d6 100644
--- a/cc/test/animation_timelines_test_common.h
+++ b/cc/test/animation_timelines_test_common.h
@@ -174,6 +174,8 @@ class AnimationTimelinesTest : public testing::Test {
   AnimationPlayer* GetPlayerForLayerId(int layer_id);
   AnimationPlayer* GetImplPlayerForLayerId(int layer_id);
 
+  int NextTestLayerId();
+
   TestHostClient client_;
   TestHostClient client_impl_;
 
@@ -182,7 +184,9 @@ class AnimationTimelinesTest : public testing::Test {
 
   const int timeline_id_;
   const int player_id_;
-  const int layer_id_;
+  int layer_id_;
+
+  int next_test_layer_id_;
 
   scoped_refptr<AnimationTimeline> timeline_;
   scoped_refptr<AnimationPlayer> player_;
-- 
cgit v1.1