servo: Merge #14589 - Reduce calls into arbitrary code with the ScriptThread::documents borrow held (from servo:harden-script); r=jdm+Ms2ger

Source-Repo: https://github.com/servo/servo
Source-Revision: 0c56499bdc5c92bac44f8c33e4781899c34d64e1
This commit is contained in:
Josh Matthews 2016-12-16 08:53:27 -08:00
parent 8dd3b1c5c8
commit bf743e57d6
4 changed files with 97 additions and 52 deletions

View File

@ -1142,7 +1142,8 @@ impl ScriptThread {
}
fn handle_resize(&self, id: PipelineId, size: WindowSizeData, size_type: WindowSizeType) {
if let Some(ref window) = self.documents.borrow().find_window(id) {
let window = self.documents.borrow().find_window(id);
if let Some(ref window) = window {
window.set_resize_event(size, size_type);
return;
}
@ -1155,7 +1156,8 @@ impl ScriptThread {
}
fn handle_viewport(&self, id: PipelineId, rect: Rect<f32>) {
if let Some(document) = self.documents.borrow().find_document(id) {
let document = self.documents.borrow().find_document(id);
if let Some(document) = document {
if document.window().set_page_clip_rect_with_new_viewport(rect) {
self.rebuild_and_force_reflow(&document, ReflowReason::Viewport);
}
@ -1172,7 +1174,7 @@ impl ScriptThread {
fn handle_set_scroll_state(&self,
id: PipelineId,
scroll_states: &[(UntrustedNodeAddress, Point2D<f32>)]) {
let window = match self.documents.borrow().find_window(id) {
let window = match { self.documents.borrow().find_window(id) } {
Some(window) => window,
None => return warn!("Set scroll state message sent to nonexistent pipeline: {:?}", id),
};
@ -1240,7 +1242,7 @@ impl ScriptThread {
}
fn handle_loads_complete(&self, pipeline: PipelineId) {
let doc = match self.documents.borrow().find_document(pipeline) {
let doc = match { self.documents.borrow().find_document(pipeline) } {
Some(doc) => doc,
None => return warn!("Message sent to closed pipeline {}.", pipeline),
};
@ -1295,7 +1297,8 @@ impl ScriptThread {
/// To slow/speed up timers and manage any other script thread resource based on visibility.
/// Returns true if successful.
fn alter_resource_utilization(&self, id: PipelineId, visible: bool) -> bool {
if let Some(window) = self.documents.borrow().find_window(id) {
let window = self.documents.borrow().find_window(id);
if let Some(window) = window {
if visible {
window.upcast::<GlobalScope>().speed_up_timers();
} else {
@ -1308,7 +1311,8 @@ impl ScriptThread {
/// Updates iframe element after a change in visibility
fn handle_visibility_change_complete_msg(&self, parent_pipeline_id: PipelineId, id: FrameId, visible: bool) {
if let Some(iframe) = self.documents.borrow().find_iframe(parent_pipeline_id, id) {
let iframe = self.documents.borrow().find_iframe(parent_pipeline_id, id);
if let Some(iframe) = iframe {
iframe.change_visibility_status(visible);
}
}
@ -1336,7 +1340,8 @@ impl ScriptThread {
/// Handles freeze message
fn handle_freeze_msg(&self, id: PipelineId) {
if let Some(window) = self.documents.borrow().find_window(id) {
let window = self.documents.borrow().find_window(id);
if let Some(window) = window {
window.upcast::<GlobalScope>().suspend();
return;
}
@ -1350,7 +1355,8 @@ impl ScriptThread {
/// Handles thaw message
fn handle_thaw_msg(&self, id: PipelineId) {
if let Some(document) = self.documents.borrow().find_document(id) {
let document = self.documents.borrow().find_document(id);
if let Some(document) = document {
if let Some(context) = document.browsing_context() {
let needed_reflow = context.set_reflow_status(false);
if needed_reflow {
@ -1401,14 +1407,16 @@ impl ScriptThread {
parent_pipeline_id: PipelineId,
frame_id: Option<FrameId>,
event: MozBrowserEvent) {
match self.documents.borrow().find_document(parent_pipeline_id) {
None => warn!("Mozbrowser event after pipeline {:?} closed.", parent_pipeline_id),
Some(doc) => match frame_id {
None => doc.window().dispatch_mozbrowser_event(event),
Some(frame_id) => match doc.find_iframe(frame_id) {
None => warn!("Mozbrowser event after iframe {:?}/{:?} closed.", parent_pipeline_id, frame_id),
Some(frame_element) => frame_element.dispatch_mozbrowser_event(event),
},
let doc = match { self.documents.borrow().find_document(parent_pipeline_id) } {
None => return warn!("Mozbrowser event after pipeline {:?} closed.", parent_pipeline_id),
Some(doc) => doc,
};
match frame_id {
None => doc.window().dispatch_mozbrowser_event(event),
Some(frame_id) => match doc.find_iframe(frame_id) {
None => warn!("Mozbrowser event after iframe {:?}/{:?} closed.", parent_pipeline_id, frame_id),
Some(frame_element) => frame_element.dispatch_mozbrowser_event(event),
},
}
}
@ -1417,7 +1425,8 @@ impl ScriptThread {
parent_pipeline_id: PipelineId,
frame_id: FrameId,
new_pipeline_id: PipelineId) {
if let Some(frame_element) = self.documents.borrow().find_iframe(parent_pipeline_id, frame_id) {
let frame_element = self.documents.borrow().find_iframe(parent_pipeline_id, frame_id);
if let Some(frame_element) = frame_element {
frame_element.update_pipeline_id(new_pipeline_id);
}
}
@ -1486,13 +1495,14 @@ impl ScriptThread {
Some(r) => r,
None => return
};
if let Some(window) = self.documents.borrow().find_window(pipeline_id) {
let script_url = maybe_registration.get_installed().get_script_url();
let scope_things = ServiceWorkerRegistration::create_scope_things(window.upcast(), script_url);
let _ = self.constellation_chan.send(ConstellationMsg::RegisterServiceWorker(scope_things, scope.clone()));
} else {
warn!("Registration failed for {}", scope);
}
let window = match { self.documents.borrow().find_window(pipeline_id) } {
Some(window) => window,
None => return warn!("Registration failed for {}", scope),
};
let script_url = maybe_registration.get_installed().get_script_url();
let scope_things = ServiceWorkerRegistration::create_scope_things(window.upcast(), script_url);
let _ = self.constellation_chan.send(ConstellationMsg::RegisterServiceWorker(scope_things, scope.clone()));
}
pub fn dispatch_job_queue(&self, job_handler: Box<AsyncJobHandler>) {
@ -1543,7 +1553,7 @@ impl ScriptThread {
/// Handles a request for the window title.
fn handle_get_title_msg(&self, pipeline_id: PipelineId) {
let document = match self.documents.borrow().find_document(pipeline_id) {
let document = match { self.documents.borrow().find_document(pipeline_id) } {
Some(document) => document,
None => return warn!("Message sent to closed pipeline {}.", pipeline_id),
};
@ -1600,7 +1610,7 @@ impl ScriptThread {
/// Handles when layout thread finishes all animation in one tick
fn handle_tick_all_animations(&self, id: PipelineId) {
let document = match self.documents.borrow().find_document(id) {
let document = match { self.documents.borrow().find_document(id) } {
Some(document) => document,
None => return warn!("Message sent to closed pipeline {}.", id),
};
@ -1641,7 +1651,8 @@ impl ScriptThread {
/// Handles a Web font being loaded. Does nothing if the page no longer exists.
fn handle_web_font_loaded(&self, pipeline_id: PipelineId) {
if let Some(document) = self.documents.borrow().find_document(pipeline_id) {
let document = self.documents.borrow().find_document(pipeline_id);
if let Some(document) = document {
self.rebuild_and_force_reflow(&document, ReflowReason::WebFontLoaded);
}
}
@ -1649,12 +1660,14 @@ impl ScriptThread {
/// Notify a window of a storage event
fn handle_storage_event(&self, pipeline_id: PipelineId, storage_type: StorageType, url: ServoUrl,
key: Option<String>, old_value: Option<String>, new_value: Option<String>) {
let storage = match self.documents.borrow().find_window(pipeline_id) {
let window = match { self.documents.borrow().find_window(pipeline_id) } {
None => return warn!("Storage event sent to closed pipeline {}.", pipeline_id),
Some(window) => match storage_type {
StorageType::Local => window.LocalStorage(),
StorageType::Session => window.SessionStorage(),
},
Some(window) => window,
};
let storage = match storage_type {
StorageType::Local => window.LocalStorage(),
StorageType::Session => window.SessionStorage(),
};
storage.queue_storage_event(url, key, old_value, new_value);
@ -1907,7 +1920,7 @@ impl ScriptThread {
}
MouseMoveEvent(point) => {
let document = match self.documents.borrow().find_document(pipeline_id) {
let document = match { self.documents.borrow().find_document(pipeline_id) } {
Some(document) => document,
None => return warn!("Message sent to closed pipeline {}.", pipeline_id),
};
@ -1979,17 +1992,19 @@ impl ScriptThread {
}
TouchpadPressureEvent(point, pressure, phase) => {
match self.documents.borrow().find_document(pipeline_id) {
Some(doc) => doc.handle_touchpad_pressure_event(self.js_runtime.rt(), point, pressure, phase),
None => warn!("Message sent to closed pipeline {}.", pipeline_id),
}
let doc = match { self.documents.borrow().find_document(pipeline_id) } {
Some(doc) => doc,
None => return warn!("Message sent to closed pipeline {}.", pipeline_id),
};
doc.handle_touchpad_pressure_event(self.js_runtime.rt(), point, pressure, phase);
}
KeyEvent(ch, key, state, modifiers) => {
match self.documents.borrow().find_document(pipeline_id) {
Some(document) => document.dispatch_key_event(ch, key, state, modifiers, &self.constellation_chan),
None => warn!("Message sent to closed pipeline {}.", pipeline_id),
}
let document = match { self.documents.borrow().find_document(pipeline_id) } {
Some(document) => document,
None => return warn!("Message sent to closed pipeline {}.", pipeline_id),
};
document.dispatch_key_event(ch, key, state, modifiers, &self.constellation_chan);
}
}
}
@ -1999,10 +2014,11 @@ impl ScriptThread {
mouse_event_type: MouseEventType,
button: MouseButton,
point: Point2D<f32>) {
match self.documents.borrow().find_document(pipeline_id) {
Some(document) => document.handle_mouse_event(self.js_runtime.rt(), button, point, mouse_event_type),
None => warn!("Message sent to closed pipeline {}.", pipeline_id),
}
let document = match { self.documents.borrow().find_document(pipeline_id) } {
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);
}
fn handle_touch_event(&self,
@ -2011,13 +2027,14 @@ impl ScriptThread {
identifier: TouchId,
point: Point2D<f32>)
-> TouchEventResult {
match self.documents.borrow().find_document(pipeline_id) {
Some(document) => document.handle_touch_event(self.js_runtime.rt(), event_type, identifier, point),
let document = match { self.documents.borrow().find_document(pipeline_id) } {
Some(document) => document,
None => {
warn!("Message sent to closed pipeline {}.", pipeline_id);
TouchEventResult::Processed(true)
return TouchEventResult::Processed(true);
},
}
};
document.handle_touch_event(self.js_runtime.rt(), event_type, identifier, point)
}
/// https://html.spec.whatwg.org/multipage/#navigating-across-documents
@ -2043,7 +2060,7 @@ impl ScriptThread {
}
fn handle_resize_event(&self, pipeline_id: PipelineId, new_size: WindowSizeData, size_type: WindowSizeType) {
let document = match self.documents.borrow().find_document(pipeline_id) {
let document = match { self.documents.borrow().find_document(pipeline_id) } {
Some(document) => document,
None => return warn!("Message sent to closed pipeline {}.", pipeline_id),
};
@ -2126,7 +2143,7 @@ impl ScriptThread {
}
fn handle_parsing_complete(&self, id: PipelineId) {
let document = match self.documents.borrow().find_document(id) {
let document = match { self.documents.borrow().find_document(id) } {
Some(document) => document,
None => return,
};
@ -2174,7 +2191,8 @@ impl ScriptThread {
}
fn handle_reload(&self, pipeline_id: PipelineId) {
if let Some(window) = self.documents.borrow().find_window(pipeline_id) {
let window = self.documents.borrow().find_window(pipeline_id);
if let Some(window) = window {
window.Location().Reload();
}
}

View File

@ -542,6 +542,9 @@ def check_rust(file_name, lines):
lambda match, line: line.startswith('use ')),
(r"^\s*else {", "else braces should be on the same line", no_filter),
(r"[^$ ]\([ \t]", "extra space after (", no_filter),
# This particular pattern is not reentrant-safe in script_thread.rs
(r"match self.documents.borrow", "use a separate variable for the match expression",
lambda match, line: file_name.endswith('script_thread.rs')),
]
for pattern, message, filter_func in regex_rules:

View File

@ -0,0 +1,18 @@
fn main() {
// This should trigger an error.
match self.documents.borrow_mut() {
_ => {}
}
// This should trigger an error.
match self.documents.borrow() {
_ => {}
}
// This should not trigger an error.
match { self.documents.borrow().find_window(id) } {
=> {}
}
// This should not trigger an error.
match self.documents_status.borrow() {
=> {}
}
}

View File

@ -141,6 +141,12 @@ class CheckTidiness(unittest.TestCase):
self.assertEqual('method declared in webidl is missing a comment with a specification link', errors.next()[2])
self.assertNoMoreErrors(errors)
def test_script_thread(self):
errors = tidy.collect_errors_for_files(iterFile('script_thread.rs'), [], [tidy.check_rust], print_text=False)
self.assertEqual('use a separate variable for the match expression', errors.next()[2])
self.assertEqual('use a separate variable for the match expression', errors.next()[2])
self.assertNoMoreErrors(errors)
def test_webidl(self):
errors = tidy.collect_errors_for_files(iterFile('spec.webidl'), [tidy.check_webidl_spec], [], print_text=False)
self.assertEqual('No specification link found.', errors.next()[2])