Skip to content

Commit 654104e

Browse files
author
bors-servo
authored
Auto merge of #11695 - asajeffrey:script-thread-dont-panic, r=Ms2ger
Removed some sources of panic from script thread. <!-- Please describe your changes on the following line: --> **This PR needs some thought!** It removes some sources of panic from script_thread.rs, caused by pipeline lookup failure. It's incomplete, as there are some uses of `get_browsing_context` elsewhere in the code. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #11693 and #11685 (and probably some other intermittents) - [X] These changes do not require tests because it is fixing intermittent panic <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11695) <!-- Reviewable:end -->
2 parents 86d65b6 + 9da00e2 commit 654104e

File tree

1 file changed

+58
-30
lines changed

1 file changed

+58
-30
lines changed

‎components/script/script_thread.rs‎

Lines changed: 58 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1008,7 +1008,10 @@ impl ScriptThread {
10081008
let context = self.root_browsing_context();
10091009
match msg {
10101010
DevtoolScriptControlMsg::EvaluateJS(id, s, reply) => {
1011-
let window = get_browsing_context(&context, id).active_window();
1011+
let window = match context.find(id) {
1012+
Some(browsing_context) => browsing_context.active_window(),
1013+
None => return warn!("Message sent to closed pipeline {}.", id),
1014+
};
10121015
let global_ref = GlobalRef::Window(window.r());
10131016
devtools::handle_evaluate_js(&global_ref, s, reply)
10141017
},
@@ -1025,7 +1028,10 @@ impl ScriptThread {
10251028
DevtoolScriptControlMsg::ModifyAttribute(id, node_id, modifications) =>
10261029
devtools::handle_modify_attribute(&context, id, node_id, modifications),
10271030
DevtoolScriptControlMsg::WantsLiveNotifications(id, to_send) => {
1028-
let window = get_browsing_context(&context, id).active_window();
1031+
let window = match context.find(id) {
1032+
Some(browsing_context) => browsing_context.active_window(),
1033+
None => return warn!("Message sent to closed pipeline {}.", id),
1034+
};
10291035
let global_ref = GlobalRef::Window(window.r());
10301036
devtools::handle_wants_live_notifications(&global_ref, to_send)
10311037
},
@@ -1108,8 +1114,7 @@ impl ScriptThread {
11081114
if let Some(inner_context) = context.find(id) {
11091115
let window = inner_context.active_window();
11101116
if window.set_page_clip_rect_with_new_viewport(rect) {
1111-
let context = get_browsing_context(&context, id);
1112-
self.rebuild_and_force_reflow(&context, ReflowReason::Viewport);
1117+
self.rebuild_and_force_reflow(&inner_context, ReflowReason::Viewport);
11131118
}
11141119
return;
11151120
}
@@ -1119,7 +1124,7 @@ impl ScriptThread {
11191124
load.clip_rect = Some(rect);
11201125
return;
11211126
}
1122-
panic!("Page rect message sent to nonexistent pipeline");
1127+
warn!("Page rect message sent to nonexistent pipeline");
11231128
}
11241129

11251130
fn handle_set_scroll_state(&self,
@@ -1134,7 +1139,7 @@ impl ScriptThread {
11341139
}
11351140
}
11361141
}
1137-
None => panic!("Set scroll state message sent to nonexistent pipeline: {:?}", id),
1142+
None => return warn!("Set scroll state message sent to nonexistent pipeline: {:?}", id),
11381143
};
11391144

11401145
let mut scroll_offsets = HashMap::new();
@@ -1199,8 +1204,10 @@ impl ScriptThread {
11991204
}
12001205

12011206
fn handle_loads_complete(&self, pipeline: PipelineId) {
1202-
let context = get_browsing_context(&self.root_browsing_context(), pipeline);
1203-
let doc = context.active_document();
1207+
let doc = match self.root_browsing_context().find(pipeline) {
1208+
Some(browsing_context) => browsing_context.active_document(),
1209+
None => return warn!("Message sent to closed pipeline {}.", pipeline),
1210+
};
12041211
let doc = doc.r();
12051212
if doc.loader().is_blocked() {
12061213
return;
@@ -1306,7 +1313,7 @@ impl ScriptThread {
13061313
load.is_frozen = true;
13071314
return;
13081315
}
1309-
panic!("freeze sent to nonexistent pipeline");
1316+
warn!("freeze sent to nonexistent pipeline");
13101317
}
13111318

13121319
/// Handles thaw message
@@ -1325,7 +1332,7 @@ impl ScriptThread {
13251332
load.is_frozen = false;
13261333
return;
13271334
}
1328-
panic!("thaw sent to nonexistent pipeline");
1335+
warn!("thaw sent to nonexistent pipeline");
13291336
}
13301337

13311338
fn handle_focus_iframe_msg(&self,
@@ -1444,8 +1451,10 @@ impl ScriptThread {
14441451

14451452
/// Handles a request for the window title.
14461453
fn handle_get_title_msg(&self, pipeline_id: PipelineId) {
1447-
let context = get_browsing_context(&self.root_browsing_context(), pipeline_id);
1448-
let document = context.active_document();
1454+
let document = match self.root_browsing_context().find(pipeline_id) {
1455+
Some(browsing_context) => browsing_context.active_document(),
1456+
None => return warn!("Message sent to closed pipeline {}.", pipeline_id),
1457+
};
14491458
document.send_title_to_compositor();
14501459
}
14511460

@@ -1499,8 +1508,10 @@ impl ScriptThread {
14991508

15001509
/// Handles when layout thread finishes all animation in one tick
15011510
fn handle_tick_all_animations(&self, id: PipelineId) {
1502-
let context = get_browsing_context(&self.root_browsing_context(), id);
1503-
let document = context.active_document();
1511+
let document = match self.root_browsing_context().find(id) {
1512+
Some(browsing_context) => browsing_context.active_document(),
1513+
None => return warn!("Message sent to closed pipeline {}.", id),
1514+
};
15041515
document.run_the_animation_frame_callbacks();
15051516
}
15061517

@@ -1513,8 +1524,10 @@ impl ScriptThread {
15131524

15141525
/// Notify the containing document of a child frame that has completed loading.
15151526
fn handle_frame_load_event(&self, containing_pipeline: PipelineId, id: PipelineId) {
1516-
let context = get_browsing_context(&self.root_browsing_context(), containing_pipeline);
1517-
let document = context.active_document();
1527+
let document = match self.root_browsing_context().find(containing_pipeline) {
1528+
Some(browsing_context) => browsing_context.active_document(),
1529+
None => return warn!("Message sent to closed pipeline {}.", containing_pipeline),
1530+
};
15181531
if let Some(iframe) = document.find_iframe_by_pipeline(id) {
15191532
iframe.iframe_load_event_steps(id);
15201533
}
@@ -1839,8 +1852,10 @@ impl ScriptThread {
18391852
}
18401853

18411854
MouseMoveEvent(point) => {
1842-
let context = get_browsing_context(&self.root_browsing_context(), pipeline_id);
1843-
let document = context.active_document();
1855+
let document = match self.root_browsing_context().find(pipeline_id) {
1856+
Some(browsing_context) => browsing_context.active_document(),
1857+
None => return warn!("Message sent to closed pipeline {}.", pipeline_id),
1858+
};
18441859

18451860
// Get the previous target temporarily
18461861
let prev_mouse_over_target = self.topmost_mouse_over_target.get();
@@ -1909,14 +1924,18 @@ impl ScriptThread {
19091924
}
19101925

19111926
TouchpadPressureEvent(point, pressure, phase) => {
1912-
let context = get_browsing_context(&self.root_browsing_context(), pipeline_id);
1913-
let document = context.active_document();
1927+
let document = match self.root_browsing_context().find(pipeline_id) {
1928+
Some(browsing_context) => browsing_context.active_document(),
1929+
None => return warn!("Message sent to closed pipeline {}.", pipeline_id),
1930+
};
19141931
document.r().handle_touchpad_pressure_event(self.js_runtime.rt(), point, pressure, phase);
19151932
}
19161933

19171934
KeyEvent(key, state, modifiers) => {
1918-
let context = get_browsing_context(&self.root_browsing_context(), pipeline_id);
1919-
let document = context.active_document();
1935+
let document = match self.root_browsing_context().find(pipeline_id) {
1936+
Some(browsing_context) => browsing_context.active_document(),
1937+
None => return warn!("Message sent to closed pipeline {}.", pipeline_id),
1938+
};
19201939
document.dispatch_key_event(key, state, modifiers, &self.constellation_chan);
19211940
}
19221941
}
@@ -1927,8 +1946,10 @@ impl ScriptThread {
19271946
mouse_event_type: MouseEventType,
19281947
button: MouseButton,
19291948
point: Point2D<f32>) {
1930-
let context = get_browsing_context(&self.root_browsing_context(), pipeline_id);
1931-
let document = context.active_document();
1949+
let document = match self.root_browsing_context().find(pipeline_id) {
1950+
Some(browsing_context) => browsing_context.active_document(),
1951+
None => return warn!("Message sent to closed pipeline {}.", pipeline_id),
1952+
};
19321953
document.handle_mouse_event(self.js_runtime.rt(), button, point, mouse_event_type);
19331954
}
19341955

@@ -1938,8 +1959,10 @@ impl ScriptThread {
19381959
identifier: TouchId,
19391960
point: Point2D<f32>)
19401961
-> bool {
1941-
let context = get_browsing_context(&self.root_browsing_context(), pipeline_id);
1942-
let document = context.active_document();
1962+
let document = match self.root_browsing_context().find(pipeline_id) {
1963+
Some(browsing_context) => browsing_context.active_document(),
1964+
None => { warn!("Message sent to closed pipeline {}.", pipeline_id); return true },
1965+
};
19431966
document.handle_touch_event(self.js_runtime.rt(), event_type, identifier, point)
19441967
}
19451968

@@ -1951,9 +1974,10 @@ impl ScriptThread {
19511974
{
19521975
let nurl = &load_data.url;
19531976
if let Some(fragment) = nurl.fragment() {
1954-
let context = get_browsing_context(&self.root_browsing_context(), pipeline_id);
1955-
let document = context.active_document();
1956-
let document = document.r();
1977+
let document = match self.root_browsing_context().find(pipeline_id) {
1978+
Some(browsing_context) => browsing_context.active_document(),
1979+
None => return warn!("Message sent to closed pipeline {}.", pipeline_id),
1980+
};
19571981
let url = document.url();
19581982
if &url[..Position::AfterQuery] == &nurl[..Position::AfterQuery] &&
19591983
load_data.method == Method::Get {
@@ -1988,7 +2012,10 @@ impl ScriptThread {
19882012
}
19892013

19902014
fn handle_resize_event(&self, pipeline_id: PipelineId, new_size: WindowSizeData, size_type: WindowSizeType) {
1991-
let context = get_browsing_context(&self.root_browsing_context(), pipeline_id);
2015+
let context = match self.root_browsing_context().find(pipeline_id) {
2016+
Some(browsing_context) => browsing_context,
2017+
None => return warn!("Message sent to closed pipeline {}.", pipeline_id),
2018+
};
19922019
let window = context.active_window();
19932020
window.set_window_size(new_size);
19942021
window.force_reflow(ReflowGoal::ForDisplay,
@@ -2159,6 +2186,7 @@ fn shut_down_layout(context_tree: &BrowsingContext) {
21592186
}
21602187
}
21612188

2189+
// TODO: remove this function, as it's a source of panic.
21622190
pub fn get_browsing_context(context: &BrowsingContext,
21632191
pipeline_id: PipelineId)
21642192
-> Root<BrowsingContext> {

0 commit comments

Comments
 (0)