Fix bug Emulation.setVirtualTimePolicy and waitForNavigation

If only a single call to Emulation.setVirtualTimePolicy is made with
waitForNavigation:true then addInspectorEmulationAgent wasn't getting
called in time, which meant InspectorEmulationAgent::FrameStartedLoading
wasn't called for the navigation, which meant the virtual time policy
was never changing.

Bug: 777763
Change-Id: I7525ed1de924319c195ecfdd602c3fad808eba59
Reviewed-on: https://chromium-review.googlesource.com/918665Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Alex Clarke <alexclarke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536721}

headless/lib/virtual_time_browsertest.cc

@@ -46,32 +46,25 @@ class VirtualTimeBrowserTest : public HeadlessAsyncDevTooledBrowserTest,
     if (!runtime_enabled)
       return;
 
-    // To avoid race conditions start with virtual time paused.
-    devtools_client_->GetEmulation()->GetExperimental()->SetVirtualTimePolicy(
-        emulation::SetVirtualTimePolicyParams::Builder()
-            .SetPolicy(emulation::VirtualTimePolicy::PAUSE)
-            .Build(),
-        base::BindRepeating(&VirtualTimeBrowserTest::SetVirtualTimePolicyDone,
-                            base::Unretained(this)));
-
-    SetAfterLoadVirtualTimePolicy();
+    SetVirtualTimePolicy();
   }
 
   void SetVirtualTimePolicyDone(
       std::unique_ptr<emulation::SetVirtualTimePolicyResult> result) {
-    EXPECT_LT(0, result->GetVirtualTimeBase());
     // Virtual time is paused, so start navigating.
     devtools_client_->GetPage()->Navigate(initial_url_);
   }
 
-  virtual void SetAfterLoadVirtualTimePolicy() {
+  virtual void SetVirtualTimePolicy() {
     devtools_client_->GetEmulation()->GetExperimental()->SetVirtualTimePolicy(
         emulation::SetVirtualTimePolicyParams::Builder()
             .SetPolicy(
                 emulation::VirtualTimePolicy::PAUSE_IF_NETWORK_FETCHES_PENDING)
             .SetBudget(5000)
             .SetWaitForNavigation(true)
-            .Build());
+            .Build(),
+        base::BindRepeating(&VirtualTimeBrowserTest::SetVirtualTimePolicyDone,
+                            base::Unretained(this)));
   }
 
   // runtime::Observer implementation:
@@ -149,7 +142,7 @@ class MaxVirtualTimeTaskStarvationCountTest : public VirtualTimeBrowserTest {
                       .spec());
   }
 
-  void SetAfterLoadVirtualTimePolicy() override {
+  void SetVirtualTimePolicy() override {
     devtools_client_->GetEmulation()->GetExperimental()->SetVirtualTimePolicy(
         emulation::SetVirtualTimePolicyParams::Builder()
             .SetPolicy(
@@ -157,7 +150,9 @@ class MaxVirtualTimeTaskStarvationCountTest : public VirtualTimeBrowserTest {
             .SetBudget(4001)
             .SetMaxVirtualTimeTaskStarvationCount(100)
             .SetWaitForNavigation(true)
-            .Build());
+            .Build(),
+        base::BindRepeating(&VirtualTimeBrowserTest::SetVirtualTimePolicyDone,
+                            base::Unretained(this)));
   }
 
   // emulation::Observer implementation:
@@ -276,16 +271,6 @@ class VirtualTimeLocalStorageTest : public VirtualTimeBrowserTest {
                       .spec());
   }
 
-  void SetAfterLoadVirtualTimePolicy() override {
-    devtools_client_->GetEmulation()->GetExperimental()->SetVirtualTimePolicy(
-        emulation::SetVirtualTimePolicyParams::Builder()
-            .SetPolicy(
-                emulation::VirtualTimePolicy::PAUSE_IF_NETWORK_FETCHES_PENDING)
-            .SetBudget(5000)
-            .SetWaitForNavigation(true)
-            .Build());
-  }
-
   void OnConsoleAPICalled(
       const runtime::ConsoleAPICalledParams& params) override {
     EXPECT_EQ(runtime::ConsoleAPICalledType::LOG, params.GetType());
@@ -322,16 +307,6 @@ class VirtualTimeSessionStorageTest : public VirtualTimeBrowserTest {
                       .spec());
   }
 
-  void SetAfterLoadVirtualTimePolicy() override {
-    devtools_client_->GetEmulation()->GetExperimental()->SetVirtualTimePolicy(
-        emulation::SetVirtualTimePolicyParams::Builder()
-            .SetPolicy(
-                emulation::VirtualTimePolicy::PAUSE_IF_NETWORK_FETCHES_PENDING)
-            .SetBudget(5000)
-            .SetWaitForNavigation(true)
-            .Build());
-  }
-
   void OnConsoleAPICalled(
       const runtime::ConsoleAPICalledParams& params) override {
     EXPECT_EQ(runtime::ConsoleAPICalledType::LOG, params.GetType());
@@ -690,7 +665,7 @@ class VirtualTimeAndResourceErrorLoopTest : public VirtualTimeBrowserTest {
     VirtualTimeBrowserTest::RunDevTooledTest();
   }
 
-  void SetAfterLoadVirtualTimePolicy() override {
+  void SetVirtualTimePolicy() override {
     devtools_client_->GetEmulation()->GetExperimental()->SetVirtualTimePolicy(
         emulation::SetVirtualTimePolicyParams::Builder()
             .SetPolicy(
@@ -698,7 +673,9 @@ class VirtualTimeAndResourceErrorLoopTest : public VirtualTimeBrowserTest {
             .SetBudget(5000)
             .SetMaxVirtualTimeTaskStarvationCount(1000000)  // Prevent flakes.
             .SetWaitForNavigation(true)
-            .Build());
+            .Build(),
+        base::BindRepeating(&VirtualTimeBrowserTest::SetVirtualTimePolicyDone,
+                            base::Unretained(this)));
   }
 
   // emulation::Observer implementation:
@@ -777,16 +754,6 @@ class JavascriptTimeAlwaysAdvancesTest : public VirtualTimeBrowserTest,
     builder.SetInitialVirtualTime(base::Time::FromJsTime(1000000.0));
   }
 
-  void SetAfterLoadVirtualTimePolicy() override {
-    devtools_client_->GetEmulation()->GetExperimental()->SetVirtualTimePolicy(
-        emulation::SetVirtualTimePolicyParams::Builder()
-            .SetPolicy(
-                emulation::VirtualTimePolicy::PAUSE_IF_NETWORK_FETCHES_PENDING)
-            .SetBudget(5000)
-            .SetWaitForNavigation(true)
-            .Build());
-  }
-
   void OnConsoleAPICalled(
       const runtime::ConsoleAPICalledParams& params) override {
     EXPECT_EQ(runtime::ConsoleAPICalledType::LOG, params.GetType());

third_party/WebKit/Source/core/inspector/InspectorEmulationAgent.cpp

@@ -153,6 +153,12 @@ Response InspectorEmulationAgent::setVirtualTimePolicy(
         max_virtual_time_task_starvation_count.fromJust();
   }
 
+  if (!virtual_time_setup_) {
+    instrumenting_agents_->addInspectorEmulationAgent(this);
+    web_local_frame_->View()->Scheduler()->AddVirtualTimeObserver(this);
+    virtual_time_setup_ = true;
+  }
+
   if (wait_for_navigation.fromMaybe(false)) {
     *virtual_time_base_ms = 0;
     pending_virtual_time_policy_ = std::move(new_policy);
@@ -172,11 +178,6 @@ WTF::TimeTicks InspectorEmulationAgent::ApplyVirtualTimePolicy(
       new_policy.policy);
   WTF::TimeTicks virtual_time_base_ticks(
       web_local_frame_->View()->Scheduler()->EnableVirtualTime());
-  if (!virtual_time_setup_) {
-    instrumenting_agents_->addInspectorEmulationAgent(this);
-    web_local_frame_->View()->Scheduler()->AddVirtualTimeObserver(this);
-    virtual_time_setup_ = true;
-  }
   if (new_policy.virtual_time_budget_ms) {
     WTF::TimeDelta budget_amount =
         WTF::TimeDelta::FromMillisecondsD(*new_policy.virtual_time_budget_ms);