Fix race conditions in window snapshot code.

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 92cba32558746765f15ba8bcd4bd4673fc826dce..a34675169b4d09ac617dc6fb8aa47738513739e3 100644
--- a/cc/trees/layer_tree_host_unittest.cc
+++ b/cc/trees/layer_tree_host_unittest.cc
@@ -636,6 +636,61 @@ class LayerTreeHostTestCompositeAndReadbackAfterForcedDraw
 
 MULTI_THREAD_TEST_F(LayerTreeHostTestCompositeAndReadbackAfterForcedDraw);
 
+class LayerTreeHostTestSetNextCommitForcesRedraw : public LayerTreeHostTest {

[bajones, 2013/09/26 22:27:23]

@enne: Not being overly familiar with these tests please let me know if you feel
this could be improved or if you don't feel it provides the right coverage.

+ public:
+  LayerTreeHostTestSetNextCommitForcesRedraw()
+      : num_draws_(0),
+        bounds_(50, 50),
+        root_layer_(ContentLayer::Create(&client_)) {
+  }
+
+  virtual void BeginTest() OVERRIDE {
+    root_layer_->SetIsDrawable(true);
+    root_layer_->SetBounds(bounds_);
+    layer_tree_host()->SetRootLayer(root_layer_);
+    layer_tree_host()->SetViewportSize(bounds_);
+    PostSetNeedsCommitToMainThread();
+  }
+
+  virtual bool PrepareToDrawOnThread(LayerTreeHostImpl* host_impl,
+                                     LayerTreeHostImpl::FrameData* frame_data,
+                                     bool result) OVERRIDE {
+    EXPECT_TRUE(result);
+
+    gfx::RectF root_damage_rect;
+    if (!frame_data->render_passes.empty())
+      root_damage_rect = frame_data->render_passes.back()->damage_rect;
+
+    // Both passes expect full frame damage
+    EXPECT_RECT_EQ(gfx::Rect(bounds_), root_damage_rect);
+
+    return result;
+  }
+
+  virtual void DrawLayersOnThread(LayerTreeHostImpl* host_impl) OVERRIDE {

[enne (OOO), 2013/09/27 19:59:07]

This test is good, but I'd like to make more sure it's testing the right thing.

Right now you have:

commit -> activate -> draw -> PostSetNextCommitForcesRedraw ->
PostSetNeedsCommit -> commit -> activate -> draw with full damage -> end test

commit -> activate -> draw -> PostSetNextCommitForcesRedraw -> *set needs
redraw* -> *draw with no damage* -> PostSetNeedsCommitToMainThread -> commit ->
*set needs redraw* -> *draw with no damage* -> activate -> draw with full damage
-> end test

Can you insert two draws into the pipeline to make sure that the forced needs
redraw is associated with the right frame? Otherwise, it could be the case that
the "next commit forces full frame damage" isn't actually applied to the right
frame and just applies to the next draw, which only coincidentally is the right
frame.

+    if (!num_draws_) {
+      // Should force full frame damage on the next commit
+      PostSetNextCommitForcesRedrawToMainThread();
+      PostSetNeedsCommitToMainThread();
+    } else {
+      EndTest();
+    }
+    num_draws_++;
+  }
+
+  virtual void AfterTest() OVERRIDE {
+    EXPECT_EQ(2, num_draws_);
+  }
+
+ private:
+  int num_draws_;
+  const gfx::Size bounds_;
+  FakeContentLayerClient client_;
+  scoped_refptr<ContentLayer> root_layer_;
+};
+
+SINGLE_AND_MULTI_THREAD_TEST_F(LayerTreeHostTestSetNextCommitForcesRedraw);
+
 // If the layerTreeHost says it can't draw, Then we should not try to draw.
 class LayerTreeHostTestCanDrawBlocksDrawing : public LayerTreeHostTest {
  public: