cc: Control defer_commits logic by Scheduler

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 62f567e88f8e16fb2940d5831af84250f3c8536e..baec8f658dbc6ca6090eecd64f782ef63301deaa 100644
--- a/cc/trees/layer_tree_host_unittest.cc
+++ b/cc/trees/layer_tree_host_unittest.cc
@@ -2023,23 +2023,27 @@ SINGLE_AND_MULTI_THREAD_TEST_F(LayerTreeHostTestContinuousInvalidate);
 
 class LayerTreeHostTestDeferCommits : public LayerTreeHostTest {
  public:
-  LayerTreeHostTestDeferCommits()
-      : num_commits_deferred_(0), num_complete_commits_(0) {}
+  LayerTreeHostTestDeferCommits() : num_complete_commits_(0) {}
 
   void BeginTest() override { PostSetNeedsCommitToMainThread(); }
 
-  void DidDeferCommit() override {
-    num_commits_deferred_++;
-    layer_tree_host()->SetDeferCommits(false);
+  void DidSetVisibleOnImplTree(LayerTreeHostImpl* host_impl,
+                               bool visible) override {
+    if (visible)
+      return;
+
+    PostSetDeferCommitsToMainThread(false);
+    PostSetNeedsCommitToMainThread();

[brianderson, 2014/12/03 02:12:11]

We should make sure we don't need to PostSetNeedsCommitToMainThread to make
forward progress.The scheduler should do that automatically after we stop
deferring commits. Please delete this line.

[simonhong, 2015/01/19 14:53:33]

Yep, this is not needed.
When defer commit is turned off, new commit will be started based on following
condition.
* BeginMainFrame is aborted because of defer commit.
* When new commit is requested during defer commit is off.

+    PostSetVisibleToMainThread(true);
   }
 
   void DidCommit() override {
     num_complete_commits_++;
     switch (num_complete_commits_) {
       case 1:
-        EXPECT_EQ(0, num_commits_deferred_);
         layer_tree_host()->SetDeferCommits(true);
         PostSetNeedsCommitToMainThread();
+        PostSetVisibleToMainThread(false);

[brianderson, 2014/12/03 02:12:11]

Instead of playing with visibility in this test, it would be better to post a
delayed task, say 100ms later, and verify WillBeginMainFrame is never called.

[simonhong, 2015/01/19 14:53:33]

Done.

         break;
       case 2:
         EndTest();
@@ -2051,12 +2055,10 @@ class LayerTreeHostTestDeferCommits : public LayerTreeHostTest {
   }
 
   void AfterTest() override {
-    EXPECT_EQ(1, num_commits_deferred_);
     EXPECT_EQ(2, num_complete_commits_);
   }
 
  private:
-  int num_commits_deferred_;
   int num_complete_commits_;
 };
 
@@ -4723,96 +4725,6 @@ class LayerTreeHostTestBreakSwapPromise : public LayerTreeHostTest {
 
 MULTI_THREAD_TEST_F(LayerTreeHostTestBreakSwapPromise);
 
-class LayerTreeHostTestBreakSwapPromiseForVisibilityAbortedCommit

[brianderson, 2014/12/03 02:12:11]

Please don't delete these tests. The new scheduler tests don't cover what these
two tests cover.

Instead of using SetDeferCommits, can you set the Visibility instead to trigger
the aborted commits?

[simonhong, 2015/01/19 14:53:33]

I think scenarios that these two tests want to simulate cannot be occurred in
real.
These tests cover visibility change from true to false when there is a deferred
commit.
These tests will only have a meaning when pending(deferred) commit is resumed
with invisible state.
With this cl, however, deferred commit is not pending.
It will be aborted and rescheduled when deferred is turned off.
Of course, new frame will not be started with invisible state when deferring is
turned off.
So, I think this test can be removed.

[enne (OOO), 2015/01/23 01:35:21]

I do not think this is true.  These tests use deferred commits to control when
commits happen, as commits happen immediately when undeferring.

The goal of these tests is to make sure that swap promises get the right result
during aborted commits.  The use of deferred commits is incidental, I think.

[simonhong, 2015/01/28 19:44:24]

Done.

-    : public LayerTreeHostTest {
- protected:
-  void BeginTest() override { PostSetNeedsCommitToMainThread(); }
-
-  void DidCommit() override {
-    layer_tree_host()->SetDeferCommits(true);
-    layer_tree_host()->SetNeedsCommit();
-  }
-
-  void DidDeferCommit() override {
-    layer_tree_host()->SetVisible(false);
-    scoped_ptr<SwapPromise> swap_promise(
-        new TestSwapPromise(&swap_promise_result_));
-    layer_tree_host()->QueueSwapPromise(swap_promise.Pass());
-    layer_tree_host()->SetDeferCommits(false);
-  }
-
-  void BeginMainFrameAbortedOnThread(LayerTreeHostImpl* host_impl,
-                                     bool did_handle) override {
-    EndTest();
-  }
-
-  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);
-    }
-  }
-
-  TestSwapPromiseResult swap_promise_result_;
-};
-
-SINGLE_AND_MULTI_THREAD_TEST_F(
-    LayerTreeHostTestBreakSwapPromiseForVisibilityAbortedCommit);
-
-class LayerTreeHostTestBreakSwapPromiseForContextAbortedCommit
-    : public LayerTreeHostTest {
- protected:
-  void BeginTest() override { PostSetNeedsCommitToMainThread(); }
-
-  void DidCommit() override {
-    if (TestEnded())
-      return;
-    layer_tree_host()->SetDeferCommits(true);
-    layer_tree_host()->SetNeedsCommit();
-  }
-
-  void DidDeferCommit() override {
-    layer_tree_host()->DidLoseOutputSurface();
-    scoped_ptr<SwapPromise> swap_promise(
-        new TestSwapPromise(&swap_promise_result_));
-    layer_tree_host()->QueueSwapPromise(swap_promise.Pass());
-    layer_tree_host()->SetDeferCommits(false);
-  }
-
-  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);
-  }
-
-  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);
-    }
-  }
-
-  TestSwapPromiseResult swap_promise_result_;
-};
-
-SINGLE_AND_MULTI_THREAD_TEST_F(
-    LayerTreeHostTestBreakSwapPromiseForContextAbortedCommit);
-
 class SimpleSwapPromiseMonitor : public SwapPromiseMonitor {
  public:
   SimpleSwapPromiseMonitor(LayerTreeHost* layer_tree_host,