Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mark the page source as loaded only after parsing is done #15098

Merged
merged 3 commits into from Jan 19, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 11 additions & 7 deletions components/script/dom/document.rs
Expand Up @@ -132,6 +132,7 @@ use style::restyle_hints::RestyleHint;
use style::selector_parser::{RestyleDamage, Snapshot};
use style::str::{split_html_space_chars, str_join};
use style::stylesheets::Stylesheet;
use task_source::TaskSource;
use time;
use url::percent_encoding::percent_decode;

Expand Down Expand Up @@ -1572,13 +1573,11 @@ impl Document {
}

if !self.loader.borrow().is_blocked() && !self.loader.borrow().events_inhibited() {
// Schedule a task to fire a "load" event (if no blocking loads have arrived in the mean time)
// NOTE: we can end up executing this code more than once, in case more blocking loads arrive.
self.loader.borrow_mut().inhibit_events();
// Schedule a task to fire a "load" event.
debug!("Document loads are complete.");
let win = self.window();
let msg = MainThreadScriptMsg::DocumentLoadsComplete(
win.upcast::<GlobalScope>().pipeline_id());
win.main_thread_script_chan().send(msg).unwrap();
let handler = box DocumentProgressHandler::new(Trusted::new(self));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should remove this variant and the code that handles it.

self.window.dom_manipulation_task_source().queue(handler, self.window.upcast()).unwrap();
}
}

Expand Down Expand Up @@ -3314,6 +3313,9 @@ impl DocumentProgressHandler {

fn dispatch_load(&self) {
let document = self.addr.root();
if document.browsing_context().is_none() {
return;
}
let window = document.window();
let event = Event::new(window.upcast(),
atom!("load"),
Expand All @@ -3331,7 +3333,6 @@ impl DocumentProgressHandler {
// http://w3c.github.io/navigation-timing/#widl-PerformanceNavigationTiming-loadEventEnd
update_with_current_time_ms(&document.load_event_end);


window.reflow(ReflowGoal::ForDisplay,
ReflowQueryType::NoQuery,
ReflowReason::DocumentLoaded);
Expand All @@ -3349,6 +3350,9 @@ impl Runnable for DocumentProgressHandler {
if window.is_alive() {
self.set_ready_state_complete();
self.dispatch_load();
if let Some(fragment) = document.url().fragment() {
document.check_and_scroll_fragment(fragment);
}
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions components/script/dom/servoparser/html.rs
Expand Up @@ -93,6 +93,10 @@ impl Tokenizer {
self.inner.end();
}

pub fn url(&self) -> &ServoUrl {
&self.inner.sink().sink().base_url
}

pub fn set_plaintext_state(&mut self) {
self.inner.set_plaintext_state();
}
Expand Down
18 changes: 13 additions & 5 deletions components/script/dom/servoparser/mod.rs
Expand Up @@ -110,7 +110,8 @@ impl ServoParser {
let url = context_document.url();

// Step 1.
let loader = DocumentLoader::new(&*context_document.loader());
let loader = DocumentLoader::new_with_threads(context_document.loader().resource_threads().clone(),
Some(url.clone()));
let document = Document::new(window, None, Some(url.clone()),
context_document.origin().alias(),
IsHTMLDocument::HTMLDocument,
Expand Down Expand Up @@ -351,14 +352,17 @@ impl ServoParser {
self.document.set_current_parser(None);

if self.pipeline.is_some() {
// Initial reflow.
self.document.disarm_reflow_timeout();
self.document.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage);
let window = self.document.window();
window.reflow(ReflowGoal::ForDisplay, ReflowQueryType::NoQuery, ReflowReason::FirstLoad);
}

// Step 3.
// Steps 3-12 are in other castles, namely process_deferred_scripts and finish_load.
let url = self.tokenizer.borrow().url().clone();
self.document.process_deferred_scripts();
self.document.finish_load(LoadType::PageSource(url));
}
}

Expand Down Expand Up @@ -401,6 +405,13 @@ impl Tokenizer {
}
}

fn url(&self) -> &ServoUrl {
match *self {
Tokenizer::Html(ref tokenizer) => tokenizer.url(),
Tokenizer::Xml(ref tokenizer) => tokenizer.url(),
}
}

fn set_plaintext_state(&mut self) {
match *self {
Tokenizer::Html(ref mut tokenizer) => tokenizer.set_plaintext_state(),
Expand Down Expand Up @@ -558,9 +569,6 @@ impl FetchResponseListener for ParserContext {
debug!("Failed to load page URL {}, error: {:?}", self.url, err);
}

parser.document
.finish_load(LoadType::PageSource(self.url.clone()));

parser.last_chunk_received.set(true);
if !parser.suspended.get() {
parser.parse_sync();
Expand Down
4 changes: 4 additions & 0 deletions components/script/dom/servoparser/xml.rs
Expand Up @@ -71,6 +71,10 @@ impl Tokenizer {
pub fn end(&mut self) {
self.inner.end()
}

pub fn url(&self) -> &ServoUrl {
&self.inner.sink().sink().base_url
}
}

#[allow(unsafe_code)]
Expand Down
31 changes: 2 additions & 29 deletions components/script/script_thread.rs
Expand Up @@ -41,7 +41,7 @@ use dom::bindings::str::DOMString;
use dom::bindings::trace::JSTraceable;
use dom::bindings::utils::WRAP_CALLBACKS;
use dom::browsingcontext::BrowsingContext;
use dom::document::{Document, DocumentProgressHandler, DocumentSource, FocusType, IsHTMLDocument, TouchEventResult};
use dom::document::{Document, DocumentSource, FocusType, IsHTMLDocument, TouchEventResult};
use dom::element::Element;
use dom::event::{Event, EventBubbles, EventCancelable};
use dom::globalscope::GlobalScope;
Expand Down Expand Up @@ -239,8 +239,6 @@ enum MixedMessage {
pub enum MainThreadScriptMsg {
/// Common variants associated with the script messages
Common(CommonScriptMsg),
/// Notify a document that all pending loads are complete.
DocumentLoadsComplete(PipelineId),
/// Notifies the script that a window associated with a particular pipeline
/// should be closed (only dispatched to ScriptThread).
ExitWindow(PipelineId),
Expand Down Expand Up @@ -1027,8 +1025,6 @@ impl ScriptThread {
self.handle_navigate(parent_pipeline_id, None, load_data, replace),
MainThreadScriptMsg::ExitWindow(id) =>
self.handle_exit_window_msg(id),
MainThreadScriptMsg::DocumentLoadsComplete(id) =>
self.handle_loads_complete(id),
MainThreadScriptMsg::Common(CommonScriptMsg::RunnableMsg(_, runnable)) => {
// The category of the runnable is ignored by the pattern, however
// it is still respected by profiling (see categorize_msg).
Expand Down Expand Up @@ -1248,29 +1244,6 @@ impl ScriptThread {
}
}

fn handle_loads_complete(&self, pipeline: PipelineId) {
let doc = match { self.documents.borrow().find_document(pipeline) } {
Some(doc) => doc,
None => return warn!("Message sent to closed pipeline {}.", pipeline),
};
if doc.loader().is_blocked() {
debug!("Script thread got loads complete while loader is blocked.");
return;
}

doc.mut_loader().inhibit_events();

// https://html.spec.whatwg.org/multipage/#the-end step 7
// Schedule a task to fire a "load" event (if no blocking loads have arrived in the mean time)
// NOTE: we can end up executing this code more than once, in case more blocking loads arrive.
let handler = box DocumentProgressHandler::new(Trusted::new(&doc));
self.dom_manipulation_task_source.queue(handler, doc.window().upcast()).unwrap();

if let Some(fragment) = doc.url().fragment() {
doc.check_and_scroll_fragment(fragment);
};
}

fn collect_reports(&self, reports_chan: ReportsChan) {
let mut path_seg = String::from("url(");
let mut dom_tree_size = 0;
Expand Down Expand Up @@ -1771,7 +1744,7 @@ impl ScriptThread {
});

let loader = DocumentLoader::new_with_threads(self.resource_threads.clone(),
Some(incomplete.url.clone()));
Some(final_url.clone()));

let is_html_document = match metadata.content_type {
Some(Serde(ContentType(Mime(TopLevel::Application, SubLevel::Ext(ref sub_level), _))))
Expand Down
@@ -0,0 +1,5 @@
[resource-selection-invoke-pause-networkState.html]
type: testharness
[NOT invoking resource selection with pause() when networkState is not NETWORK_EMPTY]
expected: FAIL

@@ -0,0 +1,5 @@
[resource-selection-invoke-remove-from-document-networkState.html]
type: testharness
[NOT invoking resource selection with implicit pause() when networkState is not NETWORK_EMPTY]
expected: FAIL

@@ -0,0 +1,5 @@
[resource-selection-invoke-set-src-not-in-document.html]
type: testharness
[invoking load by setting src on video not in a document]
expected: FAIL

@@ -0,0 +1,5 @@
[resource-selection-invoke-set-src.html]
type: testharness
[invoking load by setting src]
expected: FAIL

@@ -1,8 +1,6 @@
[subresource-integrity.sub.html]
type: testharness

expected: OK

[Style: Same-origin with correct sha256 and sha512 hash, rel='alternate stylesheet' enabled]
expected: FAIL

Expand Down
@@ -1,43 +1,43 @@
<!DOCTYPE html>
<html>
<head>
<title> adding several types of scripts through the DOM and removing some of them confuses scheduler (slow-loading scripts) </title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="testlib/testlib.js"></script>
<script type="text/javascript">
setup({explicit_done:true});
var head=document.getElementsByTagName('head')[0];
function createScript(url, contents) {
props = {};
if (url) {
props.src = url;
}
return testlib.addScript(contents, props, head, false);
}
var t = async_test(undefined, {timeout:10000})
<head>
<title> adding several types of scripts through the DOM and removing some of them confuses scheduler (slow-loading scripts) </title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="testlib/testlib.js"></script>
<script type="text/javascript">
setup({ explicit_done: true });
var head = document.getElementsByTagName('head')[0];
function createScript(url, contents) {
props = {};
if (url) {
props.src = url;
}
return testlib.addScript(contents, props, head, false);
}
var t = async_test(undefined, { timeout: 10000 })

function test() {
document.getElementById("log").textContent = "Please wait..."
var url = 'scripts/include-1.js?pipe=trickle(d1)';
var script = createScript(url);
var script2 = createScript('', 'log("Script #2 ran")');
head.removeChild(script2);
var url = 'scripts/include-2.js?pipe=trickle(d2)';
var script3 = createScript(url);
head.removeChild(script3);
function test() {
document.getElementById("log").textContent = "Please wait..."
var url = 'scripts/include-1.js?pipe=trickle(d1)';
var script = createScript(url);
var script2 = createScript('', 'log("Script #2 ran")');
head.removeChild(script2);
var url = 'scripts/include-2.js?pipe=trickle(d2)';
var script3 = createScript(url);
head.removeChild(script3);

setTimeout(t.step_func(function() {
done();
assert_array_equals(eventOrder, ['Script #2 ran', 'external script #1', 'external script #2']);
t.done();
}), 5500);
setTimeout(t.step_func(function () {
done();
assert_array_equals(eventOrder, ['Script #2 ran', 'external script #1', 'external script #2']);
t.done();
}), 5500);

};
onload = t.step_func(test)
</script>
</head>
<body>
<div id="log">FAILED (This TC requires JavaScript enabled)</div>
</body>
</html*>
};
onload = t.step_func(test)
</script>
</head>
<body>
<div id="log">FAILED (This TC requires JavaScript enabled)</div>
</body>
</html>
@@ -1,32 +1,31 @@
<!DOCTYPE html>
<html><head>
<title>scheduler: external defer script created with createContextualFragment</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="testlib/testlib.js"></script>
<title>scheduler: external defer script created with createContextualFragment</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="testlib/testlib.js"></script>
</head>
<div id="log"></div>
<script>
log('inline script #1');
var t = async_test();

t.step(function() {
var range = document.createRange();
var fragment = range.createContextualFragment("<script defer src='scripts/include-1.js?pipe=trickle(d1)'><\/script>");
document.body.appendChild(fragment.firstChild);
});

addEventListener("DOMContentLoaded", t.step_func(function() {
assert_array_equals(eventOrder, ['inline script #1', 'end inline script #1']);
t.done();
}));
<body>
<div id="log"></div>
<script>
log('inline script #1');
var t = async_test();

addEventListener("load", t.step_func(function() {
assert_array_equals(eventOrder, ['inline script #1', 'end inline script #1', 'external script #1']);
t.done();
}));
t.step(function () {
var range = document.createRange();
var fragment = range.createContextualFragment("<script defer src='scripts/include-1.js?pipe=trickle(d1)'><\/script>");
document.body.appendChild(fragment.firstChild);
});

log('end inline script #1');
</script>
addEventListener("DOMContentLoaded", t.step_func(function () {
assert_array_equals(eventOrder, ['inline script #1', 'end inline script #1']);
}));

addEventListener("load", t.step_func_done(function () {
assert_array_equals(eventOrder, ['inline script #1', 'end inline script #1', 'external script #1']);
}));

log('end inline script #1');
</script>
</body>
</html>