diff --git a/components/script/dom/htmliframeelement.rs b/components/script/dom/htmliframeelement.rs index a3a246c4edc6..f5a820458c21 100644 --- a/components/script/dom/htmliframeelement.rs +++ b/components/script/dom/htmliframeelement.rs @@ -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(); diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index a79669d6c2e1..8e139158427e 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -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::()) + .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(); } /// diff --git a/components/script/dom/windowproxy.rs b/components/script/dom/windowproxy.rs index 3ff326c683a6..7fea1728c121 100644 --- a/components/script/dom/windowproxy.rs +++ b/components/script/dom/windowproxy.rs @@ -96,6 +96,9 @@ pub struct WindowProxy { /// Has the browsing context been disowned? disowned: Cell, + /// https://html.spec.whatwg.org/multipage/#is-closing + is_closing: Cell, + /// The containing iframe element, if this is a same-origin iframe frame_element: Option>, @@ -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(); } diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index 440de991ceb6..b4363f5ada36 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -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); diff --git a/tests/wpt/metadata/html/browsers/the-window-object/close-method.window.js.ini b/tests/wpt/metadata/html/browsers/the-window-object/close-method.window.js.ini index 7af149760cd6..0e9dfd0ea04d 100644 --- a/tests/wpt/metadata/html/browsers/the-window-object/close-method.window.js.ini +++ b/tests/wpt/metadata/html/browsers/the-window-object/close-method.window.js.ini @@ -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 - diff --git a/tests/wpt/metadata/resource-timing/nested-context-navigations-iframe.html.ini b/tests/wpt/metadata/resource-timing/nested-context-navigations-iframe.html.ini index 87d3c09909f0..ec371a1bf726 100644 --- a/tests/wpt/metadata/resource-timing/nested-context-navigations-iframe.html.ini +++ b/tests/wpt/metadata/resource-timing/nested-context-navigations-iframe.html.ini @@ -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 diff --git a/tests/wpt/metadata/workers/modules/dedicated-worker-import-csp.html.ini b/tests/wpt/metadata/workers/modules/dedicated-worker-import-csp.html.ini index 48e41ffe0937..ad9025141b3a 100644 --- a/tests/wpt/metadata/workers/modules/dedicated-worker-import-csp.html.ini +++ b/tests/wpt/metadata/workers/modules/dedicated-worker-import-csp.html.ini @@ -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