Auto merge of #25515 - pshaughn:checkpoints, r=jdm

Add microtask checkpoints to script elements and custom elements

Servo had a microtask checkpoint at the end of running a script, but there was also supposed to be one at the end of HTML-parsing a script element before Javascript-parsing the script itself, and there were supposed to be checkpoints immediately after the call to a custom element constructor. This adds those, passing all cases of one WPT test file.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #25016 except for the remaining not-really-about-microtasks case #25514

<!-- Either: -->
- [X] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
This commit is contained in:
bors-servo 2020-01-25 07:30:48 -05:00 committed by GitHub
commit 53879945a9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 37 additions and 31 deletions

View file

@ -20,6 +20,7 @@ use crate::dom::bindings::error::{
use crate::dom::bindings::inheritance::Castable; use crate::dom::bindings::inheritance::Castable;
use crate::dom::bindings::reflector::{reflect_dom_object, DomObject, Reflector}; use crate::dom::bindings::reflector::{reflect_dom_object, DomObject, Reflector};
use crate::dom::bindings::root::{Dom, DomRoot}; use crate::dom::bindings::root::{Dom, DomRoot};
use crate::dom::bindings::settings_stack::is_execution_stack_empty;
use crate::dom::bindings::str::DOMString; use crate::dom::bindings::str::DOMString;
use crate::dom::document::Document; use crate::dom::document::Document;
use crate::dom::domexception::{DOMErrorName, DOMException}; use crate::dom::domexception::{DOMErrorName, DOMException};
@ -543,6 +544,14 @@ impl CustomElementDefinition {
} }
} }
// https://heycam.github.io/webidl/#construct-a-callback-function
// https://html.spec.whatwg.org/multipage/#clean-up-after-running-script
if is_execution_stack_empty() {
window
.upcast::<GlobalScope>()
.perform_a_microtask_checkpoint();
}
rooted!(in(*cx) let element_val = ObjectValue(element.get())); rooted!(in(*cx) let element_val = ObjectValue(element.get()));
let element: DomRoot<Element> = let element: DomRoot<Element> =
match unsafe { DomRoot::from_jsval(*cx, element_val.handle(), ()) } { match unsafe { DomRoot::from_jsval(*cx, element_val.handle(), ()) } {
@ -690,6 +699,15 @@ fn run_upgrade_constructor(
} { } {
return Err(Error::JSFailed); return Err(Error::JSFailed);
} }
// https://heycam.github.io/webidl/#construct-a-callback-function
// https://html.spec.whatwg.org/multipage/#clean-up-after-running-script
if is_execution_stack_empty() {
window
.upcast::<GlobalScope>()
.perform_a_microtask_checkpoint();
}
// Step 8.3 // Step 8.3
let mut same = false; let mut same = false;
rooted!(in(*cx) let construct_result_val = ObjectValue(construct_result.get())); rooted!(in(*cx) let construct_result_val = ObjectValue(construct_result.get()));

View file

@ -826,6 +826,8 @@ impl HTMLImageElement {
// Step 8 // Step 8
Some(data) => data, Some(data) => data,
None => { None => {
self.abort_request(State::Broken, ImageRequestPhase::Current);
self.abort_request(State::Broken, ImageRequestPhase::Pending);
// Step 9. // Step 9.
// FIXME(nox): Why are errors silenced here? // FIXME(nox): Why are errors silenced here?
let _ = task_source.queue( let _ = task_source.queue(
@ -843,11 +845,6 @@ impl HTMLImageElement {
if src_present || Self::uses_srcset_or_picture(elem) { if src_present || Self::uses_srcset_or_picture(elem) {
this.upcast::<EventTarget>().fire_event(atom!("error")); this.upcast::<EventTarget>().fire_event(atom!("error"));
} }
// FIXME(nox): According to the spec, setting the current
// request to the broken state is done prior to queuing a
// task, why is this here?
this.abort_request(State::Broken, ImageRequestPhase::Current);
this.abort_request(State::Broken, ImageRequestPhase::Pending);
}), }),
window.upcast(), window.upcast(),
); );
@ -864,6 +861,8 @@ impl HTMLImageElement {
self.prepare_image_request(&url, &src, pixel_density); self.prepare_image_request(&url, &src, pixel_density);
}, },
Err(_) => { Err(_) => {
self.abort_request(State::Broken, ImageRequestPhase::Current);
self.abort_request(State::Broken, ImageRequestPhase::Pending);
// Step 12.1-12.5. // Step 12.1-12.5.
let src = src.0; let src = src.0;
// FIXME(nox): Why are errors silenced here? // FIXME(nox): Why are errors silenced here?
@ -877,11 +876,6 @@ impl HTMLImageElement {
} }
this.upcast::<EventTarget>().fire_event(atom!("error")); this.upcast::<EventTarget>().fire_event(atom!("error"));
// FIXME(nox): According to the spec, setting the current
// request to the broken state is done prior to queuing a
// task, why is this here?
this.abort_request(State::Broken, ImageRequestPhase::Current);
this.abort_request(State::Broken, ImageRequestPhase::Pending);
}), }),
window.upcast(), window.upcast(),
); );

View file

@ -558,6 +558,19 @@ impl ServoParser {
Err(script) => script, Err(script) => script,
}; };
// https://html.spec.whatwg.org/multipage/#parsing-main-incdata
// branch "An end tag whose tag name is "script"
// The spec says to perform the microtask checkpoint before
// setting the insertion mode back from Text, but this is not
// possible with the way servo and html5ever currently
// relate to each other, and hopefully it is not observable.
if is_execution_stack_empty() {
self.document
.window()
.upcast::<GlobalScope>()
.perform_a_microtask_checkpoint();
}
let script_nesting_level = self.script_nesting_level.get(); let script_nesting_level = self.script_nesting_level.get();
self.script_nesting_level.set(script_nesting_level + 1); self.script_nesting_level.set(script_nesting_level + 1);

View file

@ -90,6 +90,8 @@ impl MicrotaskQueue {
// Step 1 // Step 1
self.performing_a_microtask_checkpoint.set(true); self.performing_a_microtask_checkpoint.set(true);
debug!("Now performing a microtask checkpoint");
// Steps 2 // Steps 2
while !self.microtask_queue.borrow().is_empty() { while !self.microtask_queue.borrow().is_empty() {
rooted_vec!(let mut pending_queue); rooted_vec!(let mut pending_queue);

View file

@ -1,10 +0,0 @@
[microtasks-and-constructors.html]
[Microtasks evaluate immediately when the stack is empty inside the parser]
expected: FAIL
[Microtasks evaluate immediately when the stack is empty inside the parser, causing the checks on no attributes to fail]
expected: FAIL
[Microtasks evaluate afterward when the stack is not empty using createElement()]
expected: FAIL

View file

@ -1,11 +0,0 @@
[MutationObserver-document.html]
type: testharness
[parser insertion mutations]
expected: FAIL
[parser script insertion mutation]
expected: FAIL
[removal of parent during parsing]
expected: FAIL