Fixing flakiness of TextureLayerChangeInvisibleMailboxTest

cc/trees/layer_tree_host_unittest.cc

 // Copyright 2011 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 <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
@@ skipping 7484 matching lines @@
     DCHECK_EQ(hud->clip_tree_index(), root_layer->clip_tree_index());
     DCHECK_EQ(hud->effect_tree_index(), root_layer->effect_tree_index());
     DCHECK_EQ(hud->scroll_tree_index(), root_layer->scroll_tree_index());
   }
 
   void AfterTest() override {}
 };
 
 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 SetupTree() override {
+    scoped_refptr<Layer> root = Layer::Create();
+    root->SetBounds(gfx::Size(10, 10));
+    layer_tree_host()->SetRootLayer(std::move(root));
+    LayerTreeHostTest::SetupTree();
+  }
+
+  void BeginTest() override { PostSetNeedsCommitToMainThread(); }
+
+  void WillReceiveCompositorFrameAckOnThread(
+      LayerTreeHostImpl* host_impl) override {
+    MainThreadTaskRunner()->PostTask(
+        FROM_HERE, base::Bind(&LayerTreeHostTestDiscardAckAfterRelease::
+                                  ReleaseCompositorFrameSink,
+                              base::Unretained(this)));
+  }
+
+  void DidReceiveCompositorFrameAckOnThread(
+      LayerTreeHostImpl* host_impl) override {
+    MainThreadTaskRunner()->PostDelayedTask(

[danakj, 2017/04/18 21:52:34]

Thanks! LGTM

One suggestion is pretend you are a traveller from the future and are reading
this code and want to understand it. Over-commenting in tests is very helpful
because every thing you do in a test has a reason but it's not always entirely
clear, and it makes understanding this later much easier. So like why delayed
here, so you don't have to reason it out again in the future. Same for why the
Release is happening between WillReceive and DidReceive on the compositor
thread.

+        FROM_HERE,
+        base::Bind(
+            &LayerTreeHostTestDiscardAckAfterRelease::VerifyAckNotReceived,
+            base::Unretained(this)),
+        base::TimeDelta::FromMilliseconds(100));
+  }
+
+  void DidReceiveCompositorFrameAck() override { received_ack_ = true; }
+
+  void ReleaseCompositorFrameSink() {
+    layer_tree_host()->SetVisible(false);
+    layer_tree_host()->ReleaseCompositorFrameSink();
+  }
+
+  void VerifyAckNotReceived() {
+    EXPECT_FALSE(received_ack_);
+    EndTest();
+  }
+
+  void AfterTest() override {}
+
+ private:
+  bool received_ack_ = false;
+};
+
+SINGLE_AND_MULTI_THREAD_TEST_F(LayerTreeHostTestDiscardAckAfterRelease);
+
 }  // namespace
 }  // namespace cc