From 4c7f198ee22ba5c6c1393395ceb9889d2d4decaa Mon Sep 17 00:00:00 2001 From: Delan Azabani Date: Thu, 23 Mar 2023 18:02:35 +0800 Subject: [PATCH] apply yvt/servo/fix-named-window-getter --- components/domobject_derive/lib.rs | 46 +++- components/script/dom/bindings/reflector.rs | 56 ++++- components/script/dom/bindings/root.rs | 218 ++++++++++++++++-- components/script/dom/bindings/trace.rs | 4 +- .../script/dom/webgl_extensions/extension.rs | 4 +- components/script/dom/window.rs | 13 +- components/script/dom/windowproxy.rs | 56 ++++- components/script/script_thread.rs | 32 ++- components/script/window_named_properties.rs | 18 +- 9 files changed, 398 insertions(+), 49 deletions(-) diff --git a/components/domobject_derive/lib.rs b/components/domobject_derive/lib.rs index c103bb73ece..710e932ae99 100644 --- a/components/domobject_derive/lib.rs +++ b/components/domobject_derive/lib.rs @@ -12,7 +12,7 @@ extern crate syn; use proc_macro2; use quote::TokenStreamExt; -#[proc_macro_derive(DomObject)] +#[proc_macro_derive(DomObject, attributes(dom_object))] pub fn expand_token_stream(input: proc_macro::TokenStream) -> proc_macro::TokenStream { let input = syn::parse(input).unwrap(); expand_dom_object(input).into() @@ -29,6 +29,16 @@ fn expand_dom_object(input: syn::DeriveInput) -> proc_macro2::TokenStream { .split_first() .expect("#[derive(DomObject)] should not be applied on empty structs"); + // Take additional parameters from `#[dom_object]` helper attributes (if any) + let mut args = Args::default(); + for attr in input.attrs.iter() { + if attr.path.is_ident("dom_object") { + if let Err(e) = attr.parse_meta().and_then(|meta| args.parse_meta(&meta)) { + return e.to_compile_error().into(); + } + } + } + let first_field_name = first_field.ident.as_ref().unwrap(); let mut field_types = vec![]; for field in fields { @@ -64,6 +74,14 @@ fn expand_dom_object(input: syn::DeriveInput) -> proc_macro2::TokenStream { } }; + if !args.transplantable { + items.append_all(quote! { + impl #impl_generics crate::dom::bindings::reflector::Untransplantable + for #name #ty_generics #where_clause + {} + }); + } + let mut params = proc_macro2::TokenStream::new(); params.append_separated(input.generics.type_params().map(|param| ¶m.ident), ", "); @@ -100,3 +118,29 @@ fn expand_dom_object(input: syn::DeriveInput) -> proc_macro2::TokenStream { tokens } + +#[derive(Default)] +struct Args { + transplantable: bool, +} + +impl Args { + fn parse_meta(&mut self, meta: &syn::Meta) -> Result<(), syn::Error> { + match meta { + syn::Meta::List(list) => self.parse_meta_list(list), + _ => return Err(syn::Error::new_spanned(meta, "unrecognized parameter")), + } + } + + fn parse_meta_list(&mut self, meta_list: &syn::MetaList) -> Result<(), syn::Error> { + for meta in meta_list.nested.iter() { + match meta { + syn::NestedMeta::Meta(syn::Meta::Path(path)) if path.is_ident("transplantable") => { + self.transplantable = true; + }, + _ => return Err(syn::Error::new_spanned(meta, "unrecognized parameter")), + } + } + Ok(()) + } +} diff --git a/components/script/dom/bindings/reflector.rs b/components/script/dom/bindings/reflector.rs index 04bf9fdbbed..93405380951 100644 --- a/components/script/dom/bindings/reflector.rs +++ b/components/script/dom/bindings/reflector.rs @@ -12,7 +12,7 @@ use crate::dom::globalscope::GlobalScope; use crate::script_runtime::JSContext; use js::jsapi::{Heap, JSObject}; use js::rust::HandleObject; -use std::default::Default; +use std::{default::Default, ops::Deref}; /// Create the reflector for a new DOM object and yield ownership to the /// reflector. @@ -120,3 +120,57 @@ pub trait DomObjectIteratorWrap: DomObjectWrap + JSTraceable + Iterable { Box>, ) -> Root>>; } + +/// A marker trait denoting DOM objects that are constrained to a single +/// realm. It's implemented by most DOM objects with a notable exception being +/// `WindowProxy`. +/// +/// The reflectors of transplantable types may move between realms, and the +/// compartment invariants [prohibit][1] tracable references from crossing +/// compartment boundaries. For this reason, references to transplantable types +/// must be held by `*TransplantableDom*`, which are designed to handle +/// cross-realm cases. Even when using such reference wrappers, a care must be +/// taken to ensure cross-realm references do not occur. For instance, +/// a transplantable DOM object must hold references to other DOM objects by +/// `*TransplantableDom*` because their realms can "move" in relative to +/// the transplantable DOM object's. +/// +/// [1]: https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/Internals/Garbage_collection#compartments +pub trait Untransplantable: DomObject {} + +/// Unsafely adds [`Untransplantable`] to the wrapped [`DomObject`]. +#[derive(JSTraceable)] +#[repr(transparent)] +pub struct AssertUntransplantable(T); + +impl AssertUntransplantable { + /// Wrap a reference with `AssertUntransplantable`. + /// + /// # Safety + /// + /// The constructed `&Self` must not be traced from a GC thing with an + /// associated compartment. For example, if you create `Dom` from the + /// returned reference, storing it in a `DomObject` might be unsafe. + #[inline] + pub unsafe fn from_ref(x: &T) -> &Self { + // Safety: `*x` and `Self` has the same representation + &*(x as *const T as *const Self) + } +} + +impl DomObject for AssertUntransplantable { + #[inline] + fn reflector(&self) -> &Reflector { + self.0.reflector() + } +} + +impl Untransplantable for AssertUntransplantable {} + +impl Deref for AssertUntransplantable { + type Target = T; + #[inline] + fn deref(&self) -> &Self::Target { + &self.0 + } +} diff --git a/components/script/dom/bindings/root.rs b/components/script/dom/bindings/root.rs index b0b7eeb146d..f6add2f3ae1 100644 --- a/components/script/dom/bindings/root.rs +++ b/components/script/dom/bindings/root.rs @@ -26,7 +26,7 @@ use crate::dom::bindings::conversions::DerivedFrom; use crate::dom::bindings::inheritance::Castable; -use crate::dom::bindings::reflector::{DomObject, MutDomObject, Reflector}; +use crate::dom::bindings::reflector::{DomObject, MutDomObject, Reflector, Untransplantable}; use crate::dom::bindings::trace::trace_reflector; use crate::dom::bindings::trace::JSTraceable; use crate::dom::node::Node; @@ -362,7 +362,7 @@ impl Deref for Dom { } } -unsafe impl JSTraceable for Dom { +unsafe impl JSTraceable for Dom { unsafe fn trace(&self, trc: *mut JSTracer) { let trace_string; let trace_info = if cfg!(debug_assertions) { @@ -542,11 +542,11 @@ impl LayoutDom<'_, Node> { /// on `Dom`. #[unrooted_must_root_lint::must_root] #[derive(JSTraceable)] -pub struct MutDom { +pub struct MutDom { val: UnsafeCell>, } -impl MutDom { +impl MutDom { /// Create a new `MutDom`. pub fn new(initial: &T) -> MutDom { assert_in_script(); @@ -570,20 +570,20 @@ impl MutDom { } } -impl MallocSizeOf for MutDom { +impl MallocSizeOf for MutDom { fn size_of(&self, _ops: &mut MallocSizeOfOps) -> usize { // See comment on MallocSizeOf for Dom. 0 } } -impl PartialEq for MutDom { +impl PartialEq for MutDom { fn eq(&self, other: &Self) -> bool { unsafe { *self.val.get() == *other.val.get() } } } -impl PartialEq for MutDom { +impl PartialEq for MutDom { fn eq(&self, other: &T) -> bool { unsafe { **self.val.get() == *other } } @@ -605,11 +605,11 @@ pub(crate) fn assert_in_layout() { /// on `Dom`. #[unrooted_must_root_lint::must_root] #[derive(JSTraceable)] -pub struct MutNullableDom { +pub struct MutNullableDom { ptr: UnsafeCell>>, } -impl MutNullableDom { +impl MutNullableDom { /// Create a new `MutNullableDom`. pub fn new(initial: Option<&T>) -> MutNullableDom { assert_in_script(); @@ -666,19 +666,19 @@ impl MutNullableDom { } } -impl PartialEq for MutNullableDom { +impl PartialEq for MutNullableDom { fn eq(&self, other: &Self) -> bool { unsafe { *self.ptr.get() == *other.ptr.get() } } } -impl<'a, T: DomObject> PartialEq> for MutNullableDom { +impl<'a, T: DomObject + Untransplantable> PartialEq> for MutNullableDom { fn eq(&self, other: &Option<&T>) -> bool { unsafe { *self.ptr.get() == other.map(Dom::from_ref) } } } -impl Default for MutNullableDom { +impl Default for MutNullableDom { #[allow(unrooted_must_root)] fn default() -> MutNullableDom { assert_in_script(); @@ -688,7 +688,7 @@ impl Default for MutNullableDom { } } -impl MallocSizeOf for MutNullableDom { +impl MallocSizeOf for MutNullableDom { fn size_of(&self, _ops: &mut MallocSizeOfOps) -> usize { // See comment on MallocSizeOf for Dom. 0 @@ -740,7 +740,7 @@ impl MallocSizeOf for DomOnceCell { } #[allow(unrooted_must_root)] -unsafe impl JSTraceable for DomOnceCell { +unsafe impl JSTraceable for DomOnceCell { unsafe fn trace(&self, trc: *mut JSTracer) { if let Some(ptr) = self.ptr.as_ref() { ptr.trace(trc); @@ -748,6 +748,196 @@ unsafe impl JSTraceable for DomOnceCell { } } +/// Essentially a [`MutNullableDom`], but directly references the reflector +/// so that, with a proper care, a cross-realm reference is prevented from +/// being formed by transplantation. +/// +/// This type can hold a reference to an un-[`Untransplantable`] DOM object. +/// In turn, such objects also need this sort of type to hold references to +/// other DOM objects whether they are transplantable or not (so the name is +/// inaccurate, actually). +/// +/// This should only be used as a field in other DOM objects; see warning +/// on `Dom`. +#[unrooted_must_root_lint::must_root] +pub struct MutNullableTransplantableDom { + /// A reference to the DOM object. + ptr: UnsafeCell>>, + /// A tracable reference to the reflector. + reflector: Heap<*mut JSObject>, +} + +impl MutNullableTransplantableDom { + /// Create a new `MutNullableTransplantableDom`. + /// + /// # Safety + /// + /// The constructed `MutNullableTransplantableDom` must be pinned before + /// use. + /// + /// FIXME: `std::pin::Pin` might be able to express this better + pub unsafe fn new() -> MutNullableTransplantableDom { + assert_in_script(); + MutNullableTransplantableDom { + ptr: UnsafeCell::new(None), + reflector: Heap::default(), + } + } + + /// Get a rooted DOM object out of this object. + #[allow(unrooted_must_root)] + pub fn get(&self) -> Option> { + assert_in_script(); + unsafe { ptr::read(self.ptr.get()).map(|o| DomRoot::from_ref(o.as_ref())) } + } + + /// Set this `MutNullableTransplantableDom` to the given value. The + /// reflector will be wrapped for `global`'s realm. + pub fn set(&self, val: Option<&T>, global: &crate::dom::globalscope::GlobalScope) { + assert_in_script(); + unsafe { + if let Some(dom) = val { + let cx = global.get_cx(); + let _ac = crate::realms::enter_realm(global); + rooted!(in(*cx) let mut reflector = *dom.reflector().get_jsobject()); + js::jsapi::JS_WrapObject(*cx, reflector.handle_mut().into()); + self.reflector.set(reflector.get()); + } else { + self.reflector.set(std::ptr::null_mut()); + } + + *self.ptr.get() = val.map(Into::into); + } + } +} + +unsafe impl JSTraceable for MutNullableTransplantableDom { + unsafe fn trace(&self, trc: *mut JSTracer) { + self.reflector.trace(trc); + } +} + +impl MallocSizeOf for MutNullableTransplantableDom { + fn size_of(&self, _ops: &mut MallocSizeOfOps) -> usize { + // See comment on MallocSizeOf for Dom. + 0 + } +} + +/// Essentially a [`DomOnceCell`], but directly references the reflector +/// so that, with a proper care, a cross-realm reference is prevented from +/// being formed by transplantation. +/// +/// This type can hold a reference to an un-[`Untransplantable`] DOM object. +/// In turn, such objects also need this sort of type to hold references to +/// other DOM objects whether they are transplantable or not (so the name is +/// inaccurate, actually). +/// +/// This should only be used as a field in other DOM objects; see warning +/// on `Dom`. +#[unrooted_must_root_lint::must_root] +pub struct TransplantableDomOnceCell { + /// A reference to the DOM object. + ptr: OnceCell>, + /// A tracable reference to the reflector. + /// + /// Invariant: `reflector` points to `ptr.reflector()` or its CCW. + reflector: Heap<*mut JSObject>, +} + +impl TransplantableDomOnceCell { + /// Create a new `TransplantableDomOnceCell`. + /// + /// # Safety + /// + /// The constructed `TransplantableDomOnceCell` must be pinned before + /// use. + /// + /// FIXME: `std::pin::Pin` might be able to express this better + pub unsafe fn new() -> TransplantableDomOnceCell { + assert_in_script(); + TransplantableDomOnceCell { + ptr: OnceCell::new(), + reflector: Heap::default(), + } + } + + // FIXME: The compartment invariants will be violated if an incorrect global + // scope is supplied. Should this method be `unsafe fn` because for + // this reason, or shouldn't it be because there are currently + // gazillions of other non-`unsafe` ways (`find_document` for one) to + // obtain other realms' DOM objects? + /// Set this `TransplantableDomOnceCell` to the given value. The + /// reflector will be wrapped for `global`'s realm. Does nothing if it's + /// already set. + /// + /// # Errors + /// + /// This method returns `Ok(())` if the cell was empty and `Err(())` if + /// it was full. + pub fn set<'a>( + &self, + val: Option<&T>, + global: &crate::dom::globalscope::GlobalScope, + ) -> Result<(), ()> { + assert_in_script(); + + if self.ptr.as_ref().is_some() { + return Err(()); + } + + if let Some(dom) = val { + self.ptr.init_once(|| { + // We've already checked the emptiness of `self.ptr` + debug_assert!(self.ptr.as_ref().is_none()); + + // Initialize `self.reflector`. + let cx = global.get_cx(); + let _ac = crate::realms::enter_realm(global); + rooted!(in(*cx) let mut reflector = *dom.reflector().get_jsobject()); + unsafe { js::jsapi::JS_WrapObject(*cx, reflector.handle_mut().into()) }; + + // The above code isn't supposed to initialize `self` reentrantly + assert!(self.reflector.get().is_null()); + self.reflector.set(reflector.get()); + + dom.into() + }); + } + Ok(()) + } + + /// Get a reference to the DOM object. + pub fn as_ref(&self) -> Option<&T> { + self.ptr.as_ref().map(|ptr| unsafe { &*ptr.as_ptr() }) + } + + /// Rewrap the reflector with a new realm. + pub fn rewrap(&self, global: &crate::dom::globalscope::GlobalScope) { + if self.reflector.get().is_null() { + return; + } + let cx = global.get_cx(); + let _ac = crate::realms::enter_realm(global); + rooted!(in(*cx) let mut reflector = self.reflector.get()); + unsafe { js::jsapi::JS_WrapObject(*cx, reflector.handle_mut().into()) }; + self.reflector.set(reflector.get()); + } +} + +unsafe impl JSTraceable for TransplantableDomOnceCell { + unsafe fn trace(&self, trc: *mut JSTracer) { + self.reflector.trace(trc); + } +} + +impl MallocSizeOf for TransplantableDomOnceCell { + fn size_of(&self, _ops: &mut MallocSizeOfOps) -> usize { + // See comment on MallocSizeOf for Dom. + 0 + } +} + impl<'dom, T> LayoutDom<'dom, T> where T: 'dom + DomObject, diff --git a/components/script/dom/bindings/trace.rs b/components/script/dom/bindings/trace.rs index a9b5733c9ff..54bb86667c6 100644 --- a/components/script/dom/bindings/trace.rs +++ b/components/script/dom/bindings/trace.rs @@ -32,7 +32,7 @@ use crate::dom::bindings::cell::DomRefCell; use crate::dom::bindings::error::Error; use crate::dom::bindings::refcounted::{Trusted, TrustedPromise}; -use crate::dom::bindings::reflector::{DomObject, Reflector}; +use crate::dom::bindings::reflector::{DomObject, Reflector, Untransplantable}; use crate::dom::bindings::root::{Dom, DomRoot}; use crate::dom::bindings::str::{DOMString, USVString}; use crate::dom::bindings::utils::WindowProxyHandler; @@ -1193,7 +1193,7 @@ impl<'a, T: 'static + JSTraceable> RootedVec<'a, T> { } } -impl<'a, T: 'static + JSTraceable + DomObject> RootedVec<'a, Dom> { +impl<'a, T: 'static + JSTraceable + DomObject + Untransplantable> RootedVec<'a, Dom> { /// Create a vector of items of type Dom that is rooted for /// the lifetime of this struct pub fn from_iter(root: &'a mut RootableVec>, iter: I) -> Self diff --git a/components/script/dom/webgl_extensions/extension.rs b/components/script/dom/webgl_extensions/extension.rs index 3854acee122..432b0672ce2 100644 --- a/components/script/dom/webgl_extensions/extension.rs +++ b/components/script/dom/webgl_extensions/extension.rs @@ -3,7 +3,7 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ use super::WebGLExtensions; -use crate::dom::bindings::reflector::DomObject; +use crate::dom::bindings::reflector::{DomObject, Untransplantable}; use crate::dom::bindings::root::DomRoot; use crate::dom::bindings::trace::JSTraceable; use crate::dom::webglrenderingcontext::WebGLRenderingContext; @@ -12,7 +12,7 @@ use canvas_traits::webgl::WebGLVersion; /// Trait implemented by WebGL extensions. pub trait WebGLExtension: Sized where - Self::Extension: DomObject + JSTraceable, + Self::Extension: DomObject + JSTraceable + Untransplantable, { type Extension; diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index 76dcc79424f..64ad1d72544 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -24,7 +24,7 @@ use crate::dom::bindings::inheritance::{Castable, ElementTypeId, HTMLElementType use crate::dom::bindings::num::Finite; use crate::dom::bindings::refcounted::Trusted; use crate::dom::bindings::reflector::DomObject; -use crate::dom::bindings::root::{Dom, DomRoot, MutNullableDom}; +use crate::dom::bindings::root::{Dom, DomRoot, MutNullableDom, MutNullableTransplantableDom}; use crate::dom::bindings::str::{DOMString, USVString}; use crate::dom::bindings::structuredclone; use crate::dom::bindings::trace::{JSTraceable, RootedTraceableBox}; @@ -201,7 +201,7 @@ pub struct Window { image_cache: Arc, #[ignore_malloc_size_of = "channels are hard"] image_cache_chan: Sender, - window_proxy: MutNullableDom, + window_proxy: MutNullableTransplantableDom, document: MutNullableDom, location: MutNullableDom, history: MutNullableDom, @@ -382,7 +382,7 @@ impl Window { pub fn clear_js_runtime_for_script_deallocation(&self) { unsafe { *self.js_runtime.borrow_for_script_deallocation() = None; - self.window_proxy.set(None); + self.window_proxy.set(None, &self.global().upcast()); self.current_state.set(WindowState::Zombie); self.ignore_all_tasks(); } @@ -1682,7 +1682,7 @@ impl Window { let pipeline_id = self.upcast::().pipeline_id(); if let Some(currently_active) = proxy.currently_active() { if currently_active == pipeline_id { - self.window_proxy.set(None); + self.window_proxy.set(None, &self.global()); } } } @@ -2190,7 +2190,7 @@ impl Window { #[allow(unsafe_code)] pub fn init_window_proxy(&self, window_proxy: &WindowProxy) { assert!(self.window_proxy.get().is_none()); - self.window_proxy.set(Some(&window_proxy)); + self.window_proxy.set(Some(&window_proxy), &self.global()); } #[allow(unsafe_code)] @@ -2596,7 +2596,8 @@ impl Window { location: Default::default(), history: Default::default(), custom_element_registry: Default::default(), - window_proxy: Default::default(), + // Safety: This field won't be assigned until it's pinned + window_proxy: unsafe { MutNullableTransplantableDom::new() }, document: Default::default(), performance: Default::default(), navigation_start: Cell::new(navigation_start), diff --git a/components/script/dom/windowproxy.rs b/components/script/dom/windowproxy.rs index 35850f76092..0f4264d6bb2 100644 --- a/components/script/dom/windowproxy.rs +++ b/components/script/dom/windowproxy.rs @@ -8,7 +8,7 @@ use crate::dom::bindings::error::{throw_dom_exception, Error, Fallible}; use crate::dom::bindings::inheritance::Castable; use crate::dom::bindings::proxyhandler::set_property_descriptor; use crate::dom::bindings::reflector::{DomObject, Reflector}; -use crate::dom::bindings::root::{Dom, DomRoot}; +use crate::dom::bindings::root::{DomRoot, TransplantableDomOnceCell}; use crate::dom::bindings::str::{DOMString, USVString}; use crate::dom::bindings::trace::JSTraceable; use crate::dom::bindings::utils::{get_array_index_from_id, AsVoidPtr, WindowProxyHandler}; @@ -58,10 +58,18 @@ use std::ptr; use style::attr::parse_integer; #[dom_struct] +// `dom_object(transplantable)` removes `!Untransplantable` impl. +#[dom_object(transplantable)] // NOTE: the browsing context for a window is managed in two places: // here, in script, but also in the constellation. The constellation // manages the session history, which in script is accessed through // History objects, messaging the constellation. +// +// `WindowProxy` being transplantable means all references to `WindowProxy` must +// be maintained through cross-realm compatible reference wrappers such as +// `*TransplantableDom*`. In addition, all DOM references contained within +// must be wrapped by `*TransplantableDom*` as well because their realms vary +// from `WindowProxy`'s point of view. pub struct WindowProxy { /// The JS WindowProxy object. /// Unlike other reflectors, we mutate this field because @@ -101,10 +109,10 @@ pub struct WindowProxy { is_closing: Cell, /// The containing iframe element, if this is a same-origin iframe - frame_element: Option>, + frame_element: TransplantableDomOnceCell, /// The parent browsing context's window proxy, if this is a nested browsing context - parent: Option>, + parent: TransplantableDomOnceCell, /// https://html.spec.whatwg.org/multipage/#delaying-load-events-mode delaying_load_events_mode: Cell, @@ -120,12 +128,12 @@ pub struct WindowProxy { } impl WindowProxy { + #[allow(unsafe_code)] pub fn new_inherited( browsing_context_id: BrowsingContextId, top_level_browsing_context_id: TopLevelBrowsingContextId, currently_active: Option, frame_element: Option<&Element>, - parent: Option<&WindowProxy>, opener: Option, creator: CreatorBrowsingContextInfo, ) -> WindowProxy { @@ -141,8 +149,9 @@ impl WindowProxy { 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), + // Safety: These fields are not set until they are pinned + frame_element: unsafe { TransplantableDomOnceCell::new() }, + parent: unsafe { TransplantableDomOnceCell::new() }, delaying_load_events_mode: Cell::new(false), opener, creator_base_url: creator.base_url, @@ -185,7 +194,6 @@ impl WindowProxy { top_level_browsing_context_id, current, frame_element, - parent, opener, creator, )); @@ -208,6 +216,17 @@ impl WindowProxy { js_proxy.get() ); window_proxy.reflector.set_jsobject(js_proxy.get()); + + // Set the remaining fields after pinning. + window_proxy + .frame_element + .set(frame_element, &window_proxy.global()) + .unwrap(); + window_proxy + .parent + .set(parent, &window_proxy.global()) + .unwrap(); + DomRoot::from_ref(&*Box::into_raw(window_proxy)) } } @@ -233,7 +252,6 @@ impl WindowProxy { top_level_browsing_context_id, None, None, - parent, opener, creator, )); @@ -270,6 +288,13 @@ impl WindowProxy { js_proxy.get() ); window_proxy.reflector.set_jsobject(js_proxy.get()); + + // Set the remaining field after pinning. + window_proxy + .parent + .set(parent, &window_proxy.global()) + .unwrap(); + DomRoot::from_ref(&*Box::into_raw(window_proxy)) } } @@ -423,7 +448,7 @@ impl WindowProxy { Some(opener_browsing_context_id) => opener_browsing_context_id, None => return NullValue(), }; - let parent_browsing_context = self.parent.as_deref(); + let parent_browsing_context = self.parent.as_ref(); let opener_proxy = match ScriptThread::find_window_proxy(opener_id) { Some(window_proxy) => window_proxy, None => { @@ -593,7 +618,7 @@ impl WindowProxy { } pub fn frame_element(&self) -> Option<&Element> { - self.frame_element.as_deref() + self.frame_element.as_ref() } pub fn document(&self) -> Option> { @@ -603,7 +628,7 @@ impl WindowProxy { } pub fn parent(&self) -> Option<&WindowProxy> { - self.parent.as_deref() + self.parent.as_ref() } pub fn top(&self) -> &WindowProxy { @@ -624,6 +649,11 @@ impl WindowProxy { let handler = CreateWrapperProxyHandler(traps); assert!(!handler.is_null()); + // FIXME: Disable compacting GC during this operation so that + // yet-to-be-rewrapped child pointers won't be traced with + // an incorrect compartment. We need something like + // `js::AutoDisableCompactingGC`. + let cx = window.get_cx(); let window_jsobject = window.reflector().get_jsobject(); let old_js_proxy = self.reflector.get_jsobject(); @@ -651,6 +681,10 @@ impl WindowProxy { rooted!(in(*cx) let new_js_proxy = JS_TransplantObject(*cx, old_js_proxy, new_js_proxy.handle())); debug!("Transplanted proxy is {:p}.", new_js_proxy.get()); + // Re-wrap reflectors for the new realm. + self.frame_element.rewrap(window); + self.parent.rewrap(window); + // Transfer ownership of this browsing context from the old window proxy to the new one. SetProxyReservedSlot(new_js_proxy.get(), 0, &PrivateValue(self.as_void_ptr())); diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index b417f453cdf..8b44f93aa14 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -30,7 +30,7 @@ use crate::dom::bindings::conversions::{ }; use crate::dom::bindings::inheritance::Castable; use crate::dom::bindings::refcounted::Trusted; -use crate::dom::bindings::reflector::DomObject; +use crate::dom::bindings::reflector::{AssertUntransplantable, DomObject}; use crate::dom::bindings::root::ThreadLocalStackRoots; use crate::dom::bindings::root::{Dom, DomRoot, MutNullableDom, RootCollection}; use crate::dom::bindings::str::DOMString; @@ -522,7 +522,11 @@ pub struct ScriptThread { documents: DomRefCell, /// The window proxies known by this thread /// TODO: this map grows, but never shrinks. Issue #15258. - window_proxies: DomRefCell>>, + /// + /// Safety: `AssertUntransplantable` is safe to be used here because + /// `ScriptThread` is rooted and not traced by other GC things. + window_proxies: + DomRefCell>>>, /// A list of data pertaining to loads that have not yet received a network response incomplete_loads: DomRefCell>, /// A vector containing parser contexts which have not yet been fully processed @@ -1118,7 +1122,7 @@ impl ScriptThread { .window_proxies .borrow() .get(&id) - .map(|context| DomRoot::from_ref(&**context)) + .map(|context| DomRoot::from_ref(&***context)) }) }) } @@ -1129,7 +1133,7 @@ impl ScriptThread { let script_thread = unsafe { &*script_thread }; for (_, proxy) in script_thread.window_proxies.borrow().iter() { if proxy.get_name() == *name { - return Some(DomRoot::from_ref(&**proxy)); + return Some(DomRoot::from_ref(&***proxy)); } } None @@ -3145,9 +3149,13 @@ impl ScriptThread { opener, creator, ); - self.window_proxies - .borrow_mut() - .insert(browsing_context_id, Dom::from_ref(&*window_proxy)); + + // Safety: See `ScriptThread::window_proxies`. + self.window_proxies.borrow_mut().insert( + browsing_context_id, + Dom::from_ref(unsafe { AssertUntransplantable::from_ref(&*window_proxy) }), + ); + Some(window_proxy) } @@ -3202,9 +3210,13 @@ impl ScriptThread { opener, creator, ); - self.window_proxies - .borrow_mut() - .insert(browsing_context_id, Dom::from_ref(&*window_proxy)); + + // Safety: See `ScriptThread::window_proxies`. + self.window_proxies.borrow_mut().insert( + browsing_context_id, + Dom::from_ref(unsafe { AssertUntransplantable::from_ref(&*window_proxy) }), + ); + window_proxy } diff --git a/components/script/window_named_properties.rs b/components/script/window_named_properties.rs index 89d677b1baf..320a80851a0 100644 --- a/components/script/window_named_properties.rs +++ b/components/script/window_named_properties.rs @@ -15,7 +15,9 @@ use js::error::throw_type_error; use js::glue::RUST_JSID_TO_STRING; use js::glue::{CreateProxyHandler, NewProxyObject, ProxyTraps, RUST_JSID_IS_STRING}; use js::jsapi::JS_SetImmutablePrototype; -use js::jsapi::{Handle, HandleObject, JSClass, JSContext, JSErrNum, UndefinedHandleValue}; +use js::jsapi::{ + Handle, HandleObject, JSClass, JSContext, JSErrNum, MutableHandleObject, UndefinedHandleValue, +}; use js::jsapi::{ HandleId, JSClass_NON_NATIVE, MutableHandle, MutableHandleIdVector, ObjectOpResult, PropertyDescriptor, ProxyClassExtension, ProxyClassOps, ProxyObjectOps, @@ -42,7 +44,7 @@ lazy_static! { ownPropertyKeys: Some(own_property_keys), delete_: Some(delete), enumerate: None, - getPrototypeIfOrdinary: None, + getPrototypeIfOrdinary: Some(get_prototype_if_ordinary), getPrototype: None, setPrototype: None, setImmutablePrototype: None, @@ -153,6 +155,18 @@ unsafe extern "C" fn delete( true } +#[allow(unsafe_code)] +unsafe extern "C" fn get_prototype_if_ordinary( + _cx: *mut JSContext, + proxy: HandleObject, + is_ordinary: *mut bool, + proto: MutableHandleObject, +) -> bool { + *is_ordinary = true; + proto.set(js::jsapi::GetStaticPrototype(proxy.get())); + true +} + #[allow(unsafe_code)] unsafe extern "C" fn prevent_extensions( _cx: *mut JSContext,