Fix race conditions in window snapshot code.

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 <algorithm>
 
 #include "base/auto_reset.h"
 #include "base/synchronization/lock.h"
@@ skipping 618 matching lines @@
 
     if (sfn == kReadbackReplacementSourceFrameNumber)
       EndTest();
   }
 
   virtual void AfterTest() OVERRIDE {}
 };
 
 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:
   LayerTreeHostTestCanDrawBlocksDrawing() : num_commits_(0), done_(false) {}
 
   virtual void BeginTest() OVERRIDE { PostSetNeedsCommitToMainThread(); }
 
   virtual void DrawLayersOnThread(LayerTreeHostImpl* impl) OVERRIDE {
     if (done_)
       return;
@@ skipping 4126 matching lines @@
 
  private:
   int num_commits_;
 };
 
 SINGLE_AND_MULTI_THREAD_TEST_F(LayerTreeHostTestMemoryLimits);
 
 }  // namespace
 
 }  // namespace cc