Skip to content

GitLab

T
tangled
Commit 97ad1bab authored by Zoe Clifford's avatar Zoe Clifford Committed by Commit Bot
Browse files
Options

Unpause virtual time in PendingScript earlier. 

Currently there is a virtual time deadlock between HTML imports and
PendingScript. I'm not sure about modules, but at least classic
PendingScript is effected.

* PendingScript will keep virtual time paused until it's ready to be
  executed (Until the document is not script-blocking).
* Script execution will be paused on the document until the HTML import
  has finished.
* The HTML import will not finish unless virtual time is unpaused
  (It fires an immediate timer to update it's state)

The deadlock happens because timers (even immediate ones) will not fire
if they were scheduled after virtual time was paused.

To resolve this this CL unpauses PendingScript's virtual time pauser
earlier; once the script is marked "ready". This means that
PendingScript will unpause virtual time before waiting for HTML import
to finish, and the deadlock is resolved.

Change-Id: I1763229bd73096000bc461c678b061a345c3b530
Reviewed-on: https://chromium-review.googlesource.com/1104314Reviewed-by: default avatarAlex Clarke <alexclarke@chromium.org>
Reviewed-by: default avatarPavel Feldman <pfeldman@chromium.org>
Commit-Queue: Zoe Clifford <zoeclifford@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569630}
parent 162d50fd
Hide whitespace changes
Inline Side-by-side
Showing with 77 additions and 5 deletions
headless/test/data/protocol/emulation/virtual-time-html-import-expected.txt 0 → 100644
View file @ 97ad1bab
Tests that an html import followed by a pending script does not break virtual time.
imported html
ran script
ran module
\ No newline at end of file
headless/test/data/protocol/emulation/virtual-time-html-import.js 0 → 100644
View file @ 97ad1bab
// Copyright 2018 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.
const htmlWithScript =
`<!DOCTYPE html>
<link rel="import" href="/import.html">
<script src="/script.js"></script>
<script type="module" src="/module.js"></script>`;
const htmlImport =
`<script>console.log("imported html");</script>`
const script =
`console.log("ran script");`
const module =
`console.log("ran module");`
const server = new Map([
['http://test.com/index.html', { body: htmlWithScript }],
['http://test.com/import.html', { body: htmlImport }],
['http://test.com/script.js', { body: script }],
['http://test.com/module.js',
{ body: module,
headers: ['HTTP/1.1 200 OK', 'Content-Type: application/javascript'] }
]]);
(async function(testRunner) {
var {page, session, dp} = await testRunner.startBlank(
`Tests that an html import followed by a pending script does not break ` +
`virtual time.`);
await dp.Runtime.enable();
await dp.Page.enable();
await dp.Network.enable();
await dp.Network.setRequestInterception({ patterns: [{ urlPattern: '*' }] });
dp.Network.onRequestIntercepted(event => {
let body = server.get(event.params.request.url).body || '';
let headers = server.get(event.params.request.url).headers || [];
dp.Network.continueInterceptedRequest({
interceptionId: event.params.interceptionId,
rawResponse: btoa(headers.join('\r\n') + '\r\n\r\n' + body)
});
});
dp.Runtime.onConsoleAPICalled(data => {
const text = data.params.args[0].value;
testRunner.log(text);
});
dp.Emulation.onVirtualTimeBudgetExpired(async data => {
testRunner.completeTest();
});
await dp.Emulation.setVirtualTimePolicy({policy: 'pause'});
await dp.Emulation.setVirtualTimePolicy({
policy: 'pauseIfNetworkFetchesPending', budget: 5000,
waitForNavigation: true});
dp.Page.navigate({url: 'http://test.com/index.html'});
})
headless/test/headless_protocol_browsertest.cc
View file @ 97ad1bab
...@@ -210,6 +210,8 @@ HEADLESS_PROTOCOL_TEST(VirtualTimeLocalStorage, ...@@ -210,6 +210,8 @@ HEADLESS_PROTOCOL_TEST(VirtualTimeLocalStorage,
"emulation/virtual-time-local-storage.js"); "emulation/virtual-time-local-storage.js");
HEADLESS_PROTOCOL_TEST(VirtualTimePendingScript, HEADLESS_PROTOCOL_TEST(VirtualTimePendingScript,
"emulation/virtual-time-pending-script.js"); "emulation/virtual-time-pending-script.js");
HEADLESS_PROTOCOL_TEST(VirtualTimeHtmlImport,
"emulation/virtual-time-html-import.js");
HEADLESS_PROTOCOL_TEST(VirtualTimeRedirect, HEADLESS_PROTOCOL_TEST(VirtualTimeRedirect,
"emulation/virtual-time-redirect.js"); "emulation/virtual-time-redirect.js");
HEADLESS_PROTOCOL_TEST(VirtualTimeSessionStorage, HEADLESS_PROTOCOL_TEST(VirtualTimeSessionStorage,
......
third_party/blink/renderer/core/script/classic_pending_script.cc
View file @ 97ad1bab
...@@ -349,7 +349,7 @@ void ClassicPendingScript::AdvanceReadyState(ReadyState new_ready_state) { ...@@ -349,7 +349,7 @@ void ClassicPendingScript::AdvanceReadyState(ReadyState new_ready_state) {
// Did we transition into a 'ready' state? // Did we transition into a 'ready' state?
if (IsReady() && !old_is_ready && IsWatchingForLoad()) if (IsReady() && !old_is_ready && IsWatchingForLoad())
Client()->PendingScriptFinished(this); PendingScriptFinished();
// Did we finish streaming? // Did we finish streaming?
if (IsCurrentlyStreaming()) { if (IsCurrentlyStreaming()) {
......
third_party/blink/renderer/core/script/module_pending_script.cc
View file @ 97ad1bab
...@@ -62,9 +62,7 @@ void ModulePendingScript::Trace(blink::Visitor* visitor) { ...@@ -62,9 +62,7 @@ void ModulePendingScript::Trace(blink::Visitor* visitor) {
void ModulePendingScript::NotifyModuleTreeLoadFinished() { void ModulePendingScript::NotifyModuleTreeLoadFinished() {
CHECK(!IsReady()); CHECK(!IsReady());
ready_ = true; ready_ = true;
PendingScriptFinished();
if (Client())
Client()->PendingScriptFinished(this);
} }
Script* ModulePendingScript::GetSource(const KURL& document_url, Script* ModulePendingScript::GetSource(const KURL& document_url,
......
third_party/blink/renderer/core/script/pending_script.cc
View file @ 97ad1bab
...@@ -89,7 +89,7 @@ void PendingScript::WatchForLoad(PendingScriptClient* client) { ...@@ -89,7 +89,7 @@ void PendingScript::WatchForLoad(PendingScriptClient* client) {
// notifyFinished and further stopWatchingForLoad(). // notifyFinished and further stopWatchingForLoad().
client_ = client; client_ = client;
if (IsReady()) { if (IsReady()) {
client_->PendingScriptFinished(this); PendingScriptFinished();
} else { } else {
virtual_time_pauser_.PauseVirtualTime(); virtual_time_pauser_.PauseVirtualTime();
} }
...@@ -104,6 +104,13 @@ void PendingScript::StopWatchingForLoad() { ...@@ -104,6 +104,13 @@ void PendingScript::StopWatchingForLoad() {
virtual_time_pauser_.UnpauseVirtualTime(); virtual_time_pauser_.UnpauseVirtualTime();
} }
void PendingScript::PendingScriptFinished() {
virtual_time_pauser_.UnpauseVirtualTime();
if (client_) {
client_->PendingScriptFinished(this);
}
}
ScriptElementBase* PendingScript::GetElement() const { ScriptElementBase* PendingScript::GetElement() const {
// As mentioned in the comment at |m_element| declaration, // As mentioned in the comment at |m_element| declaration,
// |m_element| must point to the corresponding ScriptLoader's // |m_element| must point to the corresponding ScriptLoader's
......
third_party/blink/renderer/core/script/pending_script.h
View file @ 97ad1bab
...@@ -79,6 +79,7 @@ class CORE_EXPORT PendingScript ...@@ -79,6 +79,7 @@ class CORE_EXPORT PendingScript
void WatchForLoad(PendingScriptClient*); void WatchForLoad(PendingScriptClient*);
void StopWatchingForLoad(); void StopWatchingForLoad();
void PendingScriptFinished();
ScriptElementBase* GetElement() const; ScriptElementBase* GetElement() const;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment