Skip to content

Commit bbc8216

Browse files
author
bors-servo
authored
Auto merge of #14173 - asajeffrey:script-thread-stores-top-level-frame-id, r=paulrouget
Report panics using the top-level frame id rather than the pipeline id <!-- Please describe your changes on the following line: --> At the moment, we report panics from script and layout using the root pipeline id. Once we are sharing more script threads, there won't be a root pipeline id any more. The plan is only to share script threads in the same tab, so there will be a top-level frame id that all the pipelines have in common. This PR reports panics using that top-level frame id. This mostly makes a difference to the browser API: rather than targeting a `mozbrowsererror` event at the root iframe of the script thread that panicked, it targets it at the containing mozbrowser iframe. --- <!-- 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 do not require tests because we don't test crash reporting <!-- 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="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14173) <!-- Reviewable:end -->
2 parents 0ea0ade + c228a4c commit bbc8216

File tree

9 files changed

+202
-194
lines changed

9 files changed

+202
-194
lines changed

‎components/constellation/constellation.rs‎

Lines changed: 159 additions & 171 deletions
Large diffs are not rendered by default.

‎components/constellation/pipeline.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,8 @@ pub struct InitialPipelineState {
8282
pub id: PipelineId,
8383
/// The ID of the frame that contains this Pipeline.
8484
pub frame_id: FrameId,
85+
/// The ID of the top-level frame that contains this Pipeline.
86+
pub top_level_frame_id: FrameId,
8587
/// The ID of the parent pipeline and frame type, if any.
8688
/// If `None`, this is the root.
8789
pub parent_info: Option<(PipelineId, FrameType)>,
@@ -204,6 +206,7 @@ impl Pipeline {
204206
let unprivileged_pipeline_content = UnprivilegedPipelineContent {
205207
id: state.id,
206208
frame_id: state.frame_id,
209+
top_level_frame_id: state.top_level_frame_id,
207210
parent_info: state.parent_info,
208211
constellation_chan: state.constellation_chan,
209212
scheduler_chan: state.scheduler_chan,
@@ -381,6 +384,7 @@ impl Pipeline {
381384
pub struct UnprivilegedPipelineContent {
382385
id: PipelineId,
383386
frame_id: FrameId,
387+
top_level_frame_id: FrameId,
384388
parent_info: Option<(PipelineId, FrameType)>,
385389
constellation_chan: IpcSender<ScriptMsg>,
386390
layout_to_constellation_chan: IpcSender<LayoutMsg>,
@@ -416,6 +420,7 @@ impl UnprivilegedPipelineContent {
416420
let layout_pair = STF::create(InitialScriptState {
417421
id: self.id,
418422
frame_id: self.frame_id,
423+
top_level_frame_id: self.top_level_frame_id,
419424
parent_info: self.parent_info,
420425
control_chan: self.script_chan.clone(),
421426
control_port: self.script_port,
@@ -433,6 +438,7 @@ impl UnprivilegedPipelineContent {
433438
}, self.load_data.clone());
434439

435440
LTF::create(self.id,
441+
Some(self.top_level_frame_id),
436442
self.load_data.url,
437443
self.parent_info.is_some(),
438444
layout_pair,

‎components/layout_thread/lib.rs‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ extern crate selectors;
4242
extern crate serde_json;
4343
extern crate servo_url;
4444
extern crate style;
45-
extern crate style_traits;
4645
extern crate util;
4746
extern crate webrender_traits;
4847

@@ -80,7 +79,7 @@ use layout::webrender_helpers::{WebRenderDisplayListConverter, WebRenderFrameBui
8079
use layout::wrapper::LayoutNodeLayoutData;
8180
use layout::wrapper::drop_style_and_layout_data;
8281
use layout_traits::LayoutThreadFactory;
83-
use msg::constellation_msg::PipelineId;
82+
use msg::constellation_msg::{FrameId, PipelineId};
8483
use net_traits::image_cache_thread::{ImageCacheChan, ImageCacheResult, ImageCacheThread};
8584
use net_traits::image_cache_thread::UsePlaceholder;
8685
use parking_lot::RwLock;
@@ -235,6 +234,7 @@ impl LayoutThreadFactory for LayoutThread {
235234

236235
/// Spawns a new layout thread.
237236
fn create(id: PipelineId,
237+
top_level_frame_id: Option<FrameId>,
238238
url: ServoUrl,
239239
is_iframe: bool,
240240
chan: (Sender<Msg>, Receiver<Msg>),
@@ -251,7 +251,11 @@ impl LayoutThreadFactory for LayoutThread {
251251
thread::spawn_named(format!("LayoutThread {:?}", id),
252252
move || {
253253
thread_state::initialize(thread_state::LAYOUT);
254-
PipelineId::install(id);
254+
255+
if let Some(top_level_frame_id) = top_level_frame_id {
256+
FrameId::install(top_level_frame_id);
257+
}
258+
255259
{ // Ensures layout thread is destroyed before we send shutdown message
256260
let sender = chan.0;
257261
let layout = LayoutThread::new(id,
@@ -718,6 +722,7 @@ impl LayoutThread {
718722

719723
fn create_layout_thread(&self, info: NewLayoutThreadInfo) {
720724
LayoutThread::create(info.id,
725+
FrameId::installed(),
721726
info.url.clone(),
722727
info.is_parent,
723728
info.layout_pair,

‎components/layout_traits/lib.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ extern crate webrender_traits;
2020

2121
use gfx::font_cache_thread::FontCacheThread;
2222
use ipc_channel::ipc::{IpcReceiver, IpcSender};
23-
use msg::constellation_msg::PipelineId;
23+
use msg::constellation_msg::{FrameId, PipelineId};
2424
use net_traits::image_cache_thread::ImageCacheThread;
2525
use profile_traits::{mem, time};
2626
use script_traits::{ConstellationControlMsg, LayoutControlMsg};
@@ -33,6 +33,7 @@ use std::sync::mpsc::{Receiver, Sender};
3333
pub trait LayoutThreadFactory {
3434
type Message;
3535
fn create(id: PipelineId,
36+
top_level_frame_id: Option<FrameId>,
3637
url: ServoUrl,
3738
is_iframe: bool,
3839
chan: (Sender<Self::Message>, Receiver<Self::Message>),

‎components/msg/constellation_msg.rs‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -217,8 +217,6 @@ impl PipelineNamespace {
217217

218218
thread_local!(pub static PIPELINE_NAMESPACE: Cell<Option<PipelineNamespace>> = Cell::new(None));
219219

220-
thread_local!(pub static PIPELINE_ID: Cell<Option<PipelineId>> = Cell::new(None));
221-
222220
#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Copy, Hash, Debug, Deserialize, Serialize, HeapSizeOf)]
223221
pub struct PipelineNamespaceId(pub u32);
224222

@@ -246,15 +244,6 @@ impl PipelineId {
246244
let PipelineIndex(index) = self.index;
247245
webrender_traits::PipelineId(namespace_id, index)
248246
}
249-
250-
pub fn install(id: PipelineId) {
251-
PIPELINE_ID.with(|tls| tls.set(Some(id)))
252-
}
253-
254-
pub fn installed() -> Option<PipelineId> {
255-
PIPELINE_ID.with(|tls| tls.get())
256-
}
257-
258247
}
259248

260249
impl fmt::Display for PipelineId {
@@ -265,6 +254,8 @@ impl fmt::Display for PipelineId {
265254
}
266255
}
267256

257+
thread_local!(pub static TOP_LEVEL_FRAME_ID: Cell<Option<FrameId>> = Cell::new(None));
258+
268259
#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Copy, Hash, Debug, Deserialize, Serialize, HeapSizeOf)]
269260
pub struct FrameIndex(pub u32);
270261

@@ -283,6 +274,16 @@ impl FrameId {
283274
new_frame_id
284275
})
285276
}
277+
278+
/// Each script and layout thread should have the top-level frame id installed,
279+
/// since it is used by crash reporting.
280+
pub fn install(id: FrameId) {
281+
TOP_LEVEL_FRAME_ID.with(|tls| tls.set(Some(id)))
282+
}
283+
284+
pub fn installed() -> Option<FrameId> {
285+
TOP_LEVEL_FRAME_ID.with(|tls| tls.get())
286+
}
286287
}
287288

288289
impl fmt::Display for FrameId {

‎components/script/dom/dedicatedworkerglobalscope.rs‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ use js::jsapi::{HandleValue, JS_SetInterruptCallback};
2626
use js::jsapi::{JSAutoCompartment, JSContext};
2727
use js::jsval::UndefinedValue;
2828
use js::rust::Runtime;
29-
use msg::constellation_msg::PipelineId;
29+
use msg::constellation_msg::FrameId;
3030
use net_traits::{IpcSend, load_whole_resource};
3131
use net_traits::request::{CredentialsMode, Destination, RequestInit, Type as RequestType};
3232
use rand::random;
@@ -158,9 +158,14 @@ impl DedicatedWorkerGlobalScope {
158158
closing: Arc<AtomicBool>) {
159159
let serialized_worker_url = worker_url.to_string();
160160
let name = format!("WebWorker for {}", serialized_worker_url);
161+
let top_level_frame_id = FrameId::installed();
162+
161163
spawn_named(name, move || {
162164
thread_state::initialize(thread_state::SCRIPT | thread_state::IN_WORKER);
163-
PipelineId::install(init.pipeline_id);
165+
166+
if let Some(top_level_frame_id) = top_level_frame_id {
167+
FrameId::install(top_level_frame_id);
168+
}
164169

165170
let roots = RootCollection::new();
166171
let _stack_roots_tls = StackRootTLS::new(&roots);

‎components/script/script_thread.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -518,8 +518,8 @@ impl ScriptThreadFactory for ScriptThread {
518518
thread::spawn_named(format!("ScriptThread {:?}", state.id),
519519
move || {
520520
thread_state::initialize(thread_state::SCRIPT);
521-
PipelineId::install(state.id);
522521
PipelineNamespace::install(state.pipeline_namespace_id);
522+
FrameId::install(state.top_level_frame_id);
523523
let roots = RootCollection::new();
524524
let _stack_roots_tls = StackRootTLS::new(&roots);
525525
let id = state.id;

‎components/script_traits/lib.rs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,8 @@ pub struct InitialScriptState {
442442
pub parent_info: Option<(PipelineId, FrameType)>,
443443
/// The ID of the frame this script is part of.
444444
pub frame_id: FrameId,
445+
/// The ID of the top-level frame this script is part of.
446+
pub top_level_frame_id: FrameId,
445447
/// A channel with which messages can be sent to us (the script thread).
446448
pub control_chan: IpcSender<ConstellationControlMsg>,
447449
/// A port on which messages sent by the constellation to script can be received.
@@ -699,8 +701,8 @@ pub enum ConstellationMsg {
699701
WebDriverCommand(WebDriverCommandMsg),
700702
/// Reload the current page.
701703
Reload,
702-
/// A log entry, with the pipeline id and thread name
703-
LogEntry(Option<PipelineId>, Option<String>, LogEntry),
704+
/// A log entry, with the top-level frame id and thread name
705+
LogEntry(Option<FrameId>, Option<String>, LogEntry),
704706
}
705707

706708
/// Resources required by workerglobalscopes

‎components/script_traits/script_msg.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ use devtools_traits::{ScriptToDevtoolsControlMsg, WorkerId};
1515
use euclid::point::Point2D;
1616
use euclid::size::Size2D;
1717
use ipc_channel::ipc::IpcSender;
18+
use msg::constellation_msg::{FrameId, PipelineId, TraversalDirection};
1819
use msg::constellation_msg::{Key, KeyModifiers, KeyState};
19-
use msg::constellation_msg::{PipelineId, TraversalDirection};
2020
use net_traits::CoreResourceMsg;
2121
use net_traits::storage_thread::StorageType;
2222
use offscreen_gl_context::{GLContextAttributes, GLLimits};
@@ -131,8 +131,8 @@ pub enum ScriptMsg {
131131
ResizeTo(Size2D<u32>),
132132
/// Script has handled a touch event, and either prevented or allowed default actions.
133133
TouchEventProcessed(EventResult),
134-
/// A log entry, with the pipeline id and thread name
135-
LogEntry(Option<PipelineId>, Option<String>, LogEntry),
134+
/// A log entry, with the top-level frame id and thread name
135+
LogEntry(Option<FrameId>, Option<String>, LogEntry),
136136
/// Notifies the constellation that this pipeline has exited.
137137
PipelineExited(PipelineId),
138138
/// Send messages from postMessage calls from serviceworker

0 commit comments

Comments
 (0)