From 0854953e3556723202a2d622eddc5d08ee46aaf0 Mon Sep 17 00:00:00 2001 From: teymour-aldridge Date: Sun, 31 Jan 2021 21:51:06 +0000 Subject: [PATCH 1/4] Mark methods as unsafe. --- components/script/dom/document.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 8ff05e67849..cee460d4fa3 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -1185,7 +1185,7 @@ impl Document { } #[allow(unsafe_code)] - pub fn handle_mouse_event( + pub unsafe fn handle_mouse_event( &self, js_runtime: *mut JSRuntime, button: MouseButton, @@ -1389,7 +1389,7 @@ impl Document { } #[allow(unsafe_code)] - pub fn handle_mouse_move_event( + pub unsafe fn handle_mouse_move_event( &self, js_runtime: *mut JSRuntime, client_point: Point2D, @@ -1562,7 +1562,7 @@ impl Document { } #[allow(unsafe_code)] - pub fn handle_wheel_event( + pub unsafe fn handle_wheel_event( &self, js_runtime: *mut JSRuntime, delta: WheelDelta, @@ -1609,7 +1609,7 @@ impl Document { } #[allow(unsafe_code)] - pub fn handle_touch_event( + pub unsafe fn handle_touch_event( &self, js_runtime: *mut JSRuntime, event_type: TouchEventType, From 466e895cdfc0cf909f49db8e4d2eb4958ad0101d Mon Sep 17 00:00:00 2001 From: teymour-aldridge Date: Mon, 1 Feb 2021 11:26:17 +0000 Subject: [PATCH 2/4] Fix errors --- components/script/dom/document.rs | 8 ++--- components/script/script_thread.rs | 56 +++++++++++++++++------------- 2 files changed, 36 insertions(+), 28 deletions(-) diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index cee460d4fa3..5dec5ff75f9 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -1203,7 +1203,7 @@ impl Document { debug!("{}: at {:?}", mouse_event_type_string, client_point); let el = node_address.and_then(|address| { - let node = unsafe { node::from_untrusted_node_address(js_runtime, address) }; + let node = node::from_untrusted_node_address(js_runtime, address); node.inclusive_ancestors(ShadowIncluding::No) .filter_map(DomRoot::downcast::) .next() @@ -1398,7 +1398,7 @@ impl Document { pressed_mouse_buttons: u16, ) { let maybe_new_target = node_address.and_then(|address| { - let node = unsafe { node::from_untrusted_node_address(js_runtime, address) }; + let node = node::from_untrusted_node_address(js_runtime, address); node.inclusive_ancestors(ShadowIncluding::No) .filter_map(DomRoot::downcast::) .next() @@ -1573,7 +1573,7 @@ impl Document { debug!("{}: at {:?}", wheel_event_type_string, client_point); let el = node_address.and_then(|address| { - let node = unsafe { node::from_untrusted_node_address(js_runtime, address) }; + let node = node::from_untrusted_node_address(js_runtime, address); node.inclusive_ancestors(ShadowIncluding::No) .filter_map(DomRoot::downcast::) .next() @@ -1627,7 +1627,7 @@ impl Document { }; let el = node_address.and_then(|address| { - let node = unsafe { node::from_untrusted_node_address(js_runtime, address) }; + let node = node::from_untrusted_node_address(js_runtime, address); node.inclusive_ancestors(ShadowIncluding::No) .filter_map(DomRoot::downcast::) .next() diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index 68bbc524764..68fef98b091 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -3525,13 +3525,15 @@ impl ScriptThread { // Get the previous target temporarily let prev_mouse_over_target = self.topmost_mouse_over_target.get(); - document.handle_mouse_move_event( - self.js_runtime.rt(), - point, - &self.topmost_mouse_over_target, - node_address, - pressed_mouse_buttons, - ); + unsafe { + document.handle_mouse_move_event( + self.js_runtime.rt(), + point, + &self.topmost_mouse_over_target, + node_address, + pressed_mouse_buttons, + ) + } // Short-circuit if nothing changed if self.topmost_mouse_over_target.get() == prev_mouse_over_target { @@ -3650,15 +3652,17 @@ impl ScriptThread { Some(document) => document, None => return warn!("Message sent to closed pipeline {}.", pipeline_id), }; - document.handle_mouse_event( - self.js_runtime.rt(), - button, - point, - mouse_event_type, - node_address, - point_in_node, - pressed_mouse_buttons, - ); + unsafe { + document.handle_mouse_event( + self.js_runtime.rt(), + button, + point, + mouse_event_type, + node_address, + point_in_node, + pressed_mouse_buttons, + ) + } } fn handle_touch_event( @@ -3676,13 +3680,15 @@ impl ScriptThread { return TouchEventResult::Processed(true); }, }; - document.handle_touch_event( - self.js_runtime.rt(), - event_type, - identifier, - point, - node_address, - ) + unsafe { + document.handle_touch_event( + self.js_runtime.rt(), + event_type, + identifier, + point, + node_address, + ) + } } fn handle_wheel_event( @@ -3696,7 +3702,9 @@ impl ScriptThread { Some(document) => document, None => return warn!("Message sent to closed pipeline {}.", pipeline_id), }; - document.handle_wheel_event(self.js_runtime.rt(), wheel_delta, point, node_address); + unsafe { + document.handle_wheel_event(self.js_runtime.rt(), wheel_delta, point, node_address) + }; } /// Handle a "navigate an iframe" message from the constellation. From ff767b13a93c0ca50fbe7e029ef57f0fee960a8d Mon Sep 17 00:00:00 2001 From: teymour-aldridge Date: Fri, 5 Feb 2021 14:25:02 +0000 Subject: [PATCH 3/4] Remove unused arguments from methods. --- components/script/animations.rs | 6 ++---- components/script/dom/document.rs | 14 +++++--------- components/script/dom/documentorshadowroot.rs | 8 ++------ components/script/dom/node.rs | 3 +-- components/script/dom/window.rs | 8 ++------ components/script/script_thread.rs | 5 +---- 6 files changed, 13 insertions(+), 31 deletions(-) diff --git a/components/script/animations.rs b/components/script/animations.rs index 25aad491f6d..cd581ed3cb3 100644 --- a/components/script/animations.rs +++ b/components/script/animations.rs @@ -125,7 +125,7 @@ impl Animations { pub(crate) fn do_post_reflow_update(&self, window: &Window, now: f64) { let pipeline_id = window.pipeline_id(); let mut sets = self.sets.sets.write(); - self.root_newly_animating_dom_nodes(&sets, window); + self.root_newly_animating_dom_nodes(&sets); for (key, set) in sets.iter_mut() { self.handle_canceled_animations(key, set, now, pipeline_id); @@ -305,9 +305,7 @@ impl Animations { fn root_newly_animating_dom_nodes( &self, sets: &FxHashMap, - window: &Window, ) { - let js_runtime = window.get_js_runtime().as_ref().unwrap().rt(); let mut rooted_nodes = self.rooted_nodes.borrow_mut(); for (key, set) in sets.iter() { let opaque_node = key.node; @@ -322,7 +320,7 @@ impl Animations { unsafe { rooted_nodes.insert( opaque_node, - Dom::from_ref(&*from_untrusted_node_address(js_runtime, address)), + Dom::from_ref(&*from_untrusted_node_address(address)), ) }; } diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 5dec5ff75f9..b2965c8832e 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -122,7 +122,7 @@ use euclid::default::{Point2D, Rect, Size2D}; use html5ever::{LocalName, Namespace, QualName}; use hyper_serde::Serde; use ipc_channel::ipc::{self, IpcSender}; -use js::jsapi::{JSObject, JSRuntime}; +use js::jsapi::{JSObject}; use keyboard_types::{Code, Key, KeyState}; use metrics::{ InteractiveFlag, InteractiveMetrics, InteractiveWindow, ProfilerMetadataFactory, @@ -1187,7 +1187,6 @@ impl Document { #[allow(unsafe_code)] pub unsafe fn handle_mouse_event( &self, - js_runtime: *mut JSRuntime, button: MouseButton, client_point: Point2D, mouse_event_type: MouseEventType, @@ -1203,7 +1202,7 @@ impl Document { debug!("{}: at {:?}", mouse_event_type_string, client_point); let el = node_address.and_then(|address| { - let node = node::from_untrusted_node_address(js_runtime, address); + let node = node::from_untrusted_node_address(address); node.inclusive_ancestors(ShadowIncluding::No) .filter_map(DomRoot::downcast::) .next() @@ -1391,14 +1390,13 @@ impl Document { #[allow(unsafe_code)] pub unsafe fn handle_mouse_move_event( &self, - js_runtime: *mut JSRuntime, client_point: Point2D, prev_mouse_over_target: &MutNullableDom, node_address: Option, pressed_mouse_buttons: u16, ) { let maybe_new_target = node_address.and_then(|address| { - let node = node::from_untrusted_node_address(js_runtime, address); + let node = node::from_untrusted_node_address(address); node.inclusive_ancestors(ShadowIncluding::No) .filter_map(DomRoot::downcast::) .next() @@ -1564,7 +1562,6 @@ impl Document { #[allow(unsafe_code)] pub unsafe fn handle_wheel_event( &self, - js_runtime: *mut JSRuntime, delta: WheelDelta, client_point: Point2D, node_address: Option, @@ -1573,7 +1570,7 @@ impl Document { debug!("{}: at {:?}", wheel_event_type_string, client_point); let el = node_address.and_then(|address| { - let node = node::from_untrusted_node_address(js_runtime, address); + let node = node::from_untrusted_node_address(address); node.inclusive_ancestors(ShadowIncluding::No) .filter_map(DomRoot::downcast::) .next() @@ -1611,7 +1608,6 @@ impl Document { #[allow(unsafe_code)] pub unsafe fn handle_touch_event( &self, - js_runtime: *mut JSRuntime, event_type: TouchEventType, touch_id: TouchId, point: Point2D, @@ -1627,7 +1623,7 @@ impl Document { }; let el = node_address.and_then(|address| { - let node = node::from_untrusted_node_address(js_runtime, address); + let node = node::from_untrusted_node_address(address); node.inclusive_ancestors(ShadowIncluding::No) .filter_map(DomRoot::downcast::) .next() diff --git a/components/script/dom/documentorshadowroot.rs b/components/script/dom/documentorshadowroot.rs index 5469287d87c..87322db2574 100644 --- a/components/script/dom/documentorshadowroot.rs +++ b/components/script/dom/documentorshadowroot.rs @@ -14,7 +14,6 @@ use crate::dom::node::{self, Node, VecPreOrderInsertionHelper}; use crate::dom::window::Window; use crate::stylesheet_set::StylesheetSetRef; use euclid::default::Point2D; -use js::jsapi::JS_GetRuntime; use script_layout_interface::message::{NodesFromPointQueryType, QueryMsg}; use script_traits::UntrustedNodeAddress; use servo_arc::Arc; @@ -131,8 +130,7 @@ impl DocumentOrShadowRoot { .first() { Some(address) => { - let js_runtime = unsafe { JS_GetRuntime(*self.window.get_cx()) }; - let node = unsafe { node::from_untrusted_node_address(js_runtime, *address) }; + let node = unsafe { node::from_untrusted_node_address(*address) }; let parent_node = node.GetParentNode().unwrap(); let element_ref = node .downcast::() @@ -167,15 +165,13 @@ impl DocumentOrShadowRoot { return vec![]; } - let js_runtime = unsafe { JS_GetRuntime(*self.window.get_cx()) }; - // Step 1 and Step 3 let nodes = self.nodes_from_point(point, NodesFromPointQueryType::All); let mut elements: Vec> = nodes .iter() .flat_map(|&untrusted_node_address| { let node = unsafe { - node::from_untrusted_node_address(js_runtime, untrusted_node_address) + node::from_untrusted_node_address(untrusted_node_address) }; DomRoot::downcast::(node) }) diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index e5ebc92e651..8382d4b4145 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -69,7 +69,7 @@ use devtools_traits::NodeInfo; use dom_struct::dom_struct; use euclid::default::{Point2D, Rect, Size2D, Vector2D}; use html5ever::{Namespace, Prefix, QualName}; -use js::jsapi::{JSObject, JSRuntime}; +use js::jsapi::{JSObject}; use libc::{self, c_void, uintptr_t}; use malloc_size_of::{MallocSizeOf, MallocSizeOfOps}; use msg::constellation_msg::{BrowsingContextId, PipelineId}; @@ -1322,7 +1322,6 @@ where /// returns it. #[allow(unsafe_code)] pub unsafe fn from_untrusted_node_address( - _runtime: *mut JSRuntime, candidate: UntrustedNodeAddress, ) -> DomRoot { // https://github.com/servo/servo/issues/6383 diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index 70734efb9bb..71ceb293664 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -1755,9 +1755,7 @@ impl Window { for image in complete.pending_images { let id = image.id; - let js_runtime = self.js_runtime.borrow(); - let js_runtime = js_runtime.as_ref().unwrap(); - let node = unsafe { from_untrusted_node_address(js_runtime.rt(), image.node) }; + let node = unsafe { from_untrusted_node_address(image.node) }; if let PendingImageState::Unrequested(ref url) = image.state { fetch_image_for_layout(url.clone(), &*node, id, self.image_cache.clone()); @@ -1988,10 +1986,8 @@ impl Window { // FIXME(nox): Layout can reply with a garbage value which doesn't // actually correspond to an element, that's unsound. let response = self.layout_rpc.offset_parent(); - let js_runtime = self.js_runtime.borrow(); - let js_runtime = js_runtime.as_ref().unwrap(); let element = response.node_address.and_then(|parent_node_address| { - let node = unsafe { from_untrusted_node_address(js_runtime.rt(), parent_node_address) }; + let node = unsafe { from_untrusted_node_address(parent_node_address) }; DomRoot::downcast(node) }); (element, response.rect) diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index 68fef98b091..9f976901d41 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -3527,7 +3527,6 @@ impl ScriptThread { unsafe { document.handle_mouse_move_event( - self.js_runtime.rt(), point, &self.topmost_mouse_over_target, node_address, @@ -3654,7 +3653,6 @@ impl ScriptThread { }; unsafe { document.handle_mouse_event( - self.js_runtime.rt(), button, point, mouse_event_type, @@ -3682,7 +3680,6 @@ impl ScriptThread { }; unsafe { document.handle_touch_event( - self.js_runtime.rt(), event_type, identifier, point, @@ -3703,7 +3700,7 @@ impl ScriptThread { None => return warn!("Message sent to closed pipeline {}.", pipeline_id), }; unsafe { - document.handle_wheel_event(self.js_runtime.rt(), wheel_delta, point, node_address) + document.handle_wheel_event(wheel_delta, point, node_address) }; } From 217147b2e41b9e15e04fb2935882a478c616bd82 Mon Sep 17 00:00:00 2001 From: teymour-aldridge Date: Fri, 5 Feb 2021 14:46:09 +0000 Subject: [PATCH 4/4] fmt --- components/script/dom/document.rs | 2 +- components/script/dom/documentorshadowroot.rs | 4 +--- components/script/dom/node.rs | 6 ++---- components/script/script_thread.rs | 13 ++----------- 4 files changed, 6 insertions(+), 19 deletions(-) diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index b2965c8832e..eccc14f67ed 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -122,7 +122,7 @@ use euclid::default::{Point2D, Rect, Size2D}; use html5ever::{LocalName, Namespace, QualName}; use hyper_serde::Serde; use ipc_channel::ipc::{self, IpcSender}; -use js::jsapi::{JSObject}; +use js::jsapi::JSObject; use keyboard_types::{Code, Key, KeyState}; use metrics::{ InteractiveFlag, InteractiveMetrics, InteractiveWindow, ProfilerMetadataFactory, diff --git a/components/script/dom/documentorshadowroot.rs b/components/script/dom/documentorshadowroot.rs index 87322db2574..43d345e5e01 100644 --- a/components/script/dom/documentorshadowroot.rs +++ b/components/script/dom/documentorshadowroot.rs @@ -170,9 +170,7 @@ impl DocumentOrShadowRoot { let mut elements: Vec> = nodes .iter() .flat_map(|&untrusted_node_address| { - let node = unsafe { - node::from_untrusted_node_address(untrusted_node_address) - }; + let node = unsafe { node::from_untrusted_node_address(untrusted_node_address) }; DomRoot::downcast::(node) }) .collect(); diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index 8382d4b4145..3ad3e656518 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -69,7 +69,7 @@ use devtools_traits::NodeInfo; use dom_struct::dom_struct; use euclid::default::{Point2D, Rect, Size2D, Vector2D}; use html5ever::{Namespace, Prefix, QualName}; -use js::jsapi::{JSObject}; +use js::jsapi::JSObject; use libc::{self, c_void, uintptr_t}; use malloc_size_of::{MallocSizeOf, MallocSizeOfOps}; use msg::constellation_msg::{BrowsingContextId, PipelineId}; @@ -1321,9 +1321,7 @@ where /// If the given untrusted node address represents a valid DOM node in the given runtime, /// returns it. #[allow(unsafe_code)] -pub unsafe fn from_untrusted_node_address( - candidate: UntrustedNodeAddress, -) -> DomRoot { +pub unsafe fn from_untrusted_node_address(candidate: UntrustedNodeAddress) -> DomRoot { // https://github.com/servo/servo/issues/6383 let candidate: uintptr_t = mem::transmute(candidate.0); // let object: *mut JSObject = jsfriendapi::bindgen::JS_GetAddressableObject(runtime, diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index 9f976901d41..1b28c337d56 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -3678,14 +3678,7 @@ impl ScriptThread { return TouchEventResult::Processed(true); }, }; - unsafe { - document.handle_touch_event( - event_type, - identifier, - point, - node_address, - ) - } + unsafe { document.handle_touch_event(event_type, identifier, point, node_address) } } fn handle_wheel_event( @@ -3699,9 +3692,7 @@ impl ScriptThread { Some(document) => document, None => return warn!("Message sent to closed pipeline {}.", pipeline_id), }; - unsafe { - document.handle_wheel_event(wheel_delta, point, node_address) - }; + unsafe { document.handle_wheel_event(wheel_delta, point, node_address) }; } /// Handle a "navigate an iframe" message from the constellation.