CC Animations: Fix draw properties invalidation on LAC destruction

cc/trees/layer_tree_host_unittest_animation_timelines.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/trees/layer_tree_host.h"
 
 #include "cc/animation/animation_curve.h"
 #include "cc/animation/animation_host.h"
 #include "cc/animation/animation_id_provider.h"
 #include "cc/animation/animation_player.h"
@@ skipping 980 matching lines @@
  private:
   scoped_refptr<Layer> layer_;
   FakeContentLayerClient client_;
   CompletionEvent completion_;
 };
 
 // An animation finishing during commit can only happen when we have a separate
 // compositor thread.
 MULTI_THREAD_TEST_F(LayerTreeHostTimelinesTestAnimationFinishesDuringCommit);
 
-#if defined(OS_ANDROID)
-#define MAYBE_LayerTreeHostTimelinesTestSetPotentiallyAnimatingOnLacDestruction \
-  DISABLED_LayerTreeHostTimelinesTestSetPotentiallyAnimatingOnLacDestruction
-#else
-#define MAYBE_LayerTreeHostTimelinesTestSetPotentiallyAnimatingOnLacDestruction \
-  LayerTreeHostTimelinesTestSetPotentiallyAnimatingOnLacDestruction
-#endif
-
 // Check that SetTransformIsPotentiallyAnimatingChanged is called
 // if we destroy LayerAnimationController and ElementAnimations.
-class MAYBE_LayerTreeHostTimelinesTestSetPotentiallyAnimatingOnLacDestruction
+class LayerTreeHostTimelinesTestSetPotentiallyAnimatingOnLacDestruction
     : public LayerTreeHostTimelinesTest {
  public:
   void SetupTree() override {
     LayerTreeHostTimelinesTest::SetupTree();
     AttachPlayersToTimeline();
     player_->AttachLayer(layer_tree_host()->root_layer()->id());
     AddAnimatedTransformToPlayer(player_.get(), 1.0, 5, 5);
   }
 
   void BeginTest() override { PostSetNeedsCommitToMainThread(); }
 
   void CommitCompleteOnThread(LayerTreeHostImpl* host_impl) override {
-    if (host_impl->active_tree()->source_frame_number() == 2) {

[Ian Vollick, 2015/10/27 14:08:18]

I'm sorry, it should probably be obvious to me, but I don't see how, if the
current code consistently passes, the old code was flaky and not just wrong all
the time.

Wouldn't the active tree have source_frame_number == 2 after activation, and
wouldn't that also imply that that the active tree's root layer would not be
animating? Could you please add an explanation either in the description of the
patch, or here?

[loyso (OOO), 2015/10/27 22:43:46]

Yes, your observation is correct. It worked only by accident - no were changes
between two CommitCompleteOnThread calls.
 
AFAIU, at CommitCompleteOnThread time LTH tree copied into LTHI pending tree
with draw properties calculated, but pending tree isn't activated and drawn.

[Ian Vollick, 2015/10/28 22:54:42]

Still a little confused (sorry). Could you add a link to a failure?

+    if (host_impl->active_tree()->source_frame_number() == 1) {
       EXPECT_TRUE(host_impl->active_tree());
       EXPECT_TRUE(host_impl->active_tree()->root_layer());
       EXPECT_TRUE(host_impl->pending_tree());
       EXPECT_TRUE(host_impl->pending_tree()->root_layer());
 
+      EXPECT_EQ(2, host_impl->pending_tree()->source_frame_number());
+
       EXPECT_TRUE(host_impl->active_tree()
                       ->root_layer()
                       ->screen_space_transform_is_animating());
       EXPECT_FALSE(host_impl->pending_tree()
                        ->root_layer()
                        ->screen_space_transform_is_animating());
     }
   }
 
-  void WillCommit() override {}
-
   void DidCommit() override { PostSetNeedsCommitToMainThread(); }
 
   void Layout() override {
     if (layer_tree_host()->source_frame_number() == 2) {
       // Destroy player.
       timeline_->DetachPlayer(player_.get());
       player_ = nullptr;
     }
   }
 
   void DrawLayersOnThread(LayerTreeHostImpl* host_impl) override {
     if (host_impl->active_tree()->source_frame_number() >= 2)
       EndTest();
   }
 
   void AfterTest() override {}
 };
 
 MULTI_THREAD_TEST_F(
-    MAYBE_LayerTreeHostTimelinesTestSetPotentiallyAnimatingOnLacDestruction);
+    LayerTreeHostTimelinesTestSetPotentiallyAnimatingOnLacDestruction);
 
 // Check that we invalidate property trees on AnimationPlayer::SetNeedsCommit.
 class LayerTreeHostTimelinesTestRebuildPropertyTreesOnAnimationSetNeedsCommit
     : public LayerTreeHostTimelinesTest {
  public:
   void SetupTree() override {
     LayerTreeHostTimelinesTest::SetupTree();
     layer_ = FakePictureLayer::Create(layer_settings(), &client_);
     layer_->SetBounds(gfx::Size(4, 4));
     layer_tree_host()->root_layer()->AddChild(layer_);
@@ skipping 31 matching lines @@
  private:
   scoped_refptr<Layer> layer_;
   FakeContentLayerClient client_;
 };
 
 MULTI_THREAD_TEST_F(
     LayerTreeHostTimelinesTestRebuildPropertyTreesOnAnimationSetNeedsCommit);
 
 }  // namespace
 }  // namespace cc