Plumb activation time to main

cc/trees/layer_tree_host_unittest_proxy.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "cc/test/fake_content_layer_client.h"
 #include "cc/test/fake_picture_layer.h"
 #include "cc/test/layer_tree_test.h"
 #include "cc/trees/proxy_impl.h"
@@ skipping 300 matching lines @@
 
  private:
   base::Lock activate_blocked_lock_;
   bool activate_blocked_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(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:
   LayerTreeHostProxyTestCommitWaitsForActivationMFBA() = default;
 
   void InitializeSettings(LayerTreeSettings* settings) override {
     settings->main_frame_before_activation_enabled = true;
     LayerTreeHostProxyTest::InitializeSettings(settings);
   }
 
@@ skipping 78 matching lines @@
 
  private:
   base::Lock activate_blocked_lock_;
   bool activate_blocked_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(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