Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-07-22 23:03:42 +01:00
Code Issues Projects Releases Packages Wiki Activity

improve spec compliance of discarding BCs

Browse source 
do not handle compositor input events when BC is being discarded

prevent firing of timers for discarded BCs

return null for opener is BC has been discarded

bundle discard BC steps into window method

return null in window.opener, if BC has already been discarded

move the window closed check pre-event to script-thread
This commit is contained in:
Gregory Terzian 2019-09-05 16:12:52 +08:00
parent 4fe8238b14
commit 45ec250b0a
7 changed files with 133 additions and 53 deletions
Download patch file Download diff file Expand all files Collapse all files

7
components/script/dom/htmliframeelement.rs
View file

@ -668,15 +668,14 @@ impl VirtualMethods for HTMLIFrameElement {
// so we need to discard the browsing contexts now, rather than
// when the `PipelineExit` message arrives.
for exited_pipeline_id in exited_pipeline_ids {
// https://html.spec.whatwg.org/multipage/#a-browsing-context-is-discarded
if let Some(exited_document) = ScriptThread::find_document(exited_pipeline_id) {
debug!(
"Discarding browsing context for pipeline {}",
exited_pipeline_id
);
exited_document
.window()
.window_proxy()
.discard_browsing_context();
let exited_window = exited_document.window();
exited_window.discard_browsing_context();
for exited_iframe in exited_document.iter_iframes() {
debug!("Discarding nested browsing context");
exited_iframe.destroy_nested_browsing_context();

97
components/script/dom/window.rs
View file

@ -63,7 +63,7 @@ use crate::script_runtime::{
use crate::script_thread::{ImageCacheMsg, MainThreadScriptChan, MainThreadScriptMsg};
use crate::script_thread::{ScriptThread, SendableMainThreadScriptChan};
use crate::task_manager::TaskManager;
use crate::task_source::TaskSourceName;
use crate::task_source::{TaskSource, TaskSourceName};
use crate::timers::{IsInterval, TimerCallback};
use crate::webdriver_handlers::jsval_to_webdriver;
use app_units::Au;
@ -82,8 +82,8 @@ use ipc_channel::router::ROUTER;
use js::jsapi::JSAutoRealm;
use js::jsapi::JSPROP_ENUMERATE;
use js::jsapi::{GCReason, JS_GC};
use js::jsval::JSVal;
use js::jsval::UndefinedValue;
use js::jsval::{JSVal, NullValue};
use js::rust::wrappers::JS_DefineProperty;
use js::rust::HandleValue;
use media::WindowGLContext;
@ -352,11 +352,26 @@ impl Window {
*self.js_runtime.borrow_for_script_deallocation() = None;
self.window_proxy.set(None);
self.current_state.set(WindowState::Zombie);
self.ignore_all_events();
self.ignore_all_tasks();
}
}
fn ignore_all_events(&self) {
/// A convenience method for
/// https://html.spec.whatwg.org/multipage/#a-browsing-context-is-discarded
pub fn discard_browsing_context(&self) {
let proxy = match self.window_proxy.get() {
Some(proxy) => proxy,
None => panic!("Discarding a BC from a window that has none"),
};
proxy.discard_browsing_context();
// Step 4 of https://html.spec.whatwg.org/multipage/#discard-a-document
// Other steps performed when the `PipelineExit` message
// is handled by the ScriptThread.
self.ignore_all_tasks();
}
/// Cancel all current, and ignore all subsequently queued, tasks.
pub fn ignore_all_tasks(&self) {
let mut ignore_flags = self.task_manager.task_cancellers.borrow_mut();
for task_source_name in TaskSourceName::all() {
let flag = ignore_flags
@ -623,10 +638,23 @@ impl WindowMethods for Window {
self.window_proxy().open(url, target, features)
}
#[allow(unsafe_code)]
// https://html.spec.whatwg.org/multipage/#dom-opener
fn Opener(&self, cx: JSContext) -> JSVal {
unsafe { self.window_proxy().opener(*cx) }
// Step 1, Let current be this Window object's browsing context.
let current = match self.window_proxy.get() {
Some(proxy) => proxy,
// Step 2, If current is null, then return null.
None => return NullValue(),
};
// Still step 2, since the window's BC is the associated doc's BC,
// see https://html.spec.whatwg.org/multipage/#window-bc
// and a doc's BC is null if it has been discarded.
// see https://html.spec.whatwg.org/multipage/#concept-document-bc
if current.is_browsing_context_discarded() {
return NullValue();
}
// Step 3 to 5.
current.opener(*cx)
}
#[allow(unsafe_code)]
@ -653,31 +681,60 @@ impl WindowMethods for Window {
fn Closed(&self) -> bool {
self.window_proxy
.get()
.map(|ref proxy| proxy.is_browsing_context_discarded())
.map(|ref proxy| proxy.is_browsing_context_discarded() || proxy.is_closing())
.unwrap_or(true)
}
// https://html.spec.whatwg.org/multipage/#dom-window-close
fn Close(&self) {
let window_proxy = self.window_proxy();
// Step 1, Let current be this Window object's browsing context.
// Step 2, If current is null or its is closing is true, then return.
let window_proxy = match self.window_proxy.get() {
Some(proxy) => proxy,
None => return,
};
if window_proxy.is_closing() {
return;
}
// Note: check the length of the "session history", as opposed to the joint session history?
// see https://github.com/whatwg/html/issues/3734
if let Ok(history_length) = self.History().GetLength() {
let is_auxiliary = window_proxy.is_auxiliary();
// https://html.spec.whatwg.org/multipage/#script-closable
let is_script_closable = (self.is_top_level() && history_length == 1) || is_auxiliary;
// TODO: rest of Step 3:
// Is the incumbent settings object's responsible browsing context familiar with current?
// Is the incumbent settings object's responsible browsing context allowed to navigate current?
if is_script_closable {
let doc = self.Document();
// https://html.spec.whatwg.org/multipage/#closing-browsing-contexts
// Step 1, prompt to unload.
if doc.prompt_to_unload(false) {
// Step 2, unload.
doc.unload(false);
// Step 3, remove from the user interface
let _ = self.send_to_embedder(EmbedderMsg::CloseBrowser);
// Step 4, discard browsing context.
let _ = self.send_to_constellation(ScriptMsg::DiscardTopLevelBrowsingContext);
}
// Step 3.1, set current's is closing to true.
window_proxy.close();
// Step 3.2, queue a task on the DOM manipulation task source to close current.
let this = Trusted::new(self);
let task = task!(window_close_browsing_context: move || {
let window = this.root();
let document = window.Document();
// https://html.spec.whatwg.org/multipage/#closing-browsing-contexts
// Step 1, prompt to unload.
if document.prompt_to_unload(false) {
// Step 2, unload.
document.unload(false);
// Step 3, remove from the user interface
let _ = window.send_to_embedder(EmbedderMsg::CloseBrowser);
// Step 4, discard browsing context.
// https://html.spec.whatwg.org/multipage/#a-browsing-context-is-discarded
// which calls into https://html.spec.whatwg.org/multipage/#discard-a-document.
window.discard_browsing_context();
let _ = window.send_to_constellation(ScriptMsg::DiscardTopLevelBrowsingContext);
}
});
self.task_manager()
.dom_manipulation_task_source()
.queue(task, &self.upcast::<GlobalScope>())
.expect("Queuing window_close_browsing_context task to work");
}
}
}
@ -1302,7 +1359,7 @@ impl Window {
if let Some(performance) = self.performance.get() {
performance.clear_and_disable_performance_entry_buffer();
}
self.ignore_all_events();
self.ignore_all_tasks();
}
/// <https://drafts.csswg.org/cssom-view/#dom-window-scroll>

21
components/script/dom/windowproxy.rs
View file

@ -96,6 +96,9 @@ pub struct WindowProxy {
/// Has the browsing context been disowned?
disowned: Cell<bool>,
/// https://html.spec.whatwg.org/multipage/#is-closing
is_closing: Cell<bool>,
/// The containing iframe element, if this is a same-origin iframe
frame_element: Option<Dom<Element>>,
@ -126,6 +129,7 @@ impl WindowProxy {
currently_active: Cell::new(currently_active),
discarded: Cell::new(false),
disowned: Cell::new(false),
is_closing: Cell::new(false),
frame_element: frame_element.map(Dom::from_ref),
parent: parent.map(Dom::from_ref),
delaying_load_events_mode: Cell::new(false),
@ -352,9 +356,20 @@ impl WindowProxy {
self.disowned.set(true);
}
/// https://html.spec.whatwg.org/multipage/#dom-window-close
/// Step 3.1, set BCs `is_closing` to true.
pub fn close(&self) {
self.is_closing.set(true);
}
/// https://html.spec.whatwg.org/multipage/#is-closing
pub fn is_closing(&self) -> bool {
self.is_closing.get()
}
#[allow(unsafe_code)]
// https://html.spec.whatwg.org/multipage/#dom-opener
pub unsafe fn opener(&self, cx: *mut JSContext) -> JSVal {
pub fn opener(&self, cx: *mut JSContext) -> JSVal {
if self.disowned.get() {
return NullValue();
}
@ -371,7 +386,7 @@ impl WindowProxy {
opener_id,
) {
Some(opener_top_id) => {
let global_to_clone_from = GlobalScope::from_context(cx);
let global_to_clone_from = unsafe { GlobalScope::from_context(cx) };
WindowProxy::new_dissimilar_origin(
&*global_to_clone_from,
opener_id,
@ -388,7 +403,7 @@ impl WindowProxy {
return NullValue();
}
rooted!(in(cx) let mut val = UndefinedValue());
opener_proxy.to_jsval(cx, val.handle_mut());
unsafe { opener_proxy.to_jsval(cx, val.handle_mut()) };
return val.get();
}

31
components/script/script_thread.rs
View file

@ -1973,7 +1973,15 @@ impl ScriptThread {
let window = self.documents.borrow().find_window(pipeline_id);
let window = match window {
Some(w) => w,
Some(w) => {
if w.Closed() {
return warn!(
"Received fire timer msg for a discarded browsing context whose pipeline is pending exit {}.",
pipeline_id
);
}
w
},
None => {
return warn!(
"Received fire timer msg for a closed pipeline {}.",
@ -2848,7 +2856,7 @@ impl ScriptThread {
// to avoid running layout on detached iframes.
let window = document.window();
if discard_bc == DiscardBrowsingContext::Yes {
window.window_proxy().discard_browsing_context();
window.discard_browsing_context();
}
window.clear_js_runtime();
}
@ -3378,9 +3386,26 @@ impl ScriptThread {
///
/// TODO: Actually perform DOM event dispatch.
fn handle_event(&self, pipeline_id: PipelineId, event: CompositorEvent) {
// Do not handle events if the pipeline exited.
let window = match { self.documents.borrow().find_window(pipeline_id) } {
Some(win) => win,
None => {
return warn!(
"Compositor event sent to a pipeline that already exited {}.",
pipeline_id
)
},
};
// Do not handle events if the BC has been, or is being, discarded
if window.Closed() {
return warn!(
"Compositor event sent to a pipeline with a closed window {}.",
pipeline_id
);
}
// Assuming all CompositionEvent are generated by user interactions.
ScriptThread::set_user_interacting(true);
match event {
ResizeEvent(new_size, size_type) => {
self.handle_resize_event(pipeline_id, new_size, size_type);

4
tests/wpt/metadata/html/browsers/the-window-object/close-method.window.js.ini
View file

@ -1,7 +1,3 @@
[close-method.window.html]
[window.close() affects name targeting immediately]
expected: FAIL
[window.close() queues a task to discard, but window.closed knows immediately]
expected: FAIL

12
tests/wpt/metadata/resource-timing/nested-context-navigations-iframe.html.ini
View file

@ -1,20 +1,20 @@
[nested-context-navigations-iframe.html]
expected: CRASH
expected: TIMEOUT
[Test that iframe navigations are not observable by the parent, even after history navigations by the parent]
expected: TIMEOUT
expected: FAIL
[Test that crossorigin iframe navigations are not observable by the parent, even after history navigations by the parent]
expected: NOTRUN
expected: FAIL
[Test that iframe refreshes are not observable by the parent]
expected: NOTRUN
expected: TIMEOUT
[Test that crossorigin iframe navigations are not observable by the parent]
expected: NOTRUN
expected: FAIL
[Test that crossorigin iframe refreshes are not observable by the parent]
expected: NOTRUN
[Test that iframe navigations are not observable by the parent]
expected: NOTRUN
expected: FAIL

14
tests/wpt/metadata/workers/modules/dedicated-worker-import-csp.html.ini
View file

@ -1,29 +1,17 @@
[dedicated-worker-import-csp.html]
expected: CRASH
expected: OK
[DedicatedWorker: CSP for ES Modules]
expected: FAIL
[worker-src 'self' directive should disallow cross origin static import.]
expected: FAIL
[worker-src * directive should allow cross origin static import.]
expected: FAIL
[script-src 'self' directive should disallow cross origin static import.]
expected: FAIL
[script-src * directive should allow cross origin static import.]
expected: FAIL
[worker-src * directive should override script-src 'self' directive and allow cross origin static import.]
expected: FAIL
[worker-src 'self' directive should override script-src * directive and disallow cross origin static import.]
expected: FAIL
[script-src 'self' directive should disallow cross origin dynamic import.]
expected: FAIL
[script-src * directive should allow cross origin dynamic import.]
expected: FAIL