Fix failing (flaky) LayerTreeHostTestLCDNotification test.

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 1787 matching lines @@
 
  private:
   FakeContentLayerClient client_;
   scoped_refptr<EvictionTestLayer> layer_;
   LayerTreeHostImpl* impl_for_evict_textures_;
   int num_commits_;
 };
 
 MULTI_THREAD_NOIMPL_TEST_F(LayerTreeHostTestEvictTextures);
 
-class LayerTreeHostTestContinuousCommit : public LayerTreeHostTest {
- public:
-  LayerTreeHostTestContinuousCommit()
-      : num_commit_complete_(0), num_draw_layers_(0) {}
-
-  virtual void BeginTest() OVERRIDE {
-    layer_tree_host()->SetViewportSize(gfx::Size(10, 10));
-    layer_tree_host()->root_layer()->SetBounds(gfx::Size(10, 10));
-
-    PostSetNeedsCommitToMainThread();
-  }
-
-  virtual void DidCommit() OVERRIDE {
-    if (num_draw_layers_ == 2)
-      return;
-    layer_tree_host()->SetNeedsCommit();
-  }
-
-  virtual void CommitCompleteOnThread(LayerTreeHostImpl* impl) OVERRIDE {
-    if (num_draw_layers_ == 1)
-      num_commit_complete_++;
-  }
-
-  virtual void DrawLayersOnThread(LayerTreeHostImpl* impl) OVERRIDE {
-    num_draw_layers_++;
-    if (num_draw_layers_ == 2)
-      EndTest();
-  }
-
-  virtual void AfterTest() OVERRIDE {
-    // Check that we didn't commit twice between first and second draw.
-    EXPECT_EQ(1, num_commit_complete_);
-  }
-
- private:
-  int num_commit_complete_;
-  int num_draw_layers_;
-};
-
-MULTI_THREAD_TEST_F(LayerTreeHostTestContinuousCommit);
-
 class LayerTreeHostTestContinuousInvalidate : public LayerTreeHostTest {
  public:
   LayerTreeHostTestContinuousInvalidate()
       : num_commit_complete_(0), num_draw_layers_(0) {}
 
   virtual void BeginTest() OVERRIDE {
     layer_tree_host()->SetViewportSize(gfx::Size(10, 10));
     layer_tree_host()->root_layer()->SetBounds(gfx::Size(10, 10));
 
     content_layer_ = ContentLayer::Create(&client_);
@@ skipping 2702 matching lines @@
   int commit_complete_count_;
   TestSwapPromiseResult swap_promise_result_[3];
 };
 
 // TODO(miletus): Flaky test: crbug.com/393995
 // MULTI_THREAD_TEST_F(LayerTreeHostTestBreakSwapPromise);
 
 class LayerTreeHostTestBreakSwapPromiseForVisibilityAbortedCommit
     : public LayerTreeHostTest {
  protected:
-  LayerTreeHostTestBreakSwapPromiseForVisibilityAbortedCommit()
-      : commit_count_(0) {}
-
   virtual void BeginTest() OVERRIDE { PostSetNeedsCommitToMainThread(); }
 
-  virtual void WillBeginMainFrame() OVERRIDE {
+  virtual void DidCommit() OVERRIDE {
     layer_tree_host()->SetDeferCommits(true);
     layer_tree_host()->SetNeedsCommit();
   }
 
   virtual void DidDeferCommit() OVERRIDE {
     layer_tree_host()->SetVisible(false);
     layer_tree_host()->SetDeferCommits(false);
 
     scoped_ptr<SwapPromise> swap_promise(
         new TestSwapPromise(&swap_promise_result_));
     layer_tree_host()->QueueSwapPromise(swap_promise.Pass());
   }
 
-  virtual void DidCommit() OVERRIDE { PostSetNeedsCommitToMainThread(); }
-
   virtual void BeginMainFrameAbortedOnThread(LayerTreeHostImpl* host_impl,
                                              bool did_handle) OVERRIDE {
     EndTest();
   }
 
   virtual void AfterTest() OVERRIDE {
     {
       base::AutoLock lock(swap_promise_result_.lock);
       EXPECT_FALSE(swap_promise_result_.did_swap_called);
       EXPECT_TRUE(swap_promise_result_.did_not_swap_called);
       EXPECT_EQ(SwapPromise::COMMIT_FAILS, swap_promise_result_.reason);
       EXPECT_TRUE(swap_promise_result_.dtor_called);
     }
   }
 
-  int commit_count_;
   TestSwapPromiseResult swap_promise_result_;
 };
 
 SINGLE_AND_MULTI_THREAD_TEST_F(
     LayerTreeHostTestBreakSwapPromiseForVisibilityAbortedCommit);
 
 class LayerTreeHostTestBreakSwapPromiseForContextAbortedCommit
     : public LayerTreeHostTest {
  protected:
-  LayerTreeHostTestBreakSwapPromiseForContextAbortedCommit()
-      : commit_count_(0) {}
-
   virtual void BeginTest() OVERRIDE { PostSetNeedsCommitToMainThread(); }
 
-  virtual void WillBeginMainFrame() OVERRIDE {
+  virtual void DidCommit() OVERRIDE {
+    if (TestEnded())
+      return;
     layer_tree_host()->SetDeferCommits(true);
     layer_tree_host()->SetNeedsCommit();
   }
 
   virtual void DidDeferCommit() OVERRIDE {
     layer_tree_host()->DidLoseOutputSurface();
     layer_tree_host()->SetDeferCommits(false);
 
     scoped_ptr<SwapPromise> swap_promise(

[enne (OOO), 2014/08/12 17:49:56]

Can you put this swap promise before the SetDeferCommits(false) call? I realize
this is my fault, but I think it's in the wrong place.  I think it could be
possible for the FindOutputSurface call to happen with the commit that this swap
promise got queued on, and it could succeed (and the test would fail, flakily?)

[danakj, 2014/08/12 18:05:43]

Sure, tho I don' think it can flake because the FindOutputSurface happens after
BeginMainFrameAbortedOnThread at which point the swap promise has already been
aborted on the main thread. And the aborted commit is the one that was deferred.

         new TestSwapPromise(&swap_promise_result_));
     layer_tree_host()->QueueSwapPromise(swap_promise.Pass());
   }
 
-  virtual void DidCommit() OVERRIDE { PostSetNeedsCommitToMainThread(); }
-
   virtual void BeginMainFrameAbortedOnThread(LayerTreeHostImpl* host_impl,
                                              bool did_handle) OVERRIDE {
     EndTest();
+    // This lets the test finally commit and exit.
+    MainThreadTaskRunner()->PostTask(
+        FROM_HERE,
+        base::Bind(&LayerTreeHostTestBreakSwapPromiseForContextAbortedCommit::
+                       FindOutputSurface,
+                   base::Unretained(this)));
+  }
+
+  void FindOutputSurface() {
+    layer_tree_host()->OnCreateAndInitializeOutputSurfaceAttempted(true);
   }
 
   virtual void AfterTest() OVERRIDE {
     {
       base::AutoLock lock(swap_promise_result_.lock);
       EXPECT_FALSE(swap_promise_result_.did_swap_called);
       EXPECT_TRUE(swap_promise_result_.did_not_swap_called);
       EXPECT_EQ(SwapPromise::COMMIT_FAILS, swap_promise_result_.reason);
       EXPECT_TRUE(swap_promise_result_.dtor_called);
     }
   }
 
-  int commit_count_;
   TestSwapPromiseResult swap_promise_result_;
 };
 
 SINGLE_AND_MULTI_THREAD_TEST_F(
     LayerTreeHostTestBreakSwapPromiseForContextAbortedCommit);
 
 class SimpleSwapPromiseMonitor : public SwapPromiseMonitor {
  public:
   SimpleSwapPromiseMonitor(LayerTreeHost* layer_tree_host,
                            LayerTreeHostImpl* layer_tree_host_impl,
@@ skipping 16 matching lines @@
  private:
   int* set_needs_commit_count_;
   int* set_needs_redraw_count_;
 };
 
 class LayerTreeHostTestSimpleSwapPromiseMonitor : public LayerTreeHostTest {
  public:
   virtual void BeginTest() OVERRIDE { PostSetNeedsCommitToMainThread(); }
 
   virtual void WillBeginMainFrame() OVERRIDE {
+    if (TestEnded())
+      return;
+
     int set_needs_commit_count = 0;
     int set_needs_redraw_count = 0;
 
     {
       scoped_ptr<SimpleSwapPromiseMonitor> swap_promise_monitor(
           new SimpleSwapPromiseMonitor(layer_tree_host(),
                                        NULL,
                                        &set_needs_commit_count,
                                        &set_needs_redraw_count));
       layer_tree_host()->SetNeedsCommit();
@@ skipping 290 matching lines @@
     child_layer_->SetBounds(bounds_);
     child_layer_->SetIsDrawable(true);
     root_layer->AddChild(child_layer_);
 
     layer_tree_host()->SetRootLayer(root_layer);
     layer_tree_host()->SetViewportSize(bounds_);
     LayerTreeHostTest::SetupTree();
   }
 
   virtual void BeginTest() OVERRIDE {
-    // Wait 50x longer than expected.
-    double milliseconds_per_frame =
-        1000 / layer_tree_host()->settings().refresh_rate;
-    EndTestAfterDelay(50 * kExpectedNumCommits * milliseconds_per_frame);
     MainThreadTaskRunner()->PostTask(
         FROM_HERE,
         base::Bind(
             &LayerTreeHostTestContinuousPainting::EnableContinuousPainting,
             base::Unretained(this)));
+    // Wait 50x longer than expected.
+    double milliseconds_per_frame =
+        1000.0 / layer_tree_host()->settings().refresh_rate;
+    MainThreadTaskRunner()->PostDelayedTask(
+        FROM_HERE,
+        base::Bind(
+            &LayerTreeHostTestContinuousPainting::DisableContinuousPainting,
+            base::Unretained(this)),
+        base::TimeDelta::FromMilliseconds(50 * kExpectedNumCommits *
+                                          milliseconds_per_frame));
   }
 
   virtual void Animate(base::TimeTicks monotonic_time) OVERRIDE {
     child_layer_->SetNeedsDisplay();
   }
 
   virtual void AfterTest() OVERRIDE {
     EXPECT_LE(kExpectedNumCommits, num_commits_);
     EXPECT_LE(kExpectedNumCommits, num_draws_);
     int update_count = content_layer_ ? content_layer_->PaintContentsCount()
@@ skipping 10 matching lines @@
     ++num_commits_;
   }
 
  private:
   void EnableContinuousPainting() {
     LayerTreeDebugState debug_state = layer_tree_host()->debug_state();
     debug_state.continuous_painting = true;
     layer_tree_host()->SetDebugState(debug_state);
   }
 
+  void DisableContinuousPainting() {
+    LayerTreeDebugState debug_state = layer_tree_host()->debug_state();
+    debug_state.continuous_painting = false;
+    layer_tree_host()->SetDebugState(debug_state);
+    EndTest();
+  }
+
   int num_commits_;
   int num_draws_;
   const gfx::Size bounds_;
   FakeContentLayerClient client_;
   scoped_refptr<ContentLayerWithUpdateTracking> content_layer_;
   scoped_refptr<FakePictureLayer> picture_layer_;
   Layer* child_layer_;
 };
 
 MULTI_THREAD_TEST_F(LayerTreeHostTestContinuousPainting);
 
 }  // namespace cc