Skip to content

Commit d1b7f19

Browse files
author
bors-servo
authored
Auto merge of #14013 - asajeffrey:script-thread-no-root-document, r=jdm
Script thread with no root document <!-- Please describe your changes on the following line: --> This PR removes the single root document from the script thread, and replaces it by a lookup table from `PipelineId`s to `Document`s. This is needed if we're going to share script threads, as per #633. The last commit is the one that matters, the ones before are #13646. cc @jdm @Ms2ger @ConnorGBrewster --- <!-- 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 refactoring <!-- 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/14013) <!-- Reviewable:end -->
2 parents 2e2eab0 + 90e9c91 commit d1b7f19

File tree

8 files changed

+494
-668
lines changed

8 files changed

+494
-668
lines changed

‎components/constellation/constellation.rs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1636,12 +1636,12 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
16361636
pipeline_id: PipelineId,
16371637
event: MozBrowserEvent) {
16381638
assert!(PREFS.is_mozbrowser_enabled());
1639-
let frame_id = self.pipelines.get(&pipeline_id).map(|pipeline| pipeline.frame_id);
16401639

16411640
// Find the script channel for the given parent pipeline,
16421641
// and pass the event to that script thread.
16431642
// If the pipeline lookup fails, it is because we have torn down the pipeline,
16441643
// so it is reasonable to silently ignore the event.
1644+
let frame_id = self.pipelines.get(&pipeline_id).map(|pipeline| pipeline.frame_id);
16451645
match self.pipelines.get(&parent_pipeline_id) {
16461646
Some(pipeline) => pipeline.trigger_mozbrowser_event(frame_id, event),
16471647
None => warn!("Pipeline {:?} handling mozbrowser event after closure.", parent_pipeline_id),
@@ -2219,6 +2219,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
22192219

22202220
// Close a frame (and all children)
22212221
fn close_frame(&mut self, frame_id: FrameId, exit_mode: ExitPipelineMode) {
2222+
debug!("Closing frame {:?}.", frame_id);
22222223
// Store information about the pipelines to be closed. Then close the
22232224
// pipelines, before removing ourself from the frames hash map. This
22242225
// ordering is vital - so that if close_pipeline() ends up closing
@@ -2254,10 +2255,12 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
22542255
};
22552256
parent_pipeline.remove_child(frame_id);
22562257
}
2258+
debug!("Closed frame {:?}.", frame_id);
22572259
}
22582260

22592261
// Close all pipelines at and beneath a given frame
22602262
fn close_pipeline(&mut self, pipeline_id: PipelineId, exit_mode: ExitPipelineMode) {
2263+
debug!("Closing pipeline {:?}.", pipeline_id);
22612264
// Store information about the frames to be closed. Then close the
22622265
// frames, before removing ourself from the pipelines hash map. This
22632266
// ordering is vital - so that if close_frames() ends up closing
@@ -2297,6 +2300,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
22972300
ExitPipelineMode::Normal => pipeline.exit(),
22982301
ExitPipelineMode::Force => pipeline.force_exit(),
22992302
}
2303+
debug!("Closed pipeline {:?}.", pipeline_id);
23002304
}
23012305

23022306
// Randomly close a pipeline -if --random-pipeline-closure-probability is set

‎components/script/devtools.rs‎

Lines changed: 40 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,15 @@ use dom::bindings::inheritance::Castable;
1717
use dom::bindings::js::Root;
1818
use dom::bindings::reflector::Reflectable;
1919
use dom::bindings::str::DOMString;
20-
use dom::browsingcontext::BrowsingContext;
2120
use dom::element::Element;
2221
use dom::globalscope::GlobalScope;
23-
use dom::node::Node;
22+
use dom::node::{Node, window_from_node};
2423
use dom::window::Window;
2524
use ipc_channel::ipc::IpcSender;
2625
use js::jsapi::{JSAutoCompartment, ObjectClassName};
2726
use js::jsval::UndefinedValue;
2827
use msg::constellation_msg::PipelineId;
28+
use script_thread::Documents;
2929
use std::ffi::CStr;
3030
use std::str;
3131
use style::properties::longhands::{margin_bottom, margin_left, margin_right, margin_top};
@@ -72,53 +72,35 @@ pub fn handle_evaluate_js(global: &GlobalScope, eval: String, reply: IpcSender<E
7272
reply.send(result).unwrap();
7373
}
7474

75-
pub fn handle_get_root_node(context: &BrowsingContext, pipeline: PipelineId, reply: IpcSender<Option<NodeInfo>>) {
76-
let context = match context.find(pipeline) {
77-
Some(found_context) => found_context,
78-
None => return reply.send(None).unwrap()
79-
};
80-
81-
let document = context.active_document();
82-
83-
let node = document.upcast::<Node>();
84-
reply.send(Some(node.summarize())).unwrap();
75+
pub fn handle_get_root_node(documents: &Documents, pipeline: PipelineId, reply: IpcSender<Option<NodeInfo>>) {
76+
let info = documents.find_document(pipeline)
77+
.map(|document| document.upcast::<Node>().summarize());
78+
reply.send(info).unwrap();
8579
}
8680

87-
pub fn handle_get_document_element(context: &BrowsingContext,
81+
pub fn handle_get_document_element(documents: &Documents,
8882
pipeline: PipelineId,
8983
reply: IpcSender<Option<NodeInfo>>) {
90-
let context = match context.find(pipeline) {
91-
Some(found_context) => found_context,
92-
None => return reply.send(None).unwrap()
93-
};
94-
95-
let document = context.active_document();
96-
let document_element = document.GetDocumentElement().unwrap();
97-
98-
let node = document_element.upcast::<Node>();
99-
reply.send(Some(node.summarize())).unwrap();
84+
let info = documents.find_document(pipeline)
85+
.and_then(|document| document.GetDocumentElement())
86+
.map(|element| element.upcast::<Node>().summarize());
87+
reply.send(info).unwrap();
10088
}
10189

102-
fn find_node_by_unique_id(context: &BrowsingContext,
90+
fn find_node_by_unique_id(documents: &Documents,
10391
pipeline: PipelineId,
10492
node_id: &str)
10593
-> Option<Root<Node>> {
106-
let context = match context.find(pipeline) {
107-
Some(found_context) => found_context,
108-
None => return None
109-
};
110-
111-
let document = context.active_document();
112-
let node = document.upcast::<Node>();
113-
114-
node.traverse_preorder().find(|candidate| candidate.unique_id() == node_id)
94+
documents.find_document(pipeline).and_then(|document|
95+
document.upcast::<Node>().traverse_preorder().find(|candidate| candidate.unique_id() == node_id)
96+
)
11597
}
11698

117-
pub fn handle_get_children(context: &BrowsingContext,
99+
pub fn handle_get_children(documents: &Documents,
118100
pipeline: PipelineId,
119101
node_id: String,
120102
reply: IpcSender<Option<Vec<NodeInfo>>>) {
121-
match find_node_by_unique_id(context, pipeline, &*node_id) {
103+
match find_node_by_unique_id(documents, pipeline, &*node_id) {
122104
None => return reply.send(None).unwrap(),
123105
Some(parent) => {
124106
let children = parent.children()
@@ -130,11 +112,11 @@ pub fn handle_get_children(context: &BrowsingContext,
130112
};
131113
}
132114

133-
pub fn handle_get_layout(context: &BrowsingContext,
115+
pub fn handle_get_layout(documents: &Documents,
134116
pipeline: PipelineId,
135117
node_id: String,
136118
reply: IpcSender<Option<ComputedNodeLayout>>) {
137-
let node = match find_node_by_unique_id(context, pipeline, &*node_id) {
119+
let node = match find_node_by_unique_id(documents, pipeline, &*node_id) {
138120
None => return reply.send(None).unwrap(),
139121
Some(found_node) => found_node
140122
};
@@ -144,7 +126,7 @@ pub fn handle_get_layout(context: &BrowsingContext,
144126
let width = rect.Width() as f32;
145127
let height = rect.Height() as f32;
146128

147-
let window = context.active_window();
129+
let window = window_from_node(&*node);
148130
let elem = node.downcast::<Element>().expect("should be getting layout of element");
149131
let computed_style = window.GetComputedStyle(elem, None);
150132

@@ -223,11 +205,11 @@ pub fn handle_get_cached_messages(_pipeline_id: PipelineId,
223205
reply.send(messages).unwrap();
224206
}
225207

226-
pub fn handle_modify_attribute(context: &BrowsingContext,
208+
pub fn handle_modify_attribute(documents: &Documents,
227209
pipeline: PipelineId,
228210
node_id: String,
229211
modifications: Vec<Modification>) {
230-
let node = match find_node_by_unique_id(context, pipeline, &*node_id) {
212+
let node = match find_node_by_unique_id(documents, pipeline, &*node_id) {
231213
None => return warn!("node id {} for pipeline id {} is not found", &node_id, &pipeline),
232214
Some(found_node) => found_node
233215
};
@@ -249,49 +231,39 @@ pub fn handle_wants_live_notifications(global: &GlobalScope, send_notifications:
249231
global.set_devtools_wants_updates(send_notifications);
250232
}
251233

252-
pub fn handle_set_timeline_markers(context: &BrowsingContext,
234+
pub fn handle_set_timeline_markers(documents: &Documents,
253235
pipeline: PipelineId,
254236
marker_types: Vec<TimelineMarkerType>,
255237
reply: IpcSender<Option<TimelineMarker>>) {
256-
match context.find(pipeline) {
238+
match documents.find_window(pipeline) {
257239
None => reply.send(None).unwrap(),
258-
Some(context) => context.active_window().set_devtools_timeline_markers(marker_types, reply),
240+
Some(window) => window.set_devtools_timeline_markers(marker_types, reply),
259241
}
260242
}
261243

262-
pub fn handle_drop_timeline_markers(context: &BrowsingContext,
244+
pub fn handle_drop_timeline_markers(documents: &Documents,
263245
pipeline: PipelineId,
264246
marker_types: Vec<TimelineMarkerType>) {
265-
if let Some(context) = context.find(pipeline) {
266-
context.active_window().drop_devtools_timeline_markers(marker_types);
247+
if let Some(window) = documents.find_window(pipeline) {
248+
window.drop_devtools_timeline_markers(marker_types);
267249
}
268250
}
269251

270-
pub fn handle_request_animation_frame(context: &BrowsingContext,
252+
pub fn handle_request_animation_frame(documents: &Documents,
271253
id: PipelineId,
272254
actor_name: String) {
273-
let context = match context.find(id) {
274-
None => return warn!("context for pipeline id {} is not found", id),
275-
Some(found_node) => found_node
276-
};
277-
278-
let doc = context.active_document();
279-
let devtools_sender =
280-
context.active_window().upcast::<GlobalScope>().devtools_chan().unwrap().clone();
281-
doc.request_animation_frame(box move |time| {
282-
let msg = ScriptToDevtoolsControlMsg::FramerateTick(actor_name, time);
283-
devtools_sender.send(msg).unwrap();
284-
});
255+
if let Some(doc) = documents.find_document(id) {
256+
let devtools_sender = doc.window().upcast::<GlobalScope>().devtools_chan().unwrap().clone();
257+
doc.request_animation_frame(box move |time| {
258+
let msg = ScriptToDevtoolsControlMsg::FramerateTick(actor_name, time);
259+
devtools_sender.send(msg).unwrap();
260+
});
261+
}
285262
}
286263

287-
pub fn handle_reload(context: &BrowsingContext,
264+
pub fn handle_reload(documents: &Documents,
288265
id: PipelineId) {
289-
let context = match context.find(id) {
290-
None => return warn!("context for pipeline id {} is not found", id),
291-
Some(found_node) => found_node
292-
};
293-
294-
let win = context.active_window();
295-
let location = win.Location();
296-
location.Reload();
266+
if let Some(win) = documents.find_window(id) {
267+
win.Location().Reload();
268+
}
297269
}

‎components/script/dom/browsingcontext.rs‎

Lines changed: 6 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
* License, v. 2.0. If a copy of the MPL was not distributed with this
33
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
44

5-
use dom::bindings::cell::DOMRefCell;
65
use dom::bindings::conversions::{ToJSValConvertible, root_from_handleobject};
76
use dom::bindings::inheritance::Castable;
87
use dom::bindings::js::{JS, MutNullableHeap, Root, RootedReference};
@@ -37,15 +36,9 @@ use std::cell::Cell;
3736
pub struct BrowsingContext {
3837
reflector: Reflector,
3938

40-
/// Pipeline id associated with this context.
41-
id: PipelineId,
42-
4339
/// Indicates if reflow is required when reloading.
4440
needs_reflow: Cell<bool>,
4541

46-
/// Stores the child browsing contexts (ex. iframe browsing context)
47-
children: DOMRefCell<Vec<JS<BrowsingContext>>>,
48-
4942
/// The current active document.
5043
/// Note that the session history is stored in the constellation,
5144
/// in the script thread we just track the current active document.
@@ -56,19 +49,17 @@ pub struct BrowsingContext {
5649
}
5750

5851
impl BrowsingContext {
59-
pub fn new_inherited(frame_element: Option<&Element>, id: PipelineId) -> BrowsingContext {
52+
pub fn new_inherited(frame_element: Option<&Element>) -> BrowsingContext {
6053
BrowsingContext {
6154
reflector: Reflector::new(),
62-
id: id,
6355
needs_reflow: Cell::new(true),
64-
children: DOMRefCell::new(vec![]),
6556
active_document: Default::default(),
6657
frame_element: frame_element.map(JS::from_ref),
6758
}
6859
}
6960

7061
#[allow(unsafe_code)]
71-
pub fn new(window: &Window, frame_element: Option<&Element>, id: PipelineId) -> Root<BrowsingContext> {
62+
pub fn new(window: &Window, frame_element: Option<&Element>) -> Root<BrowsingContext> {
7263
unsafe {
7364
let WindowProxyHandler(handler) = window.windowproxy_handler();
7465
assert!(!handler.is_null());
@@ -81,7 +72,7 @@ impl BrowsingContext {
8172
rooted!(in(cx) let window_proxy = NewWindowProxy(cx, parent, handler));
8273
assert!(!window_proxy.is_null());
8374

84-
let object = box BrowsingContext::new_inherited(frame_element, id);
75+
let object = box BrowsingContext::new_inherited(frame_element);
8576

8677
let raw = Box::into_raw(object);
8778
SetProxyExtra(window_proxy.get(), 0, &PrivateValue(raw as *const _));
@@ -118,82 +109,20 @@ impl BrowsingContext {
118109
window_proxy.get()
119110
}
120111

121-
pub fn remove(&self, id: PipelineId) -> Option<Root<BrowsingContext>> {
122-
let remove_idx = self.children
123-
.borrow()
124-
.iter()
125-
.position(|context| context.id == id);
126-
match remove_idx {
127-
Some(idx) => Some(Root::from_ref(&*self.children.borrow_mut().remove(idx))),
128-
None => {
129-
self.children
130-
.borrow_mut()
131-
.iter_mut()
132-
.filter_map(|context| context.remove(id))
133-
.next()
134-
}
135-
}
136-
}
137-
138112
pub fn set_reflow_status(&self, status: bool) -> bool {
139113
let old = self.needs_reflow.get();
140114
self.needs_reflow.set(status);
141115
old
142116
}
143117

144-
pub fn pipeline_id(&self) -> PipelineId {
145-
self.id
146-
}
147-
148-
pub fn push_child_context(&self, context: &BrowsingContext) {
149-
self.children.borrow_mut().push(JS::from_ref(&context));
150-
}
151-
152-
pub fn find_child_by_id(&self, pipeline_id: PipelineId) -> Option<Root<Window>> {
153-
self.children.borrow().iter().find(|context| {
154-
let window = context.active_window();
155-
window.upcast::<GlobalScope>().pipeline_id() == pipeline_id
156-
}).map(|context| context.active_window())
118+
pub fn active_pipeline_id(&self) -> Option<PipelineId> {
119+
self.active_document.get()
120+
.map(|doc| doc.window().upcast::<GlobalScope>().pipeline_id())
157121
}
158122

159123
pub fn unset_active_document(&self) {
160124
self.active_document.set(None)
161125
}
162-
163-
pub fn iter(&self) -> ContextIterator {
164-
ContextIterator {
165-
stack: vec!(Root::from_ref(self)),
166-
}
167-
}
168-
169-
pub fn find(&self, id: PipelineId) -> Option<Root<BrowsingContext>> {
170-
if self.id == id {
171-
return Some(Root::from_ref(self));
172-
}
173-
174-
self.children.borrow()
175-
.iter()
176-
.filter_map(|c| c.find(id))
177-
.next()
178-
}
179-
}
180-
181-
pub struct ContextIterator {
182-
stack: Vec<Root<BrowsingContext>>,
183-
}
184-
185-
impl Iterator for ContextIterator {
186-
type Item = Root<BrowsingContext>;
187-
188-
fn next(&mut self) -> Option<Root<BrowsingContext>> {
189-
let popped = self.stack.pop();
190-
if let Some(ref context) = popped {
191-
self.stack.extend(context.children.borrow()
192-
.iter()
193-
.map(|c| Root::from_ref(&**c)));
194-
}
195-
popped
196-
}
197126
}
198127

199128
#[allow(unsafe_code)]

0 commit comments

Comments
 (0)