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

Initialize iframe viewport immediately #22395

Merged
merged 5 commits into from Dec 18, 2018

Delay iframe and script element processing until the DOM is stable.

  • Loading branch information
jdm committed Dec 14, 2018
commit fc2d810bce22d4432fd22f30ac4ee06944671612
@@ -49,6 +49,7 @@ use crate::dom::bindings::utils::WindowProxyHandler;
use crate::dom::document::PendingRestyle;
use crate::dom::htmlimageelement::SourceSet;
use crate::dom::htmlmediaelement::MediaFrameRenderer;
use crate::task::TaskBox;
use crossbeam_channel::{Receiver, Sender};
use cssparser::RGBA;
use devtools_traits::{CSSError, TimelineMarkerType, WorkerId};
@@ -139,6 +140,8 @@ pub unsafe trait JSTraceable {
unsafe fn trace(&self, trc: *mut JSTracer);
}

unsafe_no_jsmanaged_fields!(Box<dyn TaskBox>);

unsafe_no_jsmanaged_fields!(CSSError);

unsafe_no_jsmanaged_fields!(&'static Encoding);
@@ -101,6 +101,7 @@ use crate::dom::windowproxy::WindowProxy;
use crate::fetch::FetchCanceller;
use crate::script_runtime::{CommonScriptMsg, ScriptThreadEventCategory};
use crate::script_thread::{MainThreadScriptMsg, ScriptThread};
use crate::task::TaskBox;
use crate::task_source::{TaskSource, TaskSourceName};
use crate::timers::OneshotTimerCallback;
use devtools_traits::ScriptToDevtoolsControlMsg;
@@ -410,8 +411,11 @@ pub struct Document {
responsive_images: DomRefCell<Vec<Dom<HTMLImageElement>>>,
/// Number of redirects for the document load
redirect_count: Cell<u16>,
///
/// Number of outstanding requests to prevent JS or layout from running.
script_and_layout_blockers: Cell<u32>,
/// List of tasks to execute as soon as last script/layout blocker is removed.
#[ignore_malloc_size_of = "Measuring trait objects is hard"]
delayed_tasks: DomRefCell<Vec<Box<dyn TaskBox>>>,
}

#[derive(JSTraceable, MallocSizeOf)]
@@ -2698,24 +2702,48 @@ impl Document {
responsive_images: Default::default(),
redirect_count: Cell::new(0),
script_and_layout_blockers: Cell::new(0),
delayed_tasks: Default::default(),
}
}

/// Prevent any JS or layout from running until the corresponding call to
/// `remove_script_and_layout_blocker`. Used to isolate periods in which
/// the DOM is in an unstable state and should not be exposed to arbitrary
/// web content. Any attempts to invoke content JS or query layout during
/// that time will trigger a panic. `add_delayed_task` will cause the
/// provided task to be executed as soon as the last blocker is removed.
pub fn add_script_and_layout_blocker(&self) {
self.script_and_layout_blockers.set(
self.script_and_layout_blockers.get() + 1
);
self.script_and_layout_blockers
.set(self.script_and_layout_blockers.get() + 1);
}

/// Terminate the period in which JS or layout is disallowed from running.
/// If no further blockers remain, any delayed tasks in the queue will
/// be executed in queue order until the queue is empty.
pub fn remove_script_and_layout_blocker(&self) {
assert!(self.script_and_layout_blockers.get() > 0);
self.script_and_layout_blockers.set(
self.script_and_layout_blockers.get() - 1
);
self.script_and_layout_blockers
.set(self.script_and_layout_blockers.get() - 1);
while self.script_and_layout_blockers.get() == 0 && !self.delayed_tasks.borrow().is_empty()
{
let task = self.delayed_tasks.borrow_mut().remove(0);
task.run_box();
}
}

/// Enqueue a task to run as soon as any JS and layout blockers are removed.
pub fn add_delayed_task<T: 'static + TaskBox>(&self, task: T) {
self.delayed_tasks.borrow_mut().push(Box::new(task));
}

/// Assert that the DOM is in a state that will allow running content JS or
/// performing a layout operation.
pub fn ensure_safe_to_run_script_or_layout(&self) {
assert_eq!(self.script_and_layout_blockers.get(), 0);
assert_eq!(
self.script_and_layout_blockers.get(),
0,
"Attempt to use script or layout while DOM not in a stable state"
);
}

// https://dom.spec.whatwg.org/#dom-document-document
@@ -623,18 +623,22 @@ impl VirtualMethods for HTMLIFrameElement {
s.bind_to_tree(tree_in_doc);
}

// https://html.spec.whatwg.org/multipage/#the-iframe-element
// "When an iframe element is inserted into a document that has
// a browsing context, the user agent must create a new
// browsing context, set the element's nested browsing context
// to the newly-created browsing context, and then process the
// iframe attributes for the "first time"."
if self.upcast::<Node>().is_in_doc_with_browsing_context() {
debug!("iframe bound to browsing context.");
debug_assert!(tree_in_doc, "is_in_doc_with_bc, but not tree_in_doc");
self.create_nested_browsing_context();
self.process_the_iframe_attributes(ProcessingMode::FirstTime);
}
let iframe = Trusted::new(self);
document_from_node(self).add_delayed_task(task!(IFrameDelayedInitialize: move || {
let this = iframe.root();
// https://html.spec.whatwg.org/multipage/#the-iframe-element
// "When an iframe element is inserted into a document that has
// a browsing context, the user agent must create a new
// browsing context, set the element's nested browsing context
// to the newly-created browsing context, and then process the
// iframe attributes for the "first time"."
if this.upcast::<Node>().is_in_doc_with_browsing_context() {
debug!("iframe bound to browsing context.");
debug_assert!(tree_in_doc, "is_in_doc_with_bc, but not tree_in_doc");
this.create_nested_browsing_context();
this.process_the_iframe_attributes(ProcessingMode::FirstTime);
}
}));
}

fn unbind_from_tree(&self, context: &UnbindContext) {
@@ -777,7 +777,10 @@ impl VirtualMethods for HTMLScriptElement {
}

if tree_in_doc && !self.parser_inserted.get() {
self.prepare();
let script = Trusted::new(self);
document_from_node(self).add_delayed_task(task!(ScriptDelayedInitialize: move || {

This comment has been minimized.

Copy link
@nox

nox Jan 31, 2019

Member

Where is it specified that scripts shouldn't be immediately prepared?

This comment has been minimized.

This comment has been minimized.

Copy link
@jdm

jdm Jan 31, 2019

Author Member

It still is immeditate, but it waits until the current DOM modification is complete, so no intermediate state can be observed by script or layout.

script.root().prepare();
}));
}
}

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.