CC Animations: Fix draw properties invalidation on LAC destruction

cc/trees/layer_tree_host_unittest_animation_timelines.cc

Index: cc/trees/layer_tree_host_unittest_animation_timelines.cc
diff --git a/cc/trees/layer_tree_host_unittest_animation_timelines.cc b/cc/trees/layer_tree_host_unittest_animation_timelines.cc
index faec049a1582861961144bb3496a13b9f4d7f49c..6b5231b6272ef4c4826b082d015b6411ee11539c 100644
--- a/cc/trees/layer_tree_host_unittest_animation_timelines.cc
+++ b/cc/trees/layer_tree_host_unittest_animation_timelines.cc
@@ -998,17 +998,9 @@ class LayerTreeHostTimelinesTestAnimationFinishesDuringCommit
 // 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 {
@@ -1021,12 +1013,14 @@ class MAYBE_LayerTreeHostTimelinesTestSetPotentiallyAnimatingOnLacDestruction
   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());
@@ -1036,8 +1030,6 @@ class MAYBE_LayerTreeHostTimelinesTestSetPotentiallyAnimatingOnLacDestruction
     }
   }
 
-  void WillCommit() override {}
-
   void DidCommit() override { PostSetNeedsCommitToMainThread(); }
 
   void Layout() override {
@@ -1057,7 +1049,7 @@ class MAYBE_LayerTreeHostTimelinesTestSetPotentiallyAnimatingOnLacDestruction
 };
 
 MULTI_THREAD_TEST_F(
-    MAYBE_LayerTreeHostTimelinesTestSetPotentiallyAnimatingOnLacDestruction);
+    LayerTreeHostTimelinesTestSetPotentiallyAnimatingOnLacDestruction);
 
 // Check that we invalidate property trees on AnimationPlayer::SetNeedsCommit.
 class LayerTreeHostTimelinesTestRebuildPropertyTreesOnAnimationSetNeedsCommit