Plumb activation time to main

cc/trees/layer_tree_host_unittest_proxy.cc

Index: cc/trees/layer_tree_host_unittest_proxy.cc
diff --git a/cc/trees/layer_tree_host_unittest_proxy.cc b/cc/trees/layer_tree_host_unittest_proxy.cc
index 27e28354534959926f3078947f4d95cb6afe10bf..0e94353864d1973ced23f318ea61ae2eb9d5f518 100644
--- a/cc/trees/layer_tree_host_unittest_proxy.cc
+++ b/cc/trees/layer_tree_host_unittest_proxy.cc
@@ -318,10 +318,6 @@ class LayerTreeHostProxyTestCommitWaitsForActivation
 
 MULTI_THREAD_TEST_F(LayerTreeHostProxyTestCommitWaitsForActivation);
 
-// Test for a corner case of main frame before activation (MFBA) and commit
-// waits for activation. If a commit (with wait for activation flag set)
-// is ready before the activation for a previous commit then the activation
-// should not signal the completion event of the second commit.
 class LayerTreeHostProxyTestCommitWaitsForActivationMFBA
     : public LayerTreeHostProxyTest {
  protected:
@@ -420,4 +416,59 @@ class LayerTreeHostProxyTestCommitWaitsForActivationMFBA
 
 MULTI_THREAD_TEST_F(LayerTreeHostProxyTestCommitWaitsForActivationMFBA);
 
+// Test for activation time passed in BeginFrameArgs.
+class LayerTreeHostProxyTestActivationTime : public LayerTreeHostProxyTest {

[brianderson, 2017/04/13 22:36:37]

I think you'll want to delay activation until after two BeginMainFrames (as
opposed to commits), then delay the third BeginMainFrame until after two
activations.

The DidSendBeginMainFrameOnThread hook might be useful.

[panicker, 2017/04/14 22:02:45]

Not sure how this would work, when activation is blocked (say in
DidSendBeginMainFrameOnThread) I cannot get the next DidCommit etc to trigger
until activation is unblocked (regardless of calling
SetNextCommitWaitsForActivation) 
i.e. cannot get 2 consequent frames DidCommit+DidBeginMainFrame without
unblocking activation.
Maybe there's a way to do this.
Though unless you have specific suggestion here let's start with this one test?

[brianderson, 2017/04/14 23:34:28]

============
Re: test for two ready_to_activate_time entires:
============
I'm not sure why the MFBA test uses a delayed task, but I think we can avoid it
in our case. The following is a more detailed description of what you should be
able to do to get two entries in ready_to_activate_time.

In this BeginCommitOnThread method, for source_frame_number 1:
a) BlockNotifyReadyToActivate(true)
b) PostSetNeedsCommitToMainThread().
c) Set a activate_blocked_ to true.

Then, in DidSendBeginMainFrameOnThread, when activate_blocked_ is true:
d) Call BlockNotifyReadyToActivate(false).
e) Set is_blocked to false.

At this point there will be a pending tree and a BeginMainFrame on its way to
the main thread.

Then, in DidActivateTreeOnThread, when the active_tree source_frame_number is 2:
f) PostSetNeedsCommitToMainThread().

Then, in DidBeginMainFrame, when source_frame_number is 3:
g) Verify that there are two entries in ready_to_activate_time.

============
Re: test for one ready_to_activate_time entires:
============
Remove steps a-e. And check for source_frame_number 2 in g.
Calling PostSetNeedsCommitToMainThread from DidActivateTreeOnThread avoids the
race between activation and the next BeginMainFrame.

+ protected:
+  LayerTreeHostProxyTestActivationTime() = default;
+
+  void BeginTest() override { PostSetNeedsCommitToMainThread(); }
+
+  void DidBeginMainFrame() override {
+    BeginFrameArgs bfargs = GetProxyMain()->BeginFrameArgsForTesting();
+    switch (bfargs.source_frame_number) {
+      case 0:
+        // Initial frame, no activation yet.
+        EXPECT_EQ(0UL, bfargs.ready_to_activate_time.size());
+        break;
+      case 1:
+        // Frame #1, since activation was delayed, nothing here yet.

[panicker, 2017/04/13 22:09:22]

I didn't actually delay this (I removed the delay that existed before), is
ready_to_activate really expected to be empty here?

[brianderson, 2017/04/13 22:36:37]

It could be since there's a race between whether activation or the next
BeginMainFrame occurs first. If the first activation is slow, it would lose the
race pretty often.

[panicker, 2017/04/14 22:02:45]

I've added back the explicit delay to make this deterministic.
you expect this to make the test flaky right?

+        EXPECT_EQ(0UL, bfargs.ready_to_activate_time.size());
+        break;
+      case 2:
+        // Frame #2, activation for Frame #1
+        EXPECT_EQ(1UL, bfargs.ready_to_activate_time.size());
+        EXPECT_EQ(1UL, bfargs.ready_to_activate_time.begin()->first);
+        break;
+      case 3:
+        // Frame #3, activation for Frame #2
+        EXPECT_EQ(1UL, bfargs.ready_to_activate_time.size());
+        EXPECT_EQ(2UL, bfargs.ready_to_activate_time.begin()->first);
+        EndTest();
+    }
+  }
+
+  void DidCommit() override {
+    switch (layer_tree_host()->SourceFrameNumber()) {
+      case 1:
+        // Request a new commit.
+        layer_tree_host()->SetNeedsCommit();
+        break;
+      case 2:
+        // Request a new commit.
+        layer_tree_host()->SetNeedsCommit();
+        break;
+      case 3:
+        layer_tree_host()->SetNeedsCommit();
+        break;
+    }
+  }
+
+  void AfterTest() override {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(LayerTreeHostProxyTestActivationTime);
+};
+
+MULTI_THREAD_TEST_F(LayerTreeHostProxyTestActivationTime);
+
 }  // namespace cc