From 389f0d4cc294dc6c871aea7fd40771d1210495d0 Mon Sep 17 00:00:00 2001 From: Narfinger Date: Mon, 29 Sep 2025 11:46:49 +0200 Subject: [PATCH] Split WindowProxies in script to own struct and allow to get an Rc to it. (#39274) Split the window_proxies in script thread into its own struct with appropiate methods. ScriptThread allows to get an Rc to it. HtmlIFrameElement, Window and WindowProxy now get the Rc on construction from ScriptThread. Testing: Just a refactor so should not change any behavior. Fixes: Addresses part of https://github.com/servo/servo/issues/37969 --------- Signed-off-by: Narfinger --- .../script/dom/html/htmliframeelement.rs | 7 +- components/script/dom/window.rs | 9 +- components/script/dom/windowproxy.rs | 14 +- components/script/lib.rs | 1 + components/script/script_thread.rs | 154 ++-------------- components/script/script_window_proxies.rs | 173 ++++++++++++++++++ 6 files changed, 216 insertions(+), 142 deletions(-) create mode 100644 components/script/script_window_proxies.rs diff --git a/components/script/dom/html/htmliframeelement.rs b/components/script/dom/html/htmliframeelement.rs index 462007e1cba..57986b2cc42 100644 --- a/components/script/dom/html/htmliframeelement.rs +++ b/components/script/dom/html/htmliframeelement.rs @@ -3,6 +3,7 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ use std::cell::Cell; +use std::rc::Rc; use base::id::{BrowsingContextId, PipelineId, WebViewId}; use bitflags::bitflags; @@ -48,6 +49,7 @@ use crate::dom::virtualmethods::VirtualMethods; use crate::dom::windowproxy::WindowProxy; use crate::script_runtime::CanGc; use crate::script_thread::ScriptThread; +use crate::script_window_proxies::ScriptWindowProxies; #[derive(Clone, Copy, JSTraceable, MallocSizeOf)] struct SandboxAllowance(u8); @@ -93,6 +95,8 @@ pub(crate) struct HTMLIFrameElement { sandbox_allowance: Cell>, load_blocker: DomRefCell>, throttled: Cell, + #[conditional_malloc_size_of] + script_window_proxies: Rc, } impl HTMLIFrameElement { @@ -490,6 +494,7 @@ impl HTMLIFrameElement { sandbox_allowance: Cell::new(None), load_blocker: DomRefCell::new(None), throttled: Cell::new(false), + script_window_proxies: ScriptThread::window_proxies(), } } @@ -649,7 +654,7 @@ impl HTMLIFrameElementMethods for HTMLIFrameElement { fn GetContentWindow(&self) -> Option> { self.browsing_context_id .get() - .and_then(ScriptThread::find_window_proxy) + .and_then(|id| self.script_window_proxies.find_window_proxy(id)) } // https://html.spec.whatwg.org/multipage/#dom-iframe-contentdocument diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index f56254af720..c99d8a91be7 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -178,6 +178,7 @@ use crate::microtask::MicrotaskQueue; use crate::realms::{InRealm, enter_realm}; use crate::script_runtime::{CanGc, JSContext, Runtime}; use crate::script_thread::ScriptThread; +use crate::script_window_proxies::ScriptWindowProxies; use crate::timers::{IsInterval, TimerCallback}; use crate::unminify::unminified_path; use crate::webdriver_handlers::{find_node_by_unique_id_in_document, jsval_to_webdriver}; @@ -442,6 +443,10 @@ pub(crate) struct Window { /// #[no_trace] endpoints_list: DomRefCell>, + + /// The window proxies the script thread knows. + #[ignore_malloc_size_of = "Rc"] + script_window_proxies: Rc, } impl Window { @@ -607,7 +612,8 @@ impl Window { /// pub(crate) fn webview_window_proxy(&self) -> Option> { self.undiscarded_window_proxy().and_then(|window_proxy| { - ScriptThread::find_window_proxy(window_proxy.webview_id().into()) + self.script_window_proxies + .find_window_proxy(window_proxy.webview_id().into()) }) } @@ -3332,6 +3338,7 @@ impl Window { reporting_observer_list: Default::default(), report_list: Default::default(), endpoints_list: Default::default(), + script_window_proxies: ScriptThread::window_proxies(), }); WindowBinding::Wrap::(GlobalScope::get_cx(), win) diff --git a/components/script/dom/windowproxy.rs b/components/script/dom/windowproxy.rs index 1c30b439604..6ac44addfe6 100644 --- a/components/script/dom/windowproxy.rs +++ b/components/script/dom/windowproxy.rs @@ -4,6 +4,7 @@ use std::cell::Cell; use std::ptr; +use std::rc::Rc; use base::generic_channel; use base::generic_channel::GenericSend; @@ -59,6 +60,7 @@ use crate::dom::window::Window; use crate::realms::{AlreadyInRealm, InRealm, enter_realm}; use crate::script_runtime::{CanGc, JSContext as SafeJSContext}; use crate::script_thread::ScriptThread; +use crate::script_window_proxies::ScriptWindowProxies; #[dom_struct] // NOTE: the browsing context for a window is managed in two places: @@ -129,6 +131,10 @@ pub(crate) struct WindowProxy { /// The creator browsing context's origin. #[no_trace] creator_origin: Option, + + /// The window proxies the script thread knows. + #[ignore_malloc_size_of = "Rc"] + script_window_proxies: Rc, } impl WindowProxy { @@ -160,6 +166,7 @@ impl WindowProxy { creator_base_url: creator.base_url, creator_url: creator.url, creator_origin: creator.origin, + script_window_proxies: ScriptThread::window_proxies(), } } @@ -444,7 +451,7 @@ impl WindowProxy { None => return retval.set(NullValue()), }; let parent_browsing_context = self.parent.as_deref(); - let opener_proxy = match ScriptThread::find_window_proxy(opener_id) { + let opener_proxy = match self.script_window_proxies.find_window_proxy(opener_id) { Some(window_proxy) => window_proxy, None => { let sender_pipeline_id = self.currently_active().unwrap(); @@ -920,6 +927,7 @@ unsafe fn GetSubframeWindowProxy( let mut slot = UndefinedValue(); GetProxyPrivate(*proxy, &mut slot); rooted!(in(cx) let target = slot.to_object()); + let script_window_proxies = ScriptThread::window_proxies(); if let Ok(win) = root_from_handleobject::(target.handle(), cx) { let browsing_context_id = win.window_proxy().browsing_context_id(); let (result_sender, result_receiver) = ipc::channel().unwrap(); @@ -935,7 +943,7 @@ unsafe fn GetSubframeWindowProxy( .recv() .ok() .and_then(|maybe_bcid| maybe_bcid) - .and_then(ScriptThread::find_window_proxy) + .and_then(|id| script_window_proxies.find_window_proxy(id)) .map(|proxy| (proxy, (JSPROP_ENUMERATE | JSPROP_READONLY) as u32)); } else if let Ok(win) = root_from_handleobject::(target.handle(), cx) @@ -954,7 +962,7 @@ unsafe fn GetSubframeWindowProxy( .recv() .ok() .and_then(|maybe_bcid| maybe_bcid) - .and_then(ScriptThread::find_window_proxy) + .and_then(|id| script_window_proxies.find_window_proxy(id)) .map(|proxy| (proxy, JSPROP_READONLY as u32)); } } diff --git a/components/script/lib.rs b/components/script/lib.rs index 28dc3d087eb..3ea1d79d8d8 100644 --- a/components/script/lib.rs +++ b/components/script/lib.rs @@ -22,6 +22,7 @@ extern crate stylo_atoms; mod animation_timeline; mod animations; +mod script_window_proxies; #[macro_use] mod task; mod body; diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index 3773ab56968..1a449b72a8a 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -81,7 +81,7 @@ use percent_encoding::percent_decode; use profile_traits::mem::{ProcessReports, ReportsChan, perform_memory_report}; use profile_traits::time::ProfilerCategory; use profile_traits::time_profile; -use rustc_hash::{FxBuildHasher, FxHashMap, FxHashSet}; +use rustc_hash::{FxHashMap, FxHashSet}; use script_traits::{ ConstellationInputEvent, DiscardBrowsingContext, DocumentActivity, InitialScriptState, NewLayoutInfo, Painter, ProgressiveWebMetricType, ScriptThreadMessage, UpdatePipelineIdReason, @@ -116,7 +116,7 @@ use crate::dom::bindings::reflector::DomGlobal; use crate::dom::bindings::root::{Dom, DomRoot}; use crate::dom::bindings::settings_stack::AutoEntryScript; use crate::dom::bindings::str::DOMString; -use crate::dom::bindings::trace::{HashMapTracedValues, JSTraceable}; +use crate::dom::bindings::trace::JSTraceable; use crate::dom::csp::{CspReporting, GlobalCspReporting, Violation}; use crate::dom::customelementregistry::{ CallbackReaction, CustomElementDefinition, CustomElementReactionStack, @@ -133,7 +133,7 @@ use crate::dom::types::DebuggerGlobalScope; #[cfg(feature = "webgpu")] use crate::dom::webgpu::identityhub::IdentityHub; use crate::dom::window::Window; -use crate::dom::windowproxy::{CreatorBrowsingContextInfo, WindowProxy}; +use crate::dom::windowproxy::WindowProxy; use crate::dom::worklet::WorkletThreadPool; use crate::dom::workletglobalscope::WorkletGlobalScopeInit; use crate::fetch::FetchCanceller; @@ -151,6 +151,7 @@ use crate::script_runtime::{ CanGc, IntroductionType, JSContext, JSContextHelper, Runtime, ScriptThreadEventCategory, ThreadSafeJSContext, }; +use crate::script_window_proxies::ScriptWindowProxies; use crate::task_queue::TaskQueue; use crate::task_source::{SendableTaskSource, TaskSourceName}; use crate::webdriver_handlers::jsval_to_webdriver; @@ -227,9 +228,7 @@ pub struct ScriptThread { /// The documents for pipelines managed by this thread documents: DomRefCell, /// The window proxies known by this thread - /// TODO: this map grows, but never shrinks. Issue #15258. - window_proxies: - DomRefCell, FxBuildHasher>>, + window_proxies: Rc, /// 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 @@ -723,24 +722,13 @@ impl ScriptThread { }) } - pub(crate) fn find_window_proxy(id: BrowsingContextId) -> Option> { - with_script_thread(|script_thread| { - script_thread - .window_proxies - .borrow() - .get(&id) - .map(|context| DomRoot::from_ref(&**context)) - }) + pub(crate) fn window_proxies() -> Rc { + with_script_thread(|script_thread| script_thread.window_proxies.clone()) } pub(crate) fn find_window_proxy_by_name(name: &DOMString) -> Option> { with_script_thread(|script_thread| { - for (_, proxy) in script_thread.window_proxies.borrow().iter() { - if proxy.get_name() == *name { - return Some(DomRoot::from_ref(&**proxy)); - } - } - None + script_thread.window_proxies.find_window_proxy_by_name(name) }) } @@ -965,7 +953,7 @@ impl ScriptThread { ScriptThread { documents: DomRefCell::new(DocumentCollection::default()), last_render_opportunity_time: Default::default(), - window_proxies: DomRefCell::new(HashMapTracedValues::new_fx()), + window_proxies: Default::default(), incomplete_loads: DomRefCell::new(vec![]), incomplete_parser_contexts: IncompleteParserContexts(RefCell::new(vec![])), senders, @@ -2744,7 +2732,8 @@ impl ScriptThread { Some(window) => { // FIXME: synchronously talks to constellation. // send the required info as part of postmessage instead. - let source = match self.remote_window_proxy( + let source = match self.window_proxies.remote_window_proxy( + &self.senders, window.upcast::(), source_browsing_context, source_pipeline_id, @@ -2804,7 +2793,9 @@ impl ScriptThread { if let Some(window) = self.documents.borrow().find_window(new_pipeline_id) { // Ensure that the state of any local window proxies accurately reflects // the new pipeline. - let _ = self.local_window_proxy( + let _ = self.window_proxies.local_window_proxy( + &self.senders, + &self.documents, &window, browsing_context_id, webview_id, @@ -3097,21 +3088,6 @@ impl ScriptThread { } } - fn ask_constellation_for_browsing_context_info( - &self, - pipeline_id: PipelineId, - ) -> Option<(BrowsingContextId, Option)> { - let (result_sender, result_receiver) = ipc::channel().unwrap(); - let msg = ScriptToConstellationMessage::GetBrowsingContextInfo(pipeline_id, result_sender); - self.senders - .pipeline_to_constellation_sender - .send((pipeline_id, msg)) - .expect("Failed to send to constellation."); - result_receiver - .recv() - .expect("Failed to get browsing context info from constellation.") - } - fn ask_constellation_for_top_level_info( &self, sender_pipeline: PipelineId, @@ -3131,104 +3107,6 @@ impl ScriptThread { .expect("Failed to get top-level id from constellation.") } - // Get the browsing context for a pipeline that may exist in another - // script thread. If the browsing context already exists in the - // `window_proxies` map, we return it, otherwise we recursively - // get the browsing context for the parent if there is one, - // construct a new dissimilar-origin browsing context, add it - // to the `window_proxies` map, and return it. - fn remote_window_proxy( - &self, - global_to_clone: &GlobalScope, - webview_id: WebViewId, - pipeline_id: PipelineId, - opener: Option, - ) -> Option> { - let (browsing_context_id, parent_pipeline_id) = - self.ask_constellation_for_browsing_context_info(pipeline_id)?; - if let Some(window_proxy) = self.window_proxies.borrow().get(&browsing_context_id) { - return Some(DomRoot::from_ref(window_proxy)); - } - - let parent_browsing_context = parent_pipeline_id.and_then(|parent_id| { - self.remote_window_proxy(global_to_clone, webview_id, parent_id, opener) - }); - - let opener_browsing_context = opener.and_then(ScriptThread::find_window_proxy); - - let creator = CreatorBrowsingContextInfo::from( - parent_browsing_context.as_deref(), - opener_browsing_context.as_deref(), - ); - - let window_proxy = WindowProxy::new_dissimilar_origin( - global_to_clone, - browsing_context_id, - webview_id, - parent_browsing_context.as_deref(), - opener, - creator, - ); - self.window_proxies - .borrow_mut() - .insert(browsing_context_id, Dom::from_ref(&*window_proxy)); - Some(window_proxy) - } - - // Get the browsing context for a pipeline that exists in this - // script thread. If the browsing context already exists in the - // `window_proxies` map, we return it, otherwise we recursively - // get the browsing context for the parent if there is one, - // construct a new similar-origin browsing context, add it - // to the `window_proxies` map, and return it. - fn local_window_proxy( - &self, - window: &Window, - browsing_context_id: BrowsingContextId, - webview_id: WebViewId, - parent_info: Option, - opener: Option, - ) -> DomRoot { - if let Some(window_proxy) = self.window_proxies.borrow().get(&browsing_context_id) { - // Note: we do not set the window to be the currently-active one, - // this will be done instead when the script-thread handles the `SetDocumentActivity` msg. - return DomRoot::from_ref(window_proxy); - } - let iframe = parent_info.and_then(|parent_id| { - self.documents - .borrow() - .find_iframe(parent_id, browsing_context_id) - }); - let parent_browsing_context = match (parent_info, iframe.as_ref()) { - (_, Some(iframe)) => Some(iframe.owner_window().window_proxy()), - (Some(parent_id), _) => { - self.remote_window_proxy(window.upcast(), webview_id, parent_id, opener) - }, - _ => None, - }; - - let opener_browsing_context = opener.and_then(ScriptThread::find_window_proxy); - - let creator = CreatorBrowsingContextInfo::from( - parent_browsing_context.as_deref(), - opener_browsing_context.as_deref(), - ); - - let window_proxy = WindowProxy::new( - window, - browsing_context_id, - webview_id, - iframe.as_deref().map(Castable::upcast), - parent_browsing_context.as_deref(), - opener, - creator, - ); - self.window_proxies - .borrow_mut() - .insert(browsing_context_id, Dom::from_ref(&*window_proxy)); - window_proxy - } - /// The entry point to document loading. Defines bindings, sets up the window and document /// objects, parses HTML and CSS, and kicks off initial layout. fn load( @@ -3343,7 +3221,9 @@ impl ScriptThread { let _realm = enter_realm(&*window); // Initialize the browsing context for the window. - let window_proxy = self.local_window_proxy( + let window_proxy = self.window_proxies.local_window_proxy( + &self.senders, + &self.documents, &window, incomplete.browsing_context_id, incomplete.webview_id, diff --git a/components/script/script_window_proxies.rs b/components/script/script_window_proxies.rs new file mode 100644 index 00000000000..dce7ee175b5 --- /dev/null +++ b/components/script/script_window_proxies.rs @@ -0,0 +1,173 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ + +use base::id::{BrowsingContextId, PipelineId, WebViewId}; +use constellation_traits::ScriptToConstellationMessage; +use ipc_channel::ipc; +use rustc_hash::FxBuildHasher; +use script_bindings::inheritance::Castable; +use script_bindings::root::{Dom, DomRoot}; +use script_bindings::str::DOMString; + +use crate::document_collection::DocumentCollection; +use crate::dom::bindings::cell::DomRefCell; +use crate::dom::bindings::trace::HashMapTracedValues; +use crate::dom::node::NodeTraits; +use crate::dom::types::{GlobalScope, Window}; +use crate::dom::windowproxy::{CreatorBrowsingContextInfo, WindowProxy}; +use crate::messaging::ScriptThreadSenders; + +#[derive(JSTraceable, Default, MallocSizeOf)] +#[cfg_attr(crown, crown::unrooted_must_root_lint::allow_unrooted_in_rc)] +#[cfg_attr(crown, crown::unrooted_must_root_lint::must_root)] +pub(crate) struct ScriptWindowProxies { + /// TODO: this map grows, but never shrinks. Issue #15258. + map: DomRefCell, FxBuildHasher>>, +} + +impl ScriptWindowProxies { + pub(crate) fn find_window_proxy(&self, id: BrowsingContextId) -> Option> { + self.map + .borrow() + .get(&id) + .map(|context| DomRoot::from_ref(&**context)) + } + + pub(crate) fn find_window_proxy_by_name( + &self, + name: &DOMString, + ) -> Option> { + for (_, proxy) in self.map.borrow().iter() { + if proxy.get_name() == *name { + return Some(DomRoot::from_ref(&**proxy)); + } + } + None + } + + pub(crate) fn get(&self, id: BrowsingContextId) -> Option> { + self.map + .borrow() + .get(&id) + .map(|context| DomRoot::from_ref(&**context)) + } + + pub(crate) fn insert(&self, id: BrowsingContextId, proxy: DomRoot) { + self.map.borrow_mut().insert(id, Dom::from_ref(&*proxy)); + } + + // Get the browsing context for a pipeline that may exist in another + // script thread. If the browsing context already exists in the + // `window_proxies` map, we return it, otherwise we recursively + // get the browsing context for the parent if there is one, + // construct a new dissimilar-origin browsing context, add it + // to the `window_proxies` map, and return it. + pub(crate) fn remote_window_proxy( + &self, + senders: &ScriptThreadSenders, + global_to_clone: &GlobalScope, + webview_id: WebViewId, + pipeline_id: PipelineId, + opener: Option, + ) -> Option> { + let (browsing_context_id, parent_pipeline_id) = + self.ask_constellation_for_browsing_context_info(senders, pipeline_id)?; + if let Some(window_proxy) = self.get(browsing_context_id) { + return Some(window_proxy); + } + + let parent_browsing_context = parent_pipeline_id.and_then(|parent_id| { + self.remote_window_proxy(senders, global_to_clone, webview_id, parent_id, opener) + }); + + let opener_browsing_context = opener.and_then(|id| self.find_window_proxy(id)); + + let creator = CreatorBrowsingContextInfo::from( + parent_browsing_context.as_deref(), + opener_browsing_context.as_deref(), + ); + + let window_proxy = WindowProxy::new_dissimilar_origin( + global_to_clone, + browsing_context_id, + webview_id, + parent_browsing_context.as_deref(), + opener, + creator, + ); + self.insert(browsing_context_id, DomRoot::from_ref(&*window_proxy)); + Some(window_proxy) + } + + // Get the browsing context for a pipeline that exists in this + // script thread. If the browsing context already exists in the + // `window_proxies` map, we return it, otherwise we recursively + // get the browsing context for the parent if there is one, + // construct a new similar-origin browsing context, add it + // to the `window_proxies` map, and return it. + #[allow(clippy::too_many_arguments)] + pub(crate) fn local_window_proxy( + &self, + senders: &ScriptThreadSenders, + documents: &DomRefCell, + window: &Window, + browsing_context_id: BrowsingContextId, + webview_id: WebViewId, + parent_info: Option, + opener: Option, + ) -> DomRoot { + if let Some(window_proxy) = self.get(browsing_context_id) { + // Note: we do not set the window to be the currently-active one, + // this will be done instead when the script-thread handles the `SetDocumentActivity` msg. + return window_proxy; + } + let iframe = parent_info.and_then(|parent_id| { + documents + .borrow() + .find_iframe(parent_id, browsing_context_id) + }); + let parent_browsing_context = match (parent_info, iframe.as_ref()) { + (_, Some(iframe)) => Some(iframe.owner_window().window_proxy()), + (Some(parent_id), _) => { + self.remote_window_proxy(senders, window.upcast(), webview_id, parent_id, opener) + }, + _ => None, + }; + + let opener_browsing_context = opener.and_then(|id| self.find_window_proxy(id)); + + let creator = CreatorBrowsingContextInfo::from( + parent_browsing_context.as_deref(), + opener_browsing_context.as_deref(), + ); + + let window_proxy = WindowProxy::new( + window, + browsing_context_id, + webview_id, + iframe.as_deref().map(Castable::upcast), + parent_browsing_context.as_deref(), + opener, + creator, + ); + self.insert(browsing_context_id, DomRoot::from_ref(&*window_proxy)); + window_proxy + } + + fn ask_constellation_for_browsing_context_info( + &self, + senders: &ScriptThreadSenders, + pipeline_id: PipelineId, + ) -> Option<(BrowsingContextId, Option)> { + let (result_sender, result_receiver) = ipc::channel().unwrap(); + let msg = ScriptToConstellationMessage::GetBrowsingContextInfo(pipeline_id, result_sender); + senders + .pipeline_to_constellation_sender + .send((pipeline_id, msg)) + .expect("Failed to send to constellation."); + result_receiver + .recv() + .expect("Failed to get browsing context info from constellation.") + } +}