Deliver IPC messages together with SwapCompositorFrame

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 b194491d7606763193f75e034c965c5b022ebeb7..3d32cbcbcdfb02bdb811a53827f408929d642a27 100644
--- a/cc/trees/layer_tree_host_unittest.cc
+++ b/cc/trees/layer_tree_host_unittest.cc
@@ -4484,11 +4484,11 @@ class LayerTreeHostTestBreakSwapPromise : public LayerTreeHostTest {
     }
 
     {
-      // The second commit aborts.
+      // The second commit is aborted since it contains no updates.
       base::AutoLock lock(swap_promise_result_[1].lock);
       EXPECT_FALSE(swap_promise_result_[1].did_swap_called);
       EXPECT_TRUE(swap_promise_result_[1].did_not_swap_called);
-      EXPECT_EQ(SwapPromise::COMMIT_FAILS, swap_promise_result_[1].reason);
+      EXPECT_EQ(SwapPromise::COMMIT_NO_UPDATE, swap_promise_result_[1].reason);
       EXPECT_TRUE(swap_promise_result_[1].dtor_called);
     }
 
@@ -4510,6 +4510,53 @@ class LayerTreeHostTestBreakSwapPromise : public LayerTreeHostTest {
 
 MULTI_THREAD_TEST_F(LayerTreeHostTestBreakSwapPromise);
 
+class LayerTreeHostTestBreakSwapPromiseForAbortedCommit
+    : public LayerTreeHostTest {
+ protected:
+  LayerTreeHostTestBreakSwapPromiseForAbortedCommit() : commit_count_(0) {}
+
+  virtual void InitializeSettings(LayerTreeSettings* settings) OVERRIDE {
+    settings->throttle_frame_production = false;
+  }
+
+  virtual void BeginTest() OVERRIDE { PostSetNeedsCommitToMainThread(); }
+
+  virtual void DidCommit() OVERRIDE {
+    commit_count_++;
+    if (commit_count_ == 1) {
+      scoped_ptr<SwapPromise> swap_promise(
+          new TestSwapPromise(&swap_promise_result_));
+      layer_tree_host()->QueueSwapPromise(swap_promise.Pass());
+    }
+  }
+
+  virtual void BeginMainFrameAbortedOnThread(LayerTreeHostImpl* host_impl,
+                                             bool did_handle) OVERRIDE {
+    EndTest();
+  }
+
+  virtual void CommitCompleteOnThread(LayerTreeHostImpl* host_impl) OVERRIDE {
+    // Initiate an abortable commit.

[piman, 2014/06/25 21:20:32]

What causes this commit to be aborted?

[mkosiba (inactive), 2014/06/25 22:08:33]

I'd be thrilled to know too, to be honest. I spent some time trying to figure
out a way to force a commit to EarlyOut due to visibility=false, but couldn't
get that to happen so I nicked bits from
LayerTreeHostTestAbortedCommitDoesntStall.

The magic sauce seems to be setting throttle_frame_production to false and
calling host_impl->SetNeedsCommit from here.

[mkosiba (inactive), 2014/06/26 12:36:47]

Ok, so I think this has something to do with the scheduler not going through
with the commit on the impl thread. From ThreadProxy perspective the
BeginMainFrame method runs to completion but the trees aren't actually swapped,
so we get a COMMIT_FAILS callback.

I tried finding a test that actually hits the EarlyOut_NotVisible path in
ThreadProxy::BeginMainFrame, the only one that does is
LayerTreeHostAnimationTestAnimationTickTimeIsMonotonic but the code there looks
equally unintuitive to me as this.

[piman, 2014/06/26 20:40:12]

Mmh, I think the case where that happens is the EarlyOut_NoLayerTree in
ThreadProxy::StartCommitOnImplThread
Not sure why we get into it though, is it because we destroy the compositor
before the commit takes place? Not sure why that would be.
I'm worried this may be due to a race and would cause a flaky test.
EarlyOut_NotVisible is essentially to handle a race:
1- main thread asked for a commit while visible
2- impl thread posted the task to the main thread to start the commit
3- main thread calls SetVisible(false)
4- main thread executes the task.

It's hard to trigger though, because (3) will also post a task to the impl
thread to basically tell it not to do commits.

Unless you add a test hook, I think the way you can test is by:
- LTH::SetDeferCommit(true)
- LTH::SetNeedsCommit()

in the DidDeferCommit hook:
- LTH::SetVisible(false)
- LTH::SetDeferCommit(false)

The last call will re-post the task to do the commit at which point we'll get
into the EarlyOut_NotVisible case.

[mkosiba (inactive), 2014/06/27 14:18:59]

That works, thanks!

+    host_impl->SetNeedsCommit();
+  }
+
+  virtual void AfterTest() OVERRIDE {
+    EXPECT_EQ(commit_count_, 2);
+    {
+      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_;
+};
+
+MULTI_THREAD_TEST_F(LayerTreeHostTestBreakSwapPromiseForAbortedCommit);
+
 class SimpleSwapPromiseMonitor : public SwapPromiseMonitor {
  public:
   SimpleSwapPromiseMonitor(LayerTreeHost* layer_tree_host,