cc: Fix flaky readback+forced draw tests

It's possible that the failed draw will trigger
other draw attempts before the readback/forced draw
arrive on the Impl thread. This patch avoids performing
followup events multiple times when that happens.

BUG=287893

Review URL: https://chromiumcodereview.appspot.com/23542014

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@222307 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 107 insertions, 65 deletions

diff --git a/cc/trees/layer_tree_host_unittest.cc b/cc/trees/layer_tree_host_unittest.cc
index 6d68070..3db7e67 100644
--- a/cc/trees/layer_tree_host_unittest.cc
+++ b/cc/trees/layer_tree_host_unittest.cc
@@ -506,6 +506,13 @@ MULTI_THREAD_TEST_F(
 class LayerTreeHostTestCompositeAndReadbackDuringForcedDraw
     : public LayerTreeHostTest {
  protected:
+  static const int kFirstCommitSourceFrameNumber = 0;
+  static const int kReadbackSourceFrameNumber = 1;
+  static const int kReadbackReplacementAndForcedDrawSourceFrameNumber = 2;
+
+  LayerTreeHostTestCompositeAndReadbackDuringForcedDraw()
+      : did_post_readback_(false) {}
+
   virtual void InitializeSettings(LayerTreeSettings* settings) OVERRIDE {
     // This enables forced draws after a single prepare to draw failure.
     settings->timeout_and_draw_when_animation_checkerboards = true;
@@ -517,40 +524,55 @@ class LayerTreeHostTestCompositeAndReadbackDuringForcedDraw
   virtual bool PrepareToDrawOnThread(LayerTreeHostImpl* host_impl,
                                      LayerTreeHostImpl::FrameData* frame_data,
                                      bool result) OVERRIDE {
-    EXPECT_GE(host_impl->active_tree()->source_frame_number(), 0);
-    EXPECT_LE(host_impl->active_tree()->source_frame_number(), 2);
+    int sfn = host_impl->active_tree()->source_frame_number();
+    EXPECT_TRUE(sfn == kFirstCommitSourceFrameNumber ||
+                sfn == kReadbackSourceFrameNumber ||
+                sfn == kReadbackReplacementAndForcedDrawSourceFrameNumber)
+        << sfn;
 
     // Before we react to the failed draw by initiating the forced draw
     // sequence, start a readback on the main thread.
-    if (host_impl->active_tree()->source_frame_number() == 0)
+    if (sfn == kFirstCommitSourceFrameNumber && !did_post_readback_) {
+      did_post_readback_ = true;
       PostReadbackToMainThread();
+    }
 
-    // Returning false will eventually result in a forced draw.
+    // Returning false will result in a forced draw.
     return false;
   }
 
   virtual void DrawLayersOnThread(LayerTreeHostImpl* host_impl) OVERRIDE {
     // We should only draw for the readback and the forced draw.
-    EXPECT_GE(host_impl->active_tree()->source_frame_number(), 1);
-    EXPECT_LE(host_impl->active_tree()->source_frame_number(), 2);
+    int sfn = host_impl->active_tree()->source_frame_number();
+    EXPECT_TRUE(sfn == kReadbackSourceFrameNumber ||
+                sfn == kReadbackReplacementAndForcedDrawSourceFrameNumber)
+        << sfn;
   }
 
   virtual void SwapBuffersOnThread(LayerTreeHostImpl* host_impl,
                                    bool result) OVERRIDE {
     // We should only swap for the forced draw.
-    EXPECT_EQ(host_impl->active_tree()->source_frame_number(), 2);
+    int sfn = host_impl->active_tree()->source_frame_number();
+    EXPECT_TRUE(sfn == kReadbackReplacementAndForcedDrawSourceFrameNumber)
+        << sfn;
     EndTest();
   }
 
   virtual void AfterTest() OVERRIDE {}
+
+  bool did_post_readback_;
 };
 
-// Flaky test. See http://crbug.com/287893.
-// MULTI_THREAD_TEST_F(LayerTreeHostTestCompositeAndReadbackDuringForcedDraw);
+MULTI_THREAD_TEST_F(LayerTreeHostTestCompositeAndReadbackDuringForcedDraw);
 
 class LayerTreeHostTestCompositeAndReadbackAfterForcedDraw
     : public LayerTreeHostTest {
  protected:
+  static const int kFirstCommitSourceFrameNumber = 0;
+  static const int kForcedDrawSourceFrameNumber = 1;
+  static const int kReadbackSourceFrameNumber = 2;
+  static const int kReadbackReplacementSourceFrameNumber = 3;
+
   virtual void InitializeSettings(LayerTreeSettings* settings) OVERRIDE {
     // This enables forced draws after a single prepare to draw failure.
     settings->timeout_and_draw_when_animation_checkerboards = true;
@@ -562,19 +584,25 @@ class LayerTreeHostTestCompositeAndReadbackAfterForcedDraw
   virtual bool PrepareToDrawOnThread(LayerTreeHostImpl* host_impl,
                                      LayerTreeHostImpl::FrameData* frame_data,
                                      bool result) OVERRIDE {
-    EXPECT_GE(host_impl->active_tree()->source_frame_number(), 0);
-    EXPECT_LE(host_impl->active_tree()->source_frame_number(), 3);
-
-    // Returning false will eventually result in a forced draw.
+    int sfn = host_impl->active_tree()->source_frame_number();
+    EXPECT_TRUE(sfn == kFirstCommitSourceFrameNumber ||
+                sfn == kForcedDrawSourceFrameNumber ||
+                sfn == kReadbackSourceFrameNumber ||
+                sfn == kReadbackReplacementSourceFrameNumber)
+        << sfn;
+
+    // Returning false will result in a forced draw.
     return false;
   }
 
   virtual void DidCommit() OVERRIDE {
-    if (layer_tree_host()->source_frame_number() == 1) {
+    if (layer_tree_host()->source_frame_number() ==
+        kForcedDrawSourceFrameNumber) {
       // Avoid aborting the forced draw commit so source_frame_number
       // increments.
       layer_tree_host()->SetNeedsCommit();
-    } else if (layer_tree_host()->source_frame_number() == 2) {
+    } else if (layer_tree_host()->source_frame_number() ==
+               kReadbackSourceFrameNumber) {
       // Perform a readback immediately after the forced draw's commit.
       char pixels[4];
       layer_tree_host()->CompositeAndReadback(&pixels, gfx::Rect(0, 0, 1, 1));
@@ -584,16 +612,22 @@ class LayerTreeHostTestCompositeAndReadbackAfterForcedDraw
   virtual void DrawLayersOnThread(LayerTreeHostImpl* host_impl) OVERRIDE {
     // We should only draw for the the forced draw, readback, and
     // replacement commit.
-    EXPECT_GE(host_impl->active_tree()->source_frame_number(), 1);
-    EXPECT_LE(host_impl->active_tree()->source_frame_number(), 3);
+    int sfn = host_impl->active_tree()->source_frame_number();
+    EXPECT_TRUE(sfn == kForcedDrawSourceFrameNumber ||
+                sfn == kReadbackSourceFrameNumber ||
+                sfn == kReadbackReplacementSourceFrameNumber)
+        << sfn;
   }
 
   virtual void SwapBuffersOnThread(LayerTreeHostImpl* host_impl,
                                    bool result) OVERRIDE {
     // We should only swap for the forced draw and replacement commit.
-    EXPECT_TRUE(host_impl->active_tree()->source_frame_number() == 1 ||
-                host_impl->active_tree()->source_frame_number() == 3);
-    if (host_impl->active_tree()->source_frame_number() == 3)
+    int sfn = host_impl->active_tree()->source_frame_number();
+    EXPECT_TRUE(sfn == kForcedDrawSourceFrameNumber ||
+                sfn == kReadbackReplacementSourceFrameNumber)
+        << sfn;
+
+    if (sfn == kReadbackReplacementSourceFrameNumber)
       EndTest();
   }
 
diff --git a/cc/trees/layer_tree_host_unittest_context.cc b/cc/trees/layer_tree_host_unittest_context.cc
index c6c2977..a592fdb 100644
--- a/cc/trees/layer_tree_host_unittest_context.cc
+++ b/cc/trees/layer_tree_host_unittest_context.cc
@@ -1491,15 +1491,18 @@ class LayerTreeHostContextTestCompositeAndReadbackBeforeOutputSurfaceInit
 SINGLE_AND_MULTI_THREAD_TEST_F(
     LayerTreeHostContextTestCompositeAndReadbackBeforeOutputSurfaceInit);
 
-// This test verifies that losing an output surface right during a
+// This test verifies that losing an output surface during a
 // simultaneous readback and forced redraw works and does not deadlock.
 class LayerTreeHostContextTestLoseOutputSurfaceDuringReadbackAndForcedDraw
     : public LayerTreeHostContextTest {
  protected:
-  static const int kCommitAfterFirstOutputSurfaceInitSourceFrameNumber = 0;
-  static const int kReadbackCommitSourceFrameNumber = 1;
-  static const int kReadbackReplacementCommitSourceFrameNumber = 2;
-  static const int kCommitAfterSecondOutputSurfaceInitSourceFrameNumber = 3;
+  static const int kFirstOutputSurfaceInitSourceFrameNumber = 0;
+  static const int kReadbackSourceFrameNumber = 1;
+  static const int kReadbackReplacementSourceFrameNumber = 2;
+  static const int kSecondOutputSurfaceInitSourceFrameNumber = 3;
+
+  LayerTreeHostContextTestLoseOutputSurfaceDuringReadbackAndForcedDraw()
+      : did_react_to_first_commit_(false) {}
 
   virtual void InitializeSettings(LayerTreeSettings* settings) OVERRIDE {
     // This enables forced draws after a single prepare to draw failure.
@@ -1512,54 +1515,55 @@ class LayerTreeHostContextTestLoseOutputSurfaceDuringReadbackAndForcedDraw
   virtual bool PrepareToDrawOnThread(LayerTreeHostImpl* host_impl,
                                      LayerTreeHostImpl::FrameData* frame_data,
                                      bool result) OVERRIDE {
-    EXPECT_TRUE(host_impl->active_tree()->source_frame_number() ==
-                    kCommitAfterFirstOutputSurfaceInitSourceFrameNumber ||
-                host_impl->active_tree()->source_frame_number() ==
-                    kCommitAfterSecondOutputSurfaceInitSourceFrameNumber ||
-                host_impl->active_tree()->source_frame_number() ==
-                    kReadbackCommitSourceFrameNumber);
+    int sfn = host_impl->active_tree()->source_frame_number();
+    EXPECT_TRUE(sfn == kFirstOutputSurfaceInitSourceFrameNumber ||
+                sfn == kSecondOutputSurfaceInitSourceFrameNumber ||
+                sfn == kReadbackSourceFrameNumber)
+        << sfn;
 
     // Before we react to the failed draw by initiating the forced draw
     // sequence, start a readback on the main thread and then lose the context
     // to start output surface initialization all at the same time.
-    if (host_impl->active_tree()->source_frame_number() ==
-        kCommitAfterFirstOutputSurfaceInitSourceFrameNumber) {
+    if (sfn == kFirstOutputSurfaceInitSourceFrameNumber &&
+        !did_react_to_first_commit_) {
+      did_react_to_first_commit_ = true;
       PostReadbackToMainThread();
       LoseContext();
     }
 
-    // Returning false will eventually result in a forced draw.
     return false;
   }
 
   virtual void InitializedRendererOnThread(LayerTreeHostImpl* host_impl,
                                            bool success) OVERRIDE {
     // -1 is for the first output surface initialization.
-    EXPECT_TRUE(host_impl->active_tree()->source_frame_number() == -1 ||
-                host_impl->active_tree()->source_frame_number() ==
-                    kReadbackReplacementCommitSourceFrameNumber);
+    int sfn = host_impl->active_tree()->source_frame_number();
+    EXPECT_TRUE(sfn == -1 || sfn == kReadbackReplacementSourceFrameNumber)
+        << sfn;
   }
 
   virtual void DrawLayersOnThread(LayerTreeHostImpl* host_impl) OVERRIDE {
     // We should only draw the first commit after output surface initialization
     // and attempt to draw the readback commit (which will fail).
     // All others should abort because the output surface is lost.
-    EXPECT_TRUE(host_impl->active_tree()->source_frame_number() ==
-                    kCommitAfterSecondOutputSurfaceInitSourceFrameNumber ||
-                host_impl->active_tree()->source_frame_number() ==
-                    kReadbackCommitSourceFrameNumber);
+    int sfn = host_impl->active_tree()->source_frame_number();
+    EXPECT_TRUE(sfn == kSecondOutputSurfaceInitSourceFrameNumber ||
+                sfn == kReadbackSourceFrameNumber)
+        << sfn;
   }
 
   virtual void SwapBuffersOnThread(LayerTreeHostImpl* host_impl,
                                    bool result) OVERRIDE {
     // We should only swap the first commit after the second output surface
     // initialization.
-    EXPECT_TRUE(host_impl->active_tree()->source_frame_number() ==
-                kCommitAfterSecondOutputSurfaceInitSourceFrameNumber);
+    int sfn = host_impl->active_tree()->source_frame_number();
+    EXPECT_TRUE(sfn == kSecondOutputSurfaceInitSourceFrameNumber) << sfn;
     EndTest();
   }
 
   virtual void AfterTest() OVERRIDE {}
+
+  int did_react_to_first_commit_;
 };
 
 MULTI_THREAD_TEST_F(
@@ -1570,11 +1574,14 @@ MULTI_THREAD_TEST_F(
 class LayerTreeHostContextTestReadbackWithForcedDrawAndOutputSurfaceInit
     : public LayerTreeHostContextTest {
  protected:
-  static const int kCommitAfterFirstOutputSurfaceInitSourceFrameNumber = 0;
-  static const int kReadbackCommitSourceFrameNumber = 1;
-  static const int kReadbackReplacementCommitSourceFrameNumber = 2;
+  static const int kFirstOutputSurfaceInitSourceFrameNumber = 0;
+  static const int kReadbackSourceFrameNumber = 1;
+  static const int kReadbackReplacementSourceFrameNumber = 2;
   static const int kForcedDrawCommitSourceFrameNumber = 2;
-  static const int kCommitAfterSecondOutputSurfaceInitSourceFrameNumber = 2;
+  static const int kSecondOutputSurfaceInitSourceFrameNumber = 2;
+
+  LayerTreeHostContextTestReadbackWithForcedDrawAndOutputSurfaceInit()
+      : did_lose_context_(false) {}
 
   virtual void InitializeSettings(LayerTreeSettings* settings) OVERRIDE {
     // This enables forced draws after a single prepare to draw failure.
@@ -1587,22 +1594,21 @@ class LayerTreeHostContextTestReadbackWithForcedDrawAndOutputSurfaceInit
   virtual bool PrepareToDrawOnThread(LayerTreeHostImpl* host_impl,
                                      LayerTreeHostImpl::FrameData* frame_data,
                                      bool result) OVERRIDE {
-    EXPECT_TRUE(host_impl->active_tree()->source_frame_number() ==
-                    kCommitAfterFirstOutputSurfaceInitSourceFrameNumber ||
-                host_impl->active_tree()->source_frame_number() ==
-                    kCommitAfterSecondOutputSurfaceInitSourceFrameNumber ||
-                host_impl->active_tree()->source_frame_number() ==
-                    kReadbackCommitSourceFrameNumber);
+    int sfn = host_impl->active_tree()->source_frame_number();
+    EXPECT_TRUE(sfn == kFirstOutputSurfaceInitSourceFrameNumber ||
+                sfn == kSecondOutputSurfaceInitSourceFrameNumber ||
+                sfn == kReadbackSourceFrameNumber)
+        << sfn;
 
     // Before we react to the failed draw by initiating the forced draw
     // sequence, start a readback on the main thread and then lose the context
     // to start output surface initialization all at the same time.
-    if (host_impl->active_tree()->source_frame_number() ==
-        kCommitAfterFirstOutputSurfaceInitSourceFrameNumber) {
+    if (sfn == kFirstOutputSurfaceInitSourceFrameNumber && !did_lose_context_) {
+      did_lose_context_ = true;
       LoseContext();
     }
 
-    // Returning false will eventually result in a forced draw.
+    // Returning false will result in a forced draw.
     return false;
   }
 
@@ -1619,31 +1625,33 @@ class LayerTreeHostContextTestReadbackWithForcedDrawAndOutputSurfaceInit
   virtual void InitializedRendererOnThread(LayerTreeHostImpl* host_impl,
                                            bool success) OVERRIDE {
     // -1 is for the first output surface initialization.
-    EXPECT_TRUE(host_impl->active_tree()->source_frame_number() == -1 ||
-                host_impl->active_tree()->source_frame_number() ==
-                    kCommitAfterFirstOutputSurfaceInitSourceFrameNumber);
+    int sfn = host_impl->active_tree()->source_frame_number();
+    EXPECT_TRUE(sfn == -1 || sfn == kFirstOutputSurfaceInitSourceFrameNumber)
+        << sfn;
   }
 
   virtual void DrawLayersOnThread(LayerTreeHostImpl* host_impl) OVERRIDE {
     // We should only draw the first commit after output surface initialization
     // and attempt to draw the readback commit (which will fail).
     // All others should abort because the output surface is lost.
-    EXPECT_TRUE(host_impl->active_tree()->source_frame_number() ==
-                    kForcedDrawCommitSourceFrameNumber ||
-                host_impl->active_tree()->source_frame_number() ==
-                    kReadbackCommitSourceFrameNumber);
+    int sfn = host_impl->active_tree()->source_frame_number();
+    EXPECT_TRUE(sfn == kForcedDrawCommitSourceFrameNumber ||
+                sfn == kReadbackSourceFrameNumber)
+        << sfn;
   }
 
   virtual void SwapBuffersOnThread(LayerTreeHostImpl* host_impl,
                                    bool result) OVERRIDE {
     // We should only swap the first commit after the second output surface
     // initialization.
-    EXPECT_TRUE(host_impl->active_tree()->source_frame_number() ==
-                kForcedDrawCommitSourceFrameNumber);
+    int sfn = host_impl->active_tree()->source_frame_number();
+    EXPECT_TRUE(sfn == kForcedDrawCommitSourceFrameNumber) << sfn;
     EndTest();
   }
 
   virtual void AfterTest() OVERRIDE {}
+
+  int did_lose_context_;
 };
 
 MULTI_THREAD_TEST_F(