Fixing flakiness of TextureLayerChangeInvisibleMailboxTest

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 2e7457a49aac6600be861bf479eb244c545a3d17..ae43ef2e7487f38a22a6b406cd409e9a43d9db4c 100644
--- a/cc/trees/layer_tree_host_unittest.cc
+++ b/cc/trees/layer_tree_host_unittest.cc
@@ -7502,5 +7502,90 @@ class LayerTreeHostTestHudLayerWithLayerLists : public LayerTreeHostTest {
 
 SINGLE_AND_MULTI_THREAD_TEST_F(LayerTreeHostTestHudLayerWithLayerLists);
 
+// Verifies that LayerTreeHostClient does not receive frame acks from a released
+// CompositorFrameSink.
+class LayerTreeHostTestDiscardAckAfterRelease : public LayerTreeHostTest {
+ protected:
+  void BeginTest() override {
+    PrepareNextCommit();
+    PostSetNeedsCommitToMainThread();
+  }
+
+  void PrepareNextCommit() {

[danakj, 2017/04/19 14:36:28]

nit: I think the standard way to write this would be to keep the layer setup in
SetupTree, then in CheckFrameAck() to do layer_tree_host()->SetNeedsCommit()
instead of resetting the root layer. And lastly to use
layer_tree_host()->source_frame_number() instead of rolling your own frame
counter as commit_count_.

[Saman Sami, 2017/04/19 15:00:15]

Seems like SetNeedsCommit doesn't work on its own. I think If I don't make the
tree dirty, the swap will not happen.

[danakj, 2017/04/19 15:23:01]

Oh oh yeah, to redraw you need some damage. One way is to do something to change
the root layer (instead of SetNeedsCommit) like

// Cause damage so that we draw and swap.
layer_tree_host()->root_layer()->SetBackgroundColor(SK_ColorGREEN);

[Saman Sami, 2017/04/19 15:45:30]

Done.

+    // Make some changes so we have something to commit.
+    scoped_refptr<Layer> root = Layer::Create();
+    root->SetBounds(gfx::Size(10, 10));
+    layer_tree_host()->SetRootLayer(std::move(root));
+    commit_count_++;
+  }
+
+  void WillReceiveCompositorFrameAckOnThread(
+      LayerTreeHostImpl* host_impl) override {
+    // This method is called before ack is posted to main thread. This ensures
+    // that WillReceiveCompositorFrameAck which we PostTask below will be called
+    // before DidReceiveCompositorFrameAck.
+    MainThreadTaskRunner()->PostTask(
+        FROM_HERE, base::Bind(&LayerTreeHostTestDiscardAckAfterRelease::
+                                  WillReceiveCompositorFrameAck,
+                              base::Unretained(this)));
+  }
+
+  void WillReceiveCompositorFrameAck() {
+    switch (commit_count_) {
+      case 1:
+        // For the first commit, don't release the CompositorFrameSink. We must
+        // receive the ack later on.
+        break;
+      case 2:
+        // Release the CompositorFrameSink for the second commit. We'll later
+        // check that the ack is discarded.
+        layer_tree_host()->SetVisible(false);
+        layer_tree_host()->ReleaseCompositorFrameSink();
+        break;
+      default:
+        NOTREACHED();
+    }
+  }
+
+  void DidReceiveCompositorFrameAckOnThread(
+      LayerTreeHostImpl* host_impl) override {
+    // Since this method is called after ack is posted to main thread, we can be
+    // sure that if the ack is not discarded, it will be definitely received
+    // before we are in CheckFrameAck.
+    MainThreadTaskRunner()->PostTask(
+        FROM_HERE,
+        base::Bind(&LayerTreeHostTestDiscardAckAfterRelease::CheckFrameAck,
+                   base::Unretained(this)));
+  }
+
+  void DidReceiveCompositorFrameAck() override { received_ack_ = true; }
+
+  void CheckFrameAck() {
+    switch (commit_count_) {
+      case 1:
+        // CompositorFrameSink was not released. We must receive the ack.
+        EXPECT_TRUE(received_ack_);
+        PrepareNextCommit();
+        break;
+      case 2:
+        // CompositorFrameSink was released. The ack must be discarded.
+        EXPECT_FALSE(received_ack_);
+        EndTest();
+        break;
+      default:
+        NOTREACHED();
+    }
+    received_ack_ = false;
+  }
+
+  void AfterTest() override {}
+
+ private:
+  bool received_ack_ = false;
+  int commit_count_ = 0;
+};
+
+SINGLE_AND_MULTI_THREAD_TEST_F(LayerTreeHostTestDiscardAckAfterRelease);
+
 }  // namespace
 }  // namespace cc