From d1715918f058649b3a637a53c2cd920740b2eb37 Mon Sep 17 00:00:00 2001 From: CYBAI Date: Sat, 11 Jul 2020 22:44:21 +0900 Subject: [PATCH] Set private reference for classic script Web developers can use `Dynamic Import` in a classic script; thus, we need to save the script's private reference so that we can reuse it when we're going to fetch a dynamic import module for a classic script. Besides, because it's possible to use different executing context for a dynamic import module (like `dynamic-import/string-compilation-other-document.html` WPT test), we can't initialize a module owner at the timing of `SetScriptPrivate`; thus, if the private module script doesn't hold an owner, we'll use a DynamicImport owner for it. --- components/script/devtools.rs | 10 ++- components/script/dom/bindings/trace.rs | 4 +- components/script/dom/globalscope.rs | 93 +++++++++++++++------ components/script/dom/htmlscriptelement.rs | 26 +++++- components/script/dom/userscripts.rs | 17 ++-- components/script/dom/workletglobalscope.rs | 11 ++- components/script/script_module.rs | 76 ++++++++++++----- components/script/script_thread.rs | 8 +- components/script/timers.rs | 9 +- components/script/webdriver_handlers.rs | 21 +++-- 10 files changed, 204 insertions(+), 71 deletions(-) diff --git a/components/script/devtools.rs b/components/script/devtools.rs index 2aad7d101c7..cb5dfa27b7f 100644 --- a/components/script/devtools.rs +++ b/components/script/devtools.rs @@ -16,6 +16,7 @@ use crate::dom::element::Element; use crate::dom::globalscope::GlobalScope; use crate::dom::node::{window_from_node, Node, ShadowIncluding}; use crate::realms::enter_realm; +use crate::script_module::ScriptFetchOptions; use crate::script_thread::Documents; use devtools_traits::{AutoMargins, ComputedNodeLayout, TimelineMarkerType}; use devtools_traits::{EvaluateJSReply, Modification, NodeInfo, TimelineMarker}; @@ -34,7 +35,14 @@ pub fn handle_evaluate_js(global: &GlobalScope, eval: String, reply: IpcSender", rval.handle_mut(), 1); + global.evaluate_script_on_global_with_result( + &eval, + "", + rval.handle_mut(), + 1, + ScriptFetchOptions::default_classic_script(&global), + global.api_base_url(), + ); if rval.is_undefined() { EvaluateJSReply::VoidValue diff --git a/components/script/dom/bindings/trace.rs b/components/script/dom/bindings/trace.rs index 35ec7beaf05..8dbb7ff08ba 100644 --- a/components/script/dom/bindings/trace.rs +++ b/components/script/dom/bindings/trace.rs @@ -95,7 +95,7 @@ use msg::constellation_msg::{ServiceWorkerId, ServiceWorkerRegistrationId}; use net_traits::filemanager_thread::RelativePos; use net_traits::image::base::{Image, ImageMetadata}; use net_traits::image_cache::{ImageCache, PendingImageId}; -use net_traits::request::{Referrer, Request, RequestBuilder}; +use net_traits::request::{CredentialsMode, ParserMetadata, Referrer, Request, RequestBuilder}; use net_traits::response::HttpsState; use net_traits::response::{Response, ResponseBody}; use net_traits::storage_thread::StorageType; @@ -556,6 +556,8 @@ unsafe_no_jsmanaged_fields!(StyleSharedRwLock); unsafe_no_jsmanaged_fields!(USVString); unsafe_no_jsmanaged_fields!(Referrer); unsafe_no_jsmanaged_fields!(ReferrerPolicy); +unsafe_no_jsmanaged_fields!(CredentialsMode); +unsafe_no_jsmanaged_fields!(ParserMetadata); unsafe_no_jsmanaged_fields!(Response); unsafe_no_jsmanaged_fields!(ResponseBody); unsafe_no_jsmanaged_fields!(ResourceThreads); diff --git a/components/script/dom/globalscope.rs b/components/script/dom/globalscope.rs index 230f2517397..c0661aab4ba 100644 --- a/components/script/dom/globalscope.rs +++ b/components/script/dom/globalscope.rs @@ -52,6 +52,7 @@ use crate::dom::workletglobalscope::WorkletGlobalScope; use crate::microtask::{Microtask, MicrotaskQueue, UserMicrotask}; use crate::realms::{enter_realm, AlreadyInRealm, InRealm}; use crate::script_module::{DynamicModuleList, ModuleTree}; +use crate::script_module::{ModuleScript, ScriptFetchOptions}; use crate::script_runtime::{ CommonScriptMsg, ContextForRequestInterrupt, JSContext as SafeJSContext, ScriptChan, ScriptPort, }; @@ -79,8 +80,10 @@ use ipc_channel::router::ROUTER; use js::glue::{IsWrapper, UnwrapObjectDynamic}; use js::jsapi::Compile1; use js::jsapi::{CurrentGlobalOrNull, GetNonCCWObjectGlobal}; +use js::jsapi::{GetScriptPrivate, SetScriptPrivate}; use js::jsapi::{HandleObject, Heap}; use js::jsapi::{JSContext, JSObject}; +use js::jsval::PrivateValue; use js::jsval::{JSVal, UndefinedValue}; use js::panic::maybe_resume_unwind; use js::rust::transform_str_to_source_text; @@ -2533,8 +2536,21 @@ impl GlobalScope { } /// Evaluate JS code on this global scope. - pub fn evaluate_js_on_global_with_result(&self, code: &str, rval: MutableHandleValue) -> bool { - self.evaluate_script_on_global_with_result(code, "", rval, 1) + pub fn evaluate_js_on_global_with_result( + &self, + code: &str, + rval: MutableHandleValue, + fetch_options: ScriptFetchOptions, + script_base_url: ServoUrl, + ) -> bool { + self.evaluate_script_on_global_with_result( + code, + "", + rval, + 1, + fetch_options, + script_base_url, + ) } /// Evaluate a JS script on this global scope. @@ -2545,6 +2561,8 @@ impl GlobalScope { filename: &str, rval: MutableHandleValue, line_number: u32, + fetch_options: ScriptFetchOptions, + script_base_url: ServoUrl, ) -> bool { let metadata = profile_time::TimerMetadata { url: if filename.is_empty() { @@ -2566,33 +2584,56 @@ impl GlobalScope { let ar = enter_realm(&*self); let _aes = AutoEntryScript::new(self); - let options = - unsafe { CompileOptionsWrapper::new(*cx, filename.as_ptr(), line_number) }; - rooted!(in(*cx) let compiled_script = unsafe { - Compile1( - *cx, - options.ptr, - &mut transform_str_to_source_text(code), - ) - }); + unsafe { + let options = CompileOptionsWrapper::new(*cx, filename.as_ptr(), line_number); - if compiled_script.is_null() { - debug!("error compiling Dom string"); - report_pending_exception(*cx, true, InRealm::Entered(&ar)); - return false; + debug!("compiling Dom string"); + rooted!(in(*cx) let compiled_script = + Compile1( + *cx, + options.ptr, + &mut transform_str_to_source_text(code), + ) + ); + + if compiled_script.is_null() { + debug!("error compiling Dom string"); + report_pending_exception(*cx, true, InRealm::Entered(&ar)); + return false; + } + + // When `ScriptPrivate` for the compiled script is undefined, + // we need to set it so that it can be used in dynamic import context. + if GetScriptPrivate(*compiled_script).is_undefined() { + debug!("Set script private for {}", script_base_url); + + let module_script_data = Box::new(ModuleScript::new( + script_base_url, + fetch_options, + // We can't initialize an module owner here because + // the executing context of script might be different + // from the dynamic import script's executing context. + None, + )); + + SetScriptPrivate( + *compiled_script, + &PrivateValue(Box::into_raw(module_script_data) as *const _), + ); + } + + debug!("evaluating Dom string"); + let result = JS_ExecuteScript(*cx, compiled_script.handle(), rval); + + if !result { + debug!("error evaluating Dom string"); + report_pending_exception(*cx, true, InRealm::Entered(&ar)); + } + + maybe_resume_unwind(); + result } - - debug!("evaluating Dom string"); - let result = unsafe { JS_ExecuteScript(*cx, compiled_script.handle(), rval) }; - - if !result { - debug!("error evaluating Dom string"); - unsafe { report_pending_exception(*cx, true, InRealm::Entered(&ar)) }; - } - - maybe_resume_unwind(); - result }, ) } diff --git a/components/script/dom/htmlscriptelement.rs b/components/script/dom/htmlscriptelement.rs index 44fe52a51d6..6a2b5e53acc 100644 --- a/components/script/dom/htmlscriptelement.rs +++ b/components/script/dom/htmlscriptelement.rs @@ -156,24 +156,37 @@ pub struct ScriptOrigin { text: Rc, url: ServoUrl, external: bool, + fetch_options: ScriptFetchOptions, type_: ScriptType, } impl ScriptOrigin { - pub fn internal(text: Rc, url: ServoUrl, type_: ScriptType) -> ScriptOrigin { + pub fn internal( + text: Rc, + url: ServoUrl, + fetch_options: ScriptFetchOptions, + type_: ScriptType, + ) -> ScriptOrigin { ScriptOrigin { text: text, url: url, external: false, + fetch_options, type_, } } - pub fn external(text: Rc, url: ServoUrl, type_: ScriptType) -> ScriptOrigin { + pub fn external( + text: Rc, + url: ServoUrl, + fetch_options: ScriptFetchOptions, + type_: ScriptType, + ) -> ScriptOrigin { ScriptOrigin { text: text, url: url, external: true, + fetch_options, type_, } } @@ -202,6 +215,8 @@ struct ClassicContext { url: ServoUrl, /// Indicates whether the request failed, and why status: Result<(), NetworkError>, + /// The fetch options of the script + fetch_options: ScriptFetchOptions, /// Timing object for this resource resource_timing: ResourceFetchTiming, } @@ -262,6 +277,7 @@ impl FetchResponseListener for ClassicContext { ScriptOrigin::external( Rc::new(DOMString::from(source_text)), metadata.final_url, + self.fetch_options.clone(), ScriptType::Classic, ) }); @@ -358,7 +374,7 @@ fn fetch_a_classic_script( cors_setting, doc.origin().immutable().clone(), script.global().pipeline_id(), - options, + options.clone(), ); // TODO: Step 3, Add custom steps to perform fetch @@ -371,6 +387,7 @@ fn fetch_a_classic_script( metadata: None, url: url.clone(), status: Ok(()), + fetch_options: options, resource_timing: ResourceFetchTiming::new(ResourceTimingType::Resource), })); @@ -630,6 +647,7 @@ impl HTMLScriptElement { let result = Ok(ScriptOrigin::internal( Rc::clone(&text_rc), base_url.clone(), + options.clone(), script_type.clone(), )); @@ -866,6 +884,8 @@ impl HTMLScriptElement { script.url.as_str(), rval.handle_mut(), line_number, + script.fetch_options.clone(), + script.url.clone(), ); } diff --git a/components/script/dom/userscripts.rs b/components/script/dom/userscripts.rs index 4e9cc59d27f..a004e0fe6b8 100644 --- a/components/script/dom/userscripts.rs +++ b/components/script/dom/userscripts.rs @@ -7,6 +7,7 @@ use crate::dom::bindings::refcounted::Trusted; use crate::dom::globalscope::GlobalScope; use crate::dom::htmlheadelement::HTMLHeadElement; use crate::dom::node::document_from_node; +use crate::script_module::ScriptFetchOptions; use js::jsval::UndefinedValue; use std::fs::{read_dir, File}; use std::io::Read; @@ -38,13 +39,15 @@ pub fn load_script(head: &HTMLHeadElement) { let mut contents = vec![]; f.read_to_end(&mut contents).unwrap(); let script_text = String::from_utf8_lossy(&contents); - win.upcast::() - .evaluate_script_on_global_with_result( - &script_text, - &file.to_string_lossy(), - rval.handle_mut(), - 1, - ); + let global = win.upcast::(); + global.evaluate_script_on_global_with_result( + &script_text, + &file.to_string_lossy(), + rval.handle_mut(), + 1, + ScriptFetchOptions::default_classic_script(&global), + global.api_base_url(), + ); } })); } diff --git a/components/script/dom/workletglobalscope.rs b/components/script/dom/workletglobalscope.rs index 88b8a13708b..47e6315c940 100644 --- a/components/script/dom/workletglobalscope.rs +++ b/components/script/dom/workletglobalscope.rs @@ -11,6 +11,7 @@ use crate::dom::paintworkletglobalscope::PaintWorkletTask; use crate::dom::testworkletglobalscope::TestWorkletGlobalScope; use crate::dom::testworkletglobalscope::TestWorkletTask; use crate::dom::worklet::WorkletExecutor; +use crate::script_module::ScriptFetchOptions; use crate::script_runtime::JSContext; use crate::script_thread::MainThreadScriptMsg; use crossbeam_channel::Sender; @@ -88,10 +89,14 @@ impl WorkletGlobalScope { /// Evaluate a JS script in this global. pub fn evaluate_js(&self, script: &str) -> bool { - debug!("Evaluating Dom."); + debug!("Evaluating Dom in a worklet."); rooted!(in (*self.globalscope.get_cx()) let mut rval = UndefinedValue()); - self.globalscope - .evaluate_js_on_global_with_result(&*script, rval.handle_mut()) + self.globalscope.evaluate_js_on_global_with_result( + &*script, + rval.handle_mut(), + ScriptFetchOptions::default_classic_script(&self.globalscope), + self.globalscope.api_base_url(), + ) } /// Register a paint worklet to the script thread. diff --git a/components/script/script_module.rs b/components/script/script_module.rs index 595415e8713..8e04fd6622a 100644 --- a/components/script/script_module.rs +++ b/components/script/script_module.rs @@ -111,10 +111,24 @@ impl Clone for RethrowError { } } -struct ModuleScript { +pub(crate) struct ModuleScript { base_url: ServoUrl, options: ScriptFetchOptions, - owner: ModuleOwner, + owner: Option, +} + +impl ModuleScript { + pub fn new( + base_url: ServoUrl, + options: ScriptFetchOptions, + owner: Option, + ) -> Self { + ModuleScript { + base_url, + options, + owner, + } + } } /// Identity for a module which will be @@ -312,15 +326,21 @@ impl ModuleTree { // We just leverage the power of Promise to run the task for `finish` the owner. // Thus, we will always `resolve` it and no need to register a callback for `reject` - fn append_handler(&self, owner: ModuleOwner, module_identity: ModuleIdentity) { + fn append_handler( + &self, + owner: ModuleOwner, + module_identity: ModuleIdentity, + fetch_options: ScriptFetchOptions, + ) { let this = owner.clone(); let identity = module_identity.clone(); + let options = fetch_options.clone(); let handler = PromiseNativeHandler::new( &owner.global(), Some(ModuleHandler::new(Box::new( task!(fetched_resolve: move || { - this.notify_owner_to_finish(identity.clone()); + this.notify_owner_to_finish(identity, options); }), ))), None, @@ -429,11 +449,7 @@ impl ModuleTree { )))); } - let module_script_data = Box::new(ModuleScript { - base_url: url.clone(), - options, - owner, - }); + let module_script_data = Box::new(ModuleScript::new(url.clone(), options, Some(owner))); SetModulePrivate( module_script.get(), @@ -885,7 +901,11 @@ impl ModuleOwner { } } - pub fn notify_owner_to_finish(&self, module_identity: ModuleIdentity) { + pub fn notify_owner_to_finish( + &self, + module_identity: ModuleIdentity, + fetch_options: ScriptFetchOptions, + ) { match &self { ModuleOwner::Worker(_) => unimplemented!(), ModuleOwner::DynamicModule(_) => unimplemented!(), @@ -904,11 +924,13 @@ impl ModuleOwner { ModuleIdentity::ModuleUrl(script_src) => Ok(ScriptOrigin::external( Rc::clone(&module_tree.get_text().borrow()), script_src.clone(), + fetch_options, ScriptType::Module, )), ModuleIdentity::ScriptId(_) => Ok(ScriptOrigin::internal( Rc::clone(&module_tree.get_text().borrow()), document.base_url().clone(), + fetch_options, ScriptType::Module, )), }, @@ -1106,6 +1128,7 @@ impl FetchResponseListener for ModuleContext { Ok(ScriptOrigin::external( Rc::new(DOMString::from(source_text)), meta.final_url, + self.options.clone(), ScriptType::Module, )) }); @@ -1239,9 +1262,9 @@ pub unsafe extern "C" fn host_import_module_dynamically( true } -#[derive(Clone)] +#[derive(Clone, JSTraceable, MallocSizeOf)] /// -pub(crate) struct ScriptFetchOptions { +pub struct ScriptFetchOptions { pub referrer: Referrer, pub integrity_metadata: String, pub credentials_mode: CredentialsMode, @@ -1252,7 +1275,7 @@ pub(crate) struct ScriptFetchOptions { impl ScriptFetchOptions { /// - fn default_classic_script(global: &GlobalScope) -> ScriptFetchOptions { + pub fn default_classic_script(global: &GlobalScope) -> ScriptFetchOptions { Self { cryptographic_nonce: String::new(), integrity_metadata: String::new(), @@ -1310,8 +1333,8 @@ fn fetch_an_import_module_script_graph( // Step 3. let owner = match unsafe { module_script_from_reference_private(&reference_private) } { - Some(module_data) => module_data.owner.clone(), - None => ModuleOwner::DynamicModule(Trusted::new(&DynamicModuleOwner::new( + Some(module_data) if module_data.owner.is_some() => module_data.owner.clone().unwrap(), + _ => ModuleOwner::DynamicModule(Trusted::new(&DynamicModuleOwner::new( global, promise.clone(), dynamic_module_id.clone(), @@ -1529,8 +1552,11 @@ fn fetch_single_module_script( ModuleIdentity::ModuleUrl(url.clone()), module, ), - None if top_level_module_fetch => module_tree - .append_handler(owner.clone(), ModuleIdentity::ModuleUrl(url.clone())), + None if top_level_module_fetch => module_tree.append_handler( + owner.clone(), + ModuleIdentity::ModuleUrl(url.clone()), + options, + ), // do nothing if it's neither a dynamic module nor a top level module None => {}, } @@ -1566,9 +1592,11 @@ fn fetch_single_module_script( ModuleIdentity::ModuleUrl(url.clone()), module, ), - None if top_level_module_fetch => { - module_tree.append_handler(owner.clone(), ModuleIdentity::ModuleUrl(url.clone())) - }, + None if top_level_module_fetch => module_tree.append_handler( + owner.clone(), + ModuleIdentity::ModuleUrl(url.clone()), + options.clone(), + ), // do nothing if it's neither a dynamic module nor a top level module None => {}, } @@ -1670,7 +1698,11 @@ pub(crate) fn fetch_inline_module_script( match compiled_module { Ok(record) => { - module_tree.append_handler(owner.clone(), ModuleIdentity::ScriptId(script_id.clone())); + module_tree.append_handler( + owner.clone(), + ModuleIdentity::ScriptId(script_id.clone()), + options.clone(), + ); module_tree.set_record(record); // We need to set `module_tree` into inline module map in case @@ -1695,7 +1727,7 @@ pub(crate) fn fetch_inline_module_script( module_tree.set_rethrow_error(exception); module_tree.set_status(ModuleStatus::Finished); global.set_inline_module_map(script_id.clone(), module_tree); - owner.notify_owner_to_finish(ModuleIdentity::ScriptId(script_id)); + owner.notify_owner_to_finish(ModuleIdentity::ScriptId(script_id), options); }, } } diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index dda8699b9f2..d2ddaebd747 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -63,6 +63,7 @@ use crate::dom::workletglobalscope::WorkletGlobalScopeInit; use crate::fetch::FetchCanceller; use crate::microtask::{Microtask, MicrotaskQueue}; use crate::realms::enter_realm; +use crate::script_module::ScriptFetchOptions; use crate::script_runtime::{ get_reports, new_rt_and_cx, ContextForRequestInterrupt, JSContext, Runtime, ScriptPort, }; @@ -3702,7 +3703,12 @@ impl ScriptThread { // Script source is ready to be evaluated (11.) let _ac = enter_realm(global_scope); rooted!(in(*global_scope.get_cx()) let mut jsval = UndefinedValue()); - global_scope.evaluate_js_on_global_with_result(&script_source, jsval.handle_mut()); + global_scope.evaluate_js_on_global_with_result( + &script_source, + jsval.handle_mut(), + ScriptFetchOptions::default_classic_script(&global_scope), + global_scope.api_base_url(), + ); load_data.js_eval_result = if jsval.get().is_string() { unsafe { diff --git a/components/script/timers.rs b/components/script/timers.rs index 5e236f07494..ebc0bd8874c 100644 --- a/components/script/timers.rs +++ b/components/script/timers.rs @@ -12,6 +12,7 @@ use crate::dom::eventsource::EventSourceTimeoutCallback; use crate::dom::globalscope::GlobalScope; use crate::dom::testbinding::TestBindingCallback; use crate::dom::xmlhttprequest::XHRTimeoutCallback; +use crate::script_module::ScriptFetchOptions; use crate::script_thread::ScriptThread; use euclid::Length; use ipc_channel::ipc::IpcSender; @@ -541,7 +542,13 @@ impl JsTimerTask { let global = this.global(); let cx = global.get_cx(); rooted!(in(*cx) let mut rval = UndefinedValue()); - global.evaluate_js_on_global_with_result(code_str, rval.handle_mut()); + // FIXME(cybai): Use base url properly by saving private reference for timers (#27260) + global.evaluate_js_on_global_with_result( + code_str, + rval.handle_mut(), + ScriptFetchOptions::default_classic_script(&global), + global.api_base_url(), + ); }, InternalTimerCallback::FunctionTimerCallback(ref function, ref arguments) => { let arguments = self.collect_heap_args(arguments); diff --git a/components/script/webdriver_handlers.rs b/components/script/webdriver_handlers.rs index 1e094ec086c..1d36337f5eb 100644 --- a/components/script/webdriver_handlers.rs +++ b/components/script/webdriver_handlers.rs @@ -38,6 +38,7 @@ use crate::dom::nodelist::NodeList; use crate::dom::window::Window; use crate::dom::xmlserializer::XMLSerializer; use crate::realms::enter_realm; +use crate::script_module::ScriptFetchOptions; use crate::script_runtime::JSContext as SafeJSContext; use crate::script_thread::{Documents, ScriptThread}; use cookie::Cookie; @@ -297,9 +298,13 @@ pub fn handle_execute_script( let result = unsafe { let cx = window.get_cx(); rooted!(in(*cx) let mut rval = UndefinedValue()); - window - .upcast::() - .evaluate_js_on_global_with_result(&eval, rval.handle_mut()); + let global = window.upcast::(); + global.evaluate_js_on_global_with_result( + &eval, + rval.handle_mut(), + ScriptFetchOptions::default_classic_script(&global), + global.api_base_url(), + ); jsval_to_webdriver(*cx, &window.upcast::(), rval.handle()) }; @@ -323,9 +328,13 @@ pub fn handle_execute_async_script( let cx = window.get_cx(); window.set_webdriver_script_chan(Some(reply)); rooted!(in(*cx) let mut rval = UndefinedValue()); - window - .upcast::() - .evaluate_js_on_global_with_result(&eval, rval.handle_mut()); + let global = window.upcast::(); + global.evaluate_js_on_global_with_result( + &eval, + rval.handle_mut(), + ScriptFetchOptions::default_classic_script(&global), + global.api_base_url(), + ); }, None => { reply