Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Root nodes for the duration of their CSS transitions #16295

Merged
merged 5 commits into from May 15, 2017
Prev

Make methods storing layout-originating nodes unsafe.

  • Loading branch information
jdm committed May 15, 2017
commit b0bf2b4bad636acfba66d55571b417ebae795408
@@ -826,6 +826,7 @@ impl Document {
}
}

#[allow(unsafe_code)]
pub fn handle_mouse_event(&self,
js_runtime: *mut JSRuntime,
button: MouseButton,
@@ -841,7 +842,9 @@ impl Document {
let node = match self.window.hit_test_query(client_point, false) {
Some(node_address) => {
debug!("node address is {:?}", node_address);
node::from_untrusted_node_address(js_runtime, node_address)
unsafe {
node::from_untrusted_node_address(js_runtime, node_address)
}
},
None => return,
};
@@ -988,13 +991,16 @@ impl Document {
*self.last_click_info.borrow_mut() = Some((now, click_pos));
}

#[allow(unsafe_code)]
pub fn handle_touchpad_pressure_event(&self,
js_runtime: *mut JSRuntime,
client_point: Point2D<f32>,
pressure: f32,
phase_now: TouchpadPressurePhase) {
let node = match self.window.hit_test_query(client_point, false) {
Some(node_address) => node::from_untrusted_node_address(js_runtime, node_address),
Some(node_address) => unsafe {
node::from_untrusted_node_address(js_runtime, node_address)
},
None => return
};

@@ -1089,6 +1095,7 @@ impl Document {
event.fire(target);
}

#[allow(unsafe_code)]
pub fn handle_mouse_move_event(&self,
js_runtime: *mut JSRuntime,
client_point: Option<Point2D<f32>>,
@@ -1104,7 +1111,7 @@ impl Document {
};

let maybe_new_target = self.window.hit_test_query(client_point, true).and_then(|address| {
let node = node::from_untrusted_node_address(js_runtime, address);
let node = unsafe { node::from_untrusted_node_address(js_runtime, address) };
node.inclusive_ancestors()
.filter_map(Root::downcast::<Element>)
.next()
@@ -1186,6 +1193,7 @@ impl Document {
ReflowReason::MouseEvent);
}

#[allow(unsafe_code)]
pub fn handle_touch_event(&self,
js_runtime: *mut JSRuntime,
event_type: TouchEventType,
@@ -1202,7 +1210,9 @@ impl Document {
};

let node = match self.window.hit_test_query(point, false) {
Some(node_address) => node::from_untrusted_node_address(js_runtime, node_address),
Some(node_address) => unsafe {
node::from_untrusted_node_address(js_runtime, node_address)
},
None => return TouchEventResult::Processed(false),
};
let el = match node.downcast::<Element>() {
@@ -3480,7 +3490,9 @@ impl DocumentMethods for Document {
Some(untrusted_node_address) => {
let js_runtime = unsafe { JS_GetRuntime(window.get_cx()) };

let node = node::from_untrusted_node_address(js_runtime, untrusted_node_address);
let node = unsafe {
node::from_untrusted_node_address(js_runtime, untrusted_node_address)
};
let parent_node = node.GetParentNode().unwrap();
let element_ref = node.downcast::<Element>().unwrap_or_else(|| {
parent_node.downcast::<Element>().unwrap()
@@ -3515,7 +3527,9 @@ impl DocumentMethods for Document {
// Step 1 and Step 3
let mut elements: Vec<Root<Element>> = self.nodes_from_point(point).iter()
.flat_map(|&untrusted_node_address| {
let node = node::from_untrusted_node_address(js_runtime, untrusted_node_address);
let node = unsafe {
node::from_untrusted_node_address(js_runtime, untrusted_node_address)
};
Root::downcast::<Element>(node)
}).collect();

@@ -927,20 +927,18 @@ fn first_node_not_in<I>(mut nodes: I, not_in: &[NodeOrString]) -> Option<Root<No
/// If the given untrusted node address represents a valid DOM node in the given runtime,
/// returns it.
#[allow(unsafe_code)]
pub fn from_untrusted_node_address(_runtime: *mut JSRuntime, candidate: UntrustedNodeAddress)
pub unsafe fn from_untrusted_node_address(_runtime: *mut JSRuntime, candidate: UntrustedNodeAddress)
-> Root<Node> {
unsafe {
// https://github.com/servo/servo/issues/6383
let candidate: uintptr_t = mem::transmute(candidate.0);
// https://github.com/servo/servo/issues/6383
let candidate: uintptr_t = mem::transmute(candidate.0);

This comment has been minimized.

Copy link
@nox

nox May 15, 2017

Member

Please don't use transmute.

// let object: *mut JSObject = jsfriendapi::bindgen::JS_GetAddressableObject(runtime,
// candidate);
let object: *mut JSObject = mem::transmute(candidate);
if object.is_null() {
panic!("Attempted to create a `JS<Node>` from an invalid pointer!")
}
let boxed_node = conversions::private_from_object(object) as *const Node;
Root::from_ref(&*boxed_node)
let object: *mut JSObject = mem::transmute(candidate);

This comment has been minimized.

Copy link
@nox

nox May 15, 2017

Member

Please don't use transmute.

if object.is_null() {
panic!("Attempted to create a `JS<Node>` from an invalid pointer!")
}
let boxed_node = conversions::private_from_object(object) as *const Node;
Root::from_ref(&*boxed_node)
}

#[allow(unsafe_code)]
@@ -1241,7 +1241,7 @@ impl Window {
let id = image.id;
let js_runtime = self.js_runtime.borrow();
let js_runtime = js_runtime.as_ref().unwrap();
let node = from_untrusted_node_address(js_runtime.rt(), image.node);
let node = unsafe { from_untrusted_node_address(js_runtime.rt(), image.node) };

if let PendingImageState::Unrequested(ref url) = image.state {
fetch_image_for_layout(url.clone(), &*node, id, self.image_cache.clone());
@@ -1261,7 +1261,9 @@ impl Window {
}
}

ScriptThread::note_newly_transitioning_nodes(complete.newly_transitioning_nodes);
unsafe {
ScriptThread::note_newly_transitioning_nodes(complete.newly_transitioning_nodes);
}

true
}
@@ -1457,6 +1459,7 @@ impl Window {
DOMString::from(resolved)
}

#[allow(unsafe_code)]
pub fn offset_parent_query(&self, node: TrustedNodeAddress) -> (Option<Root<Element>>, Rect<Au>) {
if !self.reflow(ReflowGoal::ForScriptQuery,
ReflowQueryType::OffsetParentQuery(node),
@@ -1468,7 +1471,7 @@ impl Window {
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 = from_untrusted_node_address(js_runtime.rt(), parent_node_address);
let node = unsafe { from_untrusted_node_address(js_runtime.rt(), parent_node_address) };
Root::downcast(node)
});
(element, response.rect)
@@ -576,9 +576,9 @@ impl ScriptThreadFactory for ScriptThread {
}

impl ScriptThread {
pub fn note_newly_transitioning_nodes(nodes: Vec<UntrustedNodeAddress>) {
pub unsafe fn note_newly_transitioning_nodes(nodes: Vec<UntrustedNodeAddress>) {
SCRIPT_THREAD_ROOT.with(|root| {
let script_thread = unsafe { &*root.get().unwrap() };
let script_thread = &*root.get().unwrap();
let js_runtime = script_thread.js_runtime.rt();
let new_nodes = nodes
.into_iter()
@@ -1619,7 +1619,9 @@ impl ScriptThread {
/// Handles firing of transition events.
fn handle_transition_event(&self, unsafe_node: UntrustedNodeAddress, name: String, duration: f64) {

This comment has been minimized.

Copy link
@nox

nox Apr 11, 2017

Member

This should be unsafe.

let js_runtime = self.js_runtime.rt();
let node = from_untrusted_node_address(js_runtime, unsafe_node);
let node = unsafe {
from_untrusted_node_address(js_runtime, unsafe_node)
};

let idx = self.transitioning_nodes
.borrow()
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.