cc: Abort frame when becoming invisible and waiting for ready to draw.

cc/trees/layer_tree_host_unittest.cc

Index: cc/trees/layer_tree_host_unittest.cc
diff --git a/cc/trees/layer_tree_host_unittest.cc b/cc/trees/layer_tree_host_unittest.cc
index 470a63b1b3ae3651e581e45e3b81796ba712c75e..729395de47e0c1ede597e4a91aa3f177c3aa659e 100644
--- a/cc/trees/layer_tree_host_unittest.cc
+++ b/cc/trees/layer_tree_host_unittest.cc
@@ -202,6 +202,49 @@ class LayerTreeHostTestReadyToActivateNonEmpty
 // active tree, so notify ready to activate is skipped.
 MULTI_THREAD_TEST_F(LayerTreeHostTestReadyToActivateNonEmpty);
 
+// This tests if we get the READY_TO_ACTIVATE signal if we become invisible and
+// then become visible again.
+class LayerTreeHostTestReadyToActivateVisibility
+    : public LayerTreeHostTestReadyToActivateNonEmpty {
+ public:
+  LayerTreeHostTestReadyToActivateVisibility()
+      : LayerTreeHostTestReadyToActivateNonEmpty(),
+        commit_count_(0),
+        toggled_visibility_(false) {}
+
+  void CommitCompleteOnThread(LayerTreeHostImpl* host_impl) override {
+    LayerTreeHostTestReadyToActivateNonEmpty::CommitCompleteOnThread(host_impl);
+    commit_count_++;
+    if (commit_count_ == 1)
+      PostSetVisibleToMainThread(false);
+  }
+
+  void DidFinishImplFrameOnThread(LayerTreeHostImpl* host_impl) override {
+    if (commit_count_ == 1)
+      PostSetVisibleToMainThread(true);

[brianderson, 2015/07/23 02:00:23]

I think it's possible this races with the DidSetVisibleOnImplTree(false) we'd
expect to get the main thread round trip of PostSetVisibleToMainThread(false).
Is that okay?

[sunnyps, 2015/07/23 22:20:44]

That works for the ready to draw test because that's only for STP but it won't
work for this test. I'm removing this test because there's no way to make this
race free without a lot of complexity. Also this isn't being addressed by the CL
anyway.

+  }
+
+  void DidSetVisibleOnImplTree(LayerTreeHostImpl* host_impl,
+                               bool visible) override {
+    if (!visible)
+      toggled_visibility_ = true;
+  }
+
+  void AfterTest() override {
+    LayerTreeHostTestReadyToActivateNonEmpty::AfterTest();
+    EXPECT_LE(1, commit_count_);

[brianderson, 2015/07/23 02:00:23]

I'm not sure if this test has a race between READY_TO_ACTIVATE and
DidSetVisibleOnImplTree(false).

Could you override DrawLayersOnThread and verify this test only draws after
toggled_visibility_ is true?

[sunnyps, 2015/07/23 22:20:44]

See reply above.

+    EXPECT_TRUE(toggled_visibility_);
+  }
+
+ private:
+  int commit_count_;
+  bool toggled_visibility_;
+};
+
+// Multi-thread only because in single thread the commit goes directly to the
+// active tree, so notify ready to activate is skipped.
+MULTI_THREAD_TEST_F(LayerTreeHostTestReadyToActivateVisibility);
+
 // Test if the LTHI receives ReadyToDraw notifications from the TileManager when
 // no raster tasks get scheduled.
 class LayerTreeHostTestReadyToDrawEmpty : public LayerTreeHostTest {
@@ -272,6 +315,48 @@ class LayerTreeHostTestReadyToDrawNonEmpty
 // single threaded mode.
 SINGLE_THREAD_TEST_F(LayerTreeHostTestReadyToDrawNonEmpty);
 
+// This tests if we get the READY_TO_DRAW signal if we become invisible and then
+// become visible again.
+class LayerTreeHostTestReadyToDrawVisibility

[brianderson, 2015/07/23 02:00:23]

See comments about other test.

+    : public LayerTreeHostTestReadyToDrawNonEmpty {
+ public:
+  LayerTreeHostTestReadyToDrawVisibility()
+      : LayerTreeHostTestReadyToDrawNonEmpty(),
+        commit_count_(0),
+        toggled_visibility_(false) {}
+
+  void CommitCompleteOnThread(LayerTreeHostImpl* host_impl) override {
+    commit_count_++;
+    if (commit_count_ == 1)
+      PostSetVisibleToMainThread(false);
+  }
+
+  void DidFinishImplFrameOnThread(LayerTreeHostImpl* host_impl) override {
+    if (commit_count_ == 1)
+      PostSetVisibleToMainThread(true);
+  }
+
+  void DidSetVisibleOnImplTree(LayerTreeHostImpl* host_impl,
+                               bool visible) override {
+    if (!visible)
+      toggled_visibility_ = true;
+  }
+
+  void AfterTest() override {
+    LayerTreeHostTestReadyToDrawNonEmpty::AfterTest();
+    EXPECT_LE(1, commit_count_);
+    EXPECT_TRUE(toggled_visibility_);
+  }
+
+ private:
+  int commit_count_;
+  bool toggled_visibility_;
+};
+
+// Note: With this test setup, we only get tiles flagged as REQUIRED_FOR_DRAW in
+// single threaded mode.
+SINGLE_THREAD_TEST_F(LayerTreeHostTestReadyToDrawVisibility);
+
 class LayerTreeHostFreeWorkerContextResourcesTest : public LayerTreeHostTest {
  public:
   scoped_ptr<FakeOutputSurface> CreateFakeOutputSurface() override {