cc: Use BeginFrameArgs

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 0a4c23602b416eddd9dafb3217e693fb628e6d47..1b22d93754b04382337214da1223c7c4ad02ed01 100644
--- a/cc/trees/layer_tree_host_unittest.cc
+++ b/cc/trees/layer_tree_host_unittest.cc
@@ -85,8 +85,8 @@ class LayerTreeHostTestSetNeedsCommit1 : public LayerTreeHostTest {
   }
 
   virtual void AfterTest() OVERRIDE {
-    EXPECT_GE(1, num_commits_);
-    EXPECT_GE(1, num_draws_);
+    EXPECT_GE(num_commits_, 1);
+    EXPECT_GE(num_draws_, 1);
   }
 
  private:
@@ -154,6 +154,7 @@ class LayerTreeHostTestPushPropertiesTo : public LayerTreeHostTest {
 
   virtual void BeginTest() OVERRIDE {
     index_ = STARTUP;
+    index_impl_ = STARTUP;
     PostSetNeedsCommitToMainThread();
   }
 
@@ -166,10 +167,8 @@ class LayerTreeHostTestPushPropertiesTo : public LayerTreeHostTest {
     VerifyBeforeValues(layer_tree_host()->root_layer());
 
     ++index_;
-    if (index_ == DONE) {
+    if (index_ == DONE)
       EndTest();
-      return;
-    }
 
     SetAfterValues(layer_tree_host()->root_layer());
   }
@@ -223,6 +222,7 @@ class LayerTreeHostTestPushPropertiesTo : public LayerTreeHostTest {
   }
 
   int index_;
+  int index_impl_;

[brianderson, 2013/08/05 21:56:19]

I need to revert my changes to this test.

 };
 
 SINGLE_AND_MULTI_THREAD_TEST_F(LayerTreeHostTestPushPropertiesTo);
@@ -621,7 +621,7 @@ class LayerTreeHostTestCompositeAndReadbackWhileInvisible
 
   virtual void BeginTest() OVERRIDE { PostSetNeedsCommitToMainThread(); }
 
-  virtual void DidCommitAndDrawFrame() OVERRIDE {
+  virtual void DidCommit() OVERRIDE {
     num_commits_++;
     if (num_commits_ == 1) {
       layer_tree_host()->SetVisible(false);
@@ -700,7 +700,8 @@ MULTI_THREAD_TEST_F(LayerTreeHostTestCommit);
 class LayerTreeHostTestFrameTimeUpdatesAfterActivationFails
     : public LayerTreeHostTest {
  public:
-  LayerTreeHostTestFrameTimeUpdatesAfterActivationFails() : frame_(0) {}
+  LayerTreeHostTestFrameTimeUpdatesAfterActivationFails()
+  : frame_count_with_pending_tree_(0) {}
 
   virtual void BeginTest() OVERRIDE {
     layer_tree_host()->SetViewportSize(gfx::Size(20, 20));
@@ -709,9 +710,17 @@ class LayerTreeHostTestFrameTimeUpdatesAfterActivationFails
     PostSetNeedsCommitToMainThread();
   }
 
+  virtual void WillBeginFrameOnThread(LayerTreeHostImpl* host_impl,
+                              const BeginFrameArgs &args) OVERRIDE {
+    TRACE_EVENT0("cc", "WillBeginFrameOnThreadbca");
+    if (host_impl->pending_tree())
+      frame_count_with_pending_tree_++;
+  }
+
   virtual void DrawLayersOnThread(LayerTreeHostImpl* impl) OVERRIDE {
-    if (frame_ >= 1) {
-      EXPECT_NE(first_frame_time_, impl->CurrentFrameTimeTicks());
+    if (frame_count_with_pending_tree_ > 1) {
+      EXPECT_NE(first_frame_time_.ToInternalValue(),
+                impl->CurrentFrameTimeTicks().ToInternalValue());
       EndTest();
       return;
     }
@@ -721,33 +730,34 @@ class LayerTreeHostTestFrameTimeUpdatesAfterActivationFails
   }
 
   virtual bool CanActivatePendingTree(LayerTreeHostImpl* impl) OVERRIDE {
-    if (frame_ >= 1)
-      return true;
-
-    return false;
+    return frame_count_with_pending_tree_ > 1;
   }
 
   virtual bool CanActivatePendingTreeIfNeeded(LayerTreeHostImpl* impl)
       OVERRIDE {
-    frame_++;
-    if (frame_ == 1) {
+    if (frame_count_with_pending_tree_ > 1)
+      return true;
+
+    if (first_frame_time_.is_null()) {
       first_frame_time_ = impl->CurrentFrameTimeTicks();
 
       // Since base::TimeTicks::Now() uses a low-resolution clock on
       // Windows, we need to make sure that the clock has incremented past
       // first_frame_time_.
       while (first_frame_time_ == base::TimeTicks::Now()) {}
-
-      return false;
     }
+    return false;
+  }
 
-    return true;
+  virtual void DidActivateTreeOnThread(LayerTreeHostImpl *impl) OVERRIDE {
+    if (impl->settings().impl_side_painting)
+      EXPECT_NE(frame_count_with_pending_tree_, 1);
   }
 
   virtual void AfterTest() OVERRIDE {}
 
  private:
-  int frame_;
+  int frame_count_with_pending_tree_;
   base::TimeTicks first_frame_time_;
 };
 
@@ -824,13 +834,14 @@ class LayerTreeHostTestStartPageScaleAnimation : public LayerTreeHostTest {
 
   virtual void ApplyScrollAndScale(gfx::Vector2d scroll_delta, float scale)
       OVERRIDE {
+    TRACE_EVENT0("cc", "ApplyScrollAndScale9");
     gfx::Vector2d offset = scroll_layer_->scroll_offset();
     scroll_layer_->SetScrollOffset(offset + scroll_delta);
     layer_tree_host()->SetPageScaleFactorAndLimits(scale, 0.5f, 2.f);
   }
 
-  virtual void DidActivateTreeOnThread(LayerTreeHostImpl* impl) OVERRIDE {
-    impl->ProcessScrollDeltas();
+  virtual void SwapBuffersOnThread(LayerTreeHostImpl *impl, bool result)
+      OVERRIDE {
     // We get one commit before the first draw, and the animation doesn't happen
     // until the second draw.
     switch (impl->active_tree()->source_frame_number()) {
@@ -840,10 +851,14 @@ class LayerTreeHostTestStartPageScaleAnimation : public LayerTreeHostTest {
         break;
       case 1:
         EXPECT_EQ(1.f, impl->active_tree()->page_scale_factor());
-        PostSetNeedsRedrawToMainThread();
         break;
       case 2:
+      case 3:
+      case 4:

[brianderson, 2013/08/05 21:56:19]

I will remove case 3 and 4 here. They were an experiment I forgot to remove.

         EXPECT_EQ(1.25f, impl->active_tree()->page_scale_factor());
+        PostSetNeedsCommitToMainThread();
+        break;
+      case 5:
         EndTest();
         break;
       default:
@@ -851,7 +866,7 @@ class LayerTreeHostTestStartPageScaleAnimation : public LayerTreeHostTest {
     }
   }
 
-  virtual void DidCommitAndDrawFrame() OVERRIDE {
+  virtual void DidCommit() OVERRIDE {
     switch (layer_tree_host()->source_frame_number()) {
       case 1:
         layer_tree_host()->SetPageScaleFactorAndLimits(1.f, 0.5f, 2.f);
@@ -1262,7 +1277,7 @@ class LayerTreeHostTestAtomicCommitWithPartialUpdate
     PostSetNeedsCommitToMainThread();
   }
 
-  virtual void DidCommitAndDrawFrame() OVERRIDE {
+  virtual void DidCommit() OVERRIDE {
     switch (layer_tree_host()->source_frame_number()) {
       case 1:
         parent_->SetNeedsDisplay();
@@ -1393,7 +1408,7 @@ class LayerTreeHostTestFinishAllRendering : public LayerTreeHostTest {
     PostSetNeedsCommitToMainThread();
   }
 
-  virtual void DidCommitAndDrawFrame() OVERRIDE {
+  virtual void DidCommit() OVERRIDE {
     if (once_)
       return;
     once_ = true;
@@ -1481,6 +1496,16 @@ class LayerTreeHostTestSurfaceNotAllocatedForLayersOutsideMemoryLimit
     PostSetNeedsCommitToMainThread();
   }
 
+  virtual void DidActivateTreeOnThread(LayerTreeHostImpl *host_impl) OVERRIDE {
+    // Reduce the memory limit to only fit the root layer and one render
+    // surface. This prevents any contents drawing into surfaces
+    // from being allocated.
+    if (host_impl->active_tree()->source_frame_number() == 0) {
+      host_impl->SetMemoryPolicy(
+          ManagedMemoryPolicy(100 * 100 * 4 * 2), true);
+    }
+  }
+
   virtual void DrawLayersOnThread(LayerTreeHostImpl* host_impl) OVERRIDE {
     Renderer* renderer = host_impl->renderer();
     RenderPass::Id surface1_render_pass_id = host_impl->active_tree()
@@ -1495,25 +1520,18 @@ class LayerTreeHostTestSurfaceNotAllocatedForLayersOutsideMemoryLimit
             surface1_render_pass_id));
         EXPECT_TRUE(renderer->HaveCachedResourcesForRenderPassId(
             surface2_render_pass_id));
-
-        // Reduce the memory limit to only fit the root layer and one render
-        // surface. This prevents any contents drawing into surfaces
-        // from being allocated.
-        host_impl->SetMemoryPolicy(
-            ManagedMemoryPolicy(100 * 100 * 4 * 2), true);
         break;
       case 1:
         EXPECT_FALSE(renderer->HaveCachedResourcesForRenderPassId(
             surface1_render_pass_id));
         EXPECT_FALSE(renderer->HaveCachedResourcesForRenderPassId(
             surface2_render_pass_id));
-
         EndTest();
         break;
     }
   }
 
-  virtual void DidCommitAndDrawFrame() OVERRIDE {

[enne (OOO), 2013/08/05 19:37:43]

I want to look through these test changes a little bit more closely, but I
wonder if DidCommitAndDrawFrame should just get removed.  It seems like a hazard
that all of these tests are checking the main thread here, expecting that no
other commit has happened.

[brianderson, 2013/08/05 21:56:19]

DidCommitAndDrawFrame is definitely race prone when we are impl-side-painting. I
found that most tests actually just want to wait for their commit to finish.

If it is possible to transform all the tests that actually need to trigger
actions after draw to perform their actions on the Impl thread when the draw
occurs rather than on the Main thread, that would be awesome.

+  virtual void DidCommit() OVERRIDE {
     if (layer_tree_host()->source_frame_number() < 2)
       root_layer_->SetNeedsDisplay();
   }
@@ -1819,7 +1837,7 @@ class LayerTreeHostTestContinuousInvalidate : public LayerTreeHostTest {
     PostSetNeedsCommitToMainThread();
   }
 
-  virtual void DidCommitAndDrawFrame() OVERRIDE {
+  virtual void DidCommit() OVERRIDE {
     if (num_draw_layers_ == 2)
       return;
     content_layer_->SetNeedsDisplay();
@@ -2215,9 +2233,9 @@ class LayerTreeHostTestBeginFrameNotificationShutdownWhileEnabled
     // The BeginFrame notification is turned off now but will get enabled
     // once we return. End test while it's enabled.
     ImplThreadTaskRunner()->PostTask(
-        FROM_HERE,
-        base::Bind(&LayerTreeHostTestBeginFrameNotification::EndTest,
-                   base::Unretained(this)));
+        FROM_HERE, base::Bind(
+          &LayerTreeHostTestBeginFrameNotificationShutdownWhileEnabled::EndTest,
+          base::Unretained(this)));
   }
 
   virtual void AfterTest() OVERRIDE {}
@@ -2469,7 +2487,7 @@ class LayerTreeHostTestAsyncReadback : public LayerTreeHostTest {
     PostSetNeedsCommitToMainThread();
   }
 
-  virtual void DidCommitAndDrawFrame() OVERRIDE {
+  virtual void DidCommit() OVERRIDE {
     WaitForCallback();
   }
 
@@ -3011,6 +3029,10 @@ class LayerTreeHostTestNumFramesPending : public LayerTreeHostTest {
         // Round 4 done.
         EXPECT_EQ(2, frame_);
         layer_tree_host()->SetNeedsCommit();
+        // We cant SetNeedsRedraw immediately because it will race the commit.
+        break;
+      case 5:
+        EXPECT_EQ(2, frame_);
         layer_tree_host()->SetNeedsRedraw();
         break;
     }
@@ -3325,7 +3347,7 @@ class LayerTreeHostTestLayersPushProperties : public LayerTreeHostTest {
     LayerTreeHostTest::SetupTree();
   }
 
-  virtual void DidCommitAndDrawFrame() OVERRIDE {
+  virtual void DidCommit() OVERRIDE {
     ++num_commits_;
 
     EXPECT_EQ(expected_push_properties_root_, root_->push_properties_count());
@@ -3507,7 +3529,7 @@ class LayerTreeHostTestPropertyChangesDuringUpdateArePushed
     LayerTreeHostTest::SetupTree();
   }
 
-  virtual void DidCommitAndDrawFrame() OVERRIDE {
+  virtual void DidCommit() OVERRIDE {
     switch (layer_tree_host()->source_frame_number()) {
       case 0:
         break;
@@ -3590,7 +3612,7 @@ class LayerTreeHostTestCasePushPropertiesThreeGrandChildren
 class LayerTreeHostTestPushPropertiesAddingToTreeRequiresPush
     : public LayerTreeHostTestCasePushPropertiesThreeGrandChildren {
  protected:
-  virtual void DidCommitAndDrawFrame() OVERRIDE {
+  virtual void DidCommit() OVERRIDE {
     int last_source_frame_number = layer_tree_host()->source_frame_number() - 1;
     switch (last_source_frame_number) {
       case 0:
@@ -3630,7 +3652,7 @@ MULTI_THREAD_TEST_F(LayerTreeHostTestPushPropertiesAddingToTreeRequiresPush);
 class LayerTreeHostTestPushPropertiesRemovingChildStopsRecursion
     : public LayerTreeHostTestCasePushPropertiesThreeGrandChildren {
  protected:
-  virtual void DidCommitAndDrawFrame() OVERRIDE {
+  virtual void DidCommit() OVERRIDE {
     int last_source_frame_number = layer_tree_host()->source_frame_number() - 1;
     switch (last_source_frame_number) {
       case 0:
@@ -3713,7 +3735,7 @@ MULTI_THREAD_TEST_F(LayerTreeHostTestPushPropertiesRemovingChildStopsRecursion);
 class LayerTreeHostTestPushPropertiesRemovingChildStopsRecursionWithPersistence
     : public LayerTreeHostTestCasePushPropertiesThreeGrandChildren {
  protected:
-  virtual void DidCommitAndDrawFrame() OVERRIDE {
+  virtual void DidCommit() OVERRIDE {
     int last_source_frame_number = layer_tree_host()->source_frame_number() - 1;
     switch (last_source_frame_number) {
       case 0:
@@ -3761,7 +3783,7 @@ MULTI_THREAD_TEST_F(
 class LayerTreeHostTestPushPropertiesSetPropertiesWhileOutsideTree
     : public LayerTreeHostTestCasePushPropertiesThreeGrandChildren {
  protected:
-  virtual void DidCommitAndDrawFrame() OVERRIDE {
+  virtual void DidCommit() OVERRIDE {
     int last_source_frame_number = layer_tree_host()->source_frame_number() - 1;
     switch (last_source_frame_number) {
       case 0:
@@ -3829,7 +3851,7 @@ MULTI_THREAD_TEST_F(
 class LayerTreeHostTestPushPropertiesSetPropertyInParentThenChild
     : public LayerTreeHostTestCasePushPropertiesThreeGrandChildren {
  protected:
-  virtual void DidCommitAndDrawFrame() OVERRIDE {
+  virtual void DidCommit() OVERRIDE {
     int last_source_frame_number = layer_tree_host()->source_frame_number() - 1;
     switch (last_source_frame_number) {
       case 0:
@@ -3893,7 +3915,7 @@ MULTI_THREAD_TEST_F(
 class LayerTreeHostTestPushPropertiesSetPropertyInChildThenParent
     : public LayerTreeHostTestCasePushPropertiesThreeGrandChildren {
  protected:
-  virtual void DidCommitAndDrawFrame() OVERRIDE {
+  virtual void DidCommit() OVERRIDE {
     int last_source_frame_number = layer_tree_host()->source_frame_number() - 1;
     switch (last_source_frame_number) {
       case 0: