Skip to content

Commit 56fbfd4

Browse files
aneeshusaaneeshusa
committed
Excise SubpageId and use only PipelineIds
SubpageId was originally introduced in 2013 to help iframes keep track of their associated (children) pipelines. However, since each pipeline already has a PipelineId, and those are unique, those are sufficient to keep track of children.
1 parent b9b25b6 commit 56fbfd4

File tree

12 files changed

+145
-217
lines changed

12 files changed

+145
-217
lines changed

‎components/constellation/constellation.rs‎

Lines changed: 39 additions & 63 deletions
Large diffs are not rendered by default.

‎components/constellation/pipeline.rs‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,7 @@ use ipc_channel::ipc::{self, IpcReceiver, IpcSender};
1717
use ipc_channel::router::ROUTER;
1818
use layers::geometry::DevicePixel;
1919
use layout_traits::LayoutThreadFactory;
20-
use msg::constellation_msg::{FrameId, FrameType, LoadData, PipelineId};
21-
use msg::constellation_msg::{PipelineNamespaceId, SubpageId};
20+
use msg::constellation_msg::{FrameId, FrameType, LoadData, PipelineId, PipelineNamespaceId};
2221
use net_traits::{IpcSend, ResourceThreads};
2322
use net_traits::bluetooth_thread::BluetoothMethodMsg;
2423
use net_traits::image_cache_thread::ImageCacheThread;
@@ -51,7 +50,7 @@ pub enum ChildProcess {
5150
/// A uniquely-identifiable pipeline of script thread, layout thread, and paint thread.
5251
pub struct Pipeline {
5352
pub id: PipelineId,
54-
pub parent_info: Option<(PipelineId, SubpageId, FrameType)>,
53+
pub parent_info: Option<(PipelineId, FrameType)>,
5554
pub script_chan: IpcSender<ConstellationControlMsg>,
5655
/// A channel to layout, for performing reflows and shutdown.
5756
pub layout_chan: IpcSender<LayoutControlMsg>,
@@ -84,9 +83,9 @@ pub struct Pipeline {
8483
pub struct InitialPipelineState {
8584
/// The ID of the pipeline to create.
8685
pub id: PipelineId,
87-
/// The subpage ID of this pipeline to create in its pipeline parent.
86+
/// The ID of the parent pipeline and frame type, if any.
8887
/// If `None`, this is the root.
89-
pub parent_info: Option<(PipelineId, SubpageId, FrameType)>,
88+
pub parent_info: Option<(PipelineId, FrameType)>,
9089
/// A channel to the associated constellation.
9190
pub constellation_chan: IpcSender<ScriptMsg>,
9291
/// A channel for the layout thread to send messages to the constellation.
@@ -150,12 +149,11 @@ impl Pipeline {
150149

151150
let (script_chan, content_ports) = match state.script_chan {
152151
Some(script_chan) => {
153-
let (parent_pipeline_id, subpage_id, frame_type) =
154-
state.parent_info.expect("script_pipeline != None but subpage_id == None");
152+
let (parent_pipeline_id, frame_type) =
153+
state.parent_info.expect("script_pipeline != None but parent_info == None");
155154
let new_layout_info = NewLayoutInfo {
156155
parent_pipeline_id: parent_pipeline_id,
157156
new_pipeline_id: state.id,
158-
subpage_id: subpage_id,
159157
frame_type: frame_type,
160158
load_data: state.load_data.clone(),
161159
paint_chan: layout_to_paint_chan.clone().to_opaque(),
@@ -273,7 +271,7 @@ impl Pipeline {
273271
}
274272

275273
fn new(id: PipelineId,
276-
parent_info: Option<(PipelineId, SubpageId, FrameType)>,
274+
parent_info: Option<(PipelineId, FrameType)>,
277275
script_chan: IpcSender<ConstellationControlMsg>,
278276
layout_chan: IpcSender<LayoutControlMsg>,
279277
compositor_proxy: Box<CompositorProxy + 'static + Send>,
@@ -377,12 +375,12 @@ impl Pipeline {
377375
}
378376

379377
pub fn trigger_mozbrowser_event(&self,
380-
subpage_id: Option<SubpageId>,
378+
child_id: Option<PipelineId>,
381379
event: MozBrowserEvent) {
382380
assert!(PREFS.is_mozbrowser_enabled());
383381

384382
let event = ConstellationControlMsg::MozBrowserEvent(self.id,
385-
subpage_id,
383+
child_id,
386384
event);
387385
if let Err(e) = self.script_chan.send(event) {
388386
warn!("Sending mozbrowser event to script failed ({}).", e);
@@ -409,7 +407,7 @@ impl Pipeline {
409407
#[derive(Deserialize, Serialize)]
410408
pub struct UnprivilegedPipelineContent {
411409
id: PipelineId,
412-
parent_info: Option<(PipelineId, SubpageId, FrameType)>,
410+
parent_info: Option<(PipelineId, FrameType)>,
413411
constellation_chan: IpcSender<ScriptMsg>,
414412
layout_to_constellation_chan: IpcSender<LayoutMsg>,
415413
scheduler_chan: IpcSender<TimerEventRequest>,

‎components/msg/constellation_msg.rs‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -331,9 +331,6 @@ impl fmt::Display for PipelineId {
331331
}
332332
}
333333

334-
#[derive(Clone, PartialEq, Eq, Copy, Hash, Debug, Deserialize, Serialize, HeapSizeOf)]
335-
pub struct SubpageId(pub u32);
336-
337334
#[derive(Clone, PartialEq, Eq, Copy, Hash, Debug, Deserialize, Serialize, HeapSizeOf)]
338335
pub enum FrameType {
339336
IFrame,

‎components/script/dom/bindings/trace.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ use js::jsapi::{GCTraceKindToAscii, Heap, JSObject, JSTracer, TraceKind};
5757
use js::jsval::JSVal;
5858
use js::rust::Runtime;
5959
use libc;
60-
use msg::constellation_msg::{FrameType, PipelineId, ReferrerPolicy, SubpageId, WindowSizeType};
60+
use msg::constellation_msg::{FrameType, PipelineId, ReferrerPolicy, WindowSizeType};
6161
use net_traits::{Metadata, NetworkError, ResourceThreads};
6262
use net_traits::filemanager_thread::RelativePos;
6363
use net_traits::image::base::{Image, ImageMetadata};
@@ -308,7 +308,7 @@ no_jsmanaged_fields!(PropertyDeclarationBlock);
308308
no_jsmanaged_fields!(HashSet<T>);
309309
// These three are interdependent, if you plan to put jsmanaged data
310310
// in one of these make sure it is propagated properly to containing structs
311-
no_jsmanaged_fields!(FrameType, SubpageId, WindowSizeData, WindowSizeType, PipelineId);
311+
no_jsmanaged_fields!(FrameType, WindowSizeData, WindowSizeType, PipelineId);
312312
no_jsmanaged_fields!(TimerEventId, TimerSource);
313313
no_jsmanaged_fields!(WorkerId);
314314
no_jsmanaged_fields!(QuirksMode);

‎components/script/dom/browsingcontext.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ use js::jsapi::{JS_GetOwnPropertyDescriptorById, JS_HasPropertyById};
2626
use js::jsapi::{MutableHandle, MutableHandleObject, MutableHandleValue};
2727
use js::jsapi::{ObjectOpResult, PropertyDescriptor};
2828
use js::jsval::{UndefinedValue, PrivateValue};
29-
use msg::constellation_msg::{PipelineId, SubpageId};
29+
use msg::constellation_msg::PipelineId;
3030
use std::cell::Cell;
3131
use url::Url;
3232

@@ -160,10 +160,10 @@ impl BrowsingContext {
160160
self.children.borrow_mut().push(JS::from_ref(&context));
161161
}
162162

163-
pub fn find_child_by_subpage(&self, subpage_id: SubpageId) -> Option<Root<Window>> {
163+
pub fn find_child_by_id(&self, pipeline_id: PipelineId) -> Option<Root<Window>> {
164164
self.children.borrow().iter().find(|context| {
165165
let window = context.active_window();
166-
window.subpage() == Some(subpage_id)
166+
window.pipeline_id() == pipeline_id
167167
}).map(|context| context.active_window())
168168
}
169169

‎components/script/dom/document.rs‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ use js::jsapi::{JSContext, JSObject, JSRuntime};
9494
use js::jsapi::JS_GetRuntime;
9595
use msg::constellation_msg::{ALT, CONTROL, SHIFT, SUPER};
9696
use msg::constellation_msg::{Key, KeyModifiers, KeyState};
97-
use msg::constellation_msg::{PipelineId, ReferrerPolicy, SubpageId};
97+
use msg::constellation_msg::{PipelineId, ReferrerPolicy};
9898
use net_traits::{AsyncResponseTarget, IpcSend, PendingAsyncLoad};
9999
use net_traits::CookieSource::NonHTTP;
100100
use net_traits::CoreResourceMsg::{GetCookiesForUrl, SetCookiesForUrl};
@@ -1342,9 +1342,9 @@ impl Document {
13421342

13431343
pub fn trigger_mozbrowser_event(&self, event: MozBrowserEvent) {
13441344
if PREFS.is_mozbrowser_enabled() {
1345-
if let Some((parent_pipeline_id, subpage_id, _)) = self.window.parent_info() {
1345+
if let Some((parent_pipeline_id, _)) = self.window.parent_info() {
13461346
let event = ConstellationMsg::MozBrowserEvent(parent_pipeline_id,
1347-
Some(subpage_id),
1347+
Some(self.window.pipeline_id()),
13481348
event);
13491349
self.window.constellation_chan().send(event).unwrap();
13501350
}
@@ -1581,15 +1581,7 @@ impl Document {
15811581
}
15821582

15831583
/// Find an iframe element in the document.
1584-
pub fn find_iframe(&self, subpage_id: SubpageId) -> Option<Root<HTMLIFrameElement>> {
1585-
self.upcast::<Node>()
1586-
.traverse_preorder()
1587-
.filter_map(Root::downcast::<HTMLIFrameElement>)
1588-
.find(|node| node.subpage_id() == Some(subpage_id))
1589-
}
1590-
1591-
/// Find an iframe element in the document.
1592-
pub fn find_iframe_by_pipeline(&self, pipeline: PipelineId) -> Option<Root<HTMLIFrameElement>> {
1584+
pub fn find_iframe(&self, pipeline: PipelineId) -> Option<Root<HTMLIFrameElement>> {
15931585
self.upcast::<Node>()
15941586
.traverse_preorder()
15951587
.filter_map(Root::downcast::<HTMLIFrameElement>)

‎components/script/dom/htmliframeelement.rs‎

Lines changed: 12 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ use dom::window::{ReflowReason, Window};
3838
use ipc_channel::ipc;
3939
use js::jsapi::{JSAutoCompartment, JSContext, MutableHandleValue};
4040
use js::jsval::{NullValue, UndefinedValue};
41-
use msg::constellation_msg::{FrameType, LoadData, PipelineId, SubpageId, TraversalDirection};
41+
use msg::constellation_msg::{FrameType, LoadData, PipelineId, TraversalDirection};
4242
use net_traits::response::HttpsState;
4343
use script_layout_interface::message::ReflowQueryType;
4444
use script_traits::{IFrameLoadInfo, MozBrowserEvent, ScriptMsg as ConstellationMsg};
@@ -68,7 +68,6 @@ bitflags! {
6868
pub struct HTMLIFrameElement {
6969
htmlelement: HTMLElement,
7070
pipeline_id: Cell<Option<PipelineId>>,
71-
subpage_id: Cell<Option<SubpageId>>,
7271
sandbox: MutNullableHeap<JS<DOMTokenList>>,
7372
sandbox_allowance: Cell<Option<SandboxAllowance>>,
7473
load_blocker: DOMRefCell<Option<LoadBlocker>>,
@@ -94,14 +93,11 @@ impl HTMLIFrameElement {
9493
}).unwrap_or_else(|| Url::parse("about:blank").unwrap())
9594
}
9695

97-
pub fn generate_new_subpage_id(&self) -> (SubpageId, Option<SubpageId>) {
98-
self.pipeline_id.set(Some(PipelineId::new()));
99-
100-
let old_subpage_id = self.subpage_id.get();
101-
let win = window_from_node(self);
102-
let subpage_id = win.get_next_subpage_id();
103-
self.subpage_id.set(Some(subpage_id));
104-
(subpage_id, old_subpage_id)
96+
pub fn generate_new_pipeline_id(&self) -> (Option<PipelineId>, PipelineId) {
97+
let old_pipeline_id = self.pipeline_id.get();
98+
let new_pipeline_id = PipelineId::new();
99+
self.pipeline_id.set(Some(new_pipeline_id));
100+
(old_pipeline_id, new_pipeline_id)
105101
}
106102

107103
pub fn navigate_or_reload_child_browsing_context(&self, load_data: Option<LoadData>) {
@@ -126,16 +122,14 @@ impl HTMLIFrameElement {
126122
}
127123

128124
let window = window_from_node(self);
129-
let (new_subpage_id, old_subpage_id) = self.generate_new_subpage_id();
130-
let new_pipeline_id = self.pipeline_id.get().unwrap();
125+
let (old_pipeline_id, new_pipeline_id) = self.generate_new_pipeline_id();
131126
let private_iframe = self.privatebrowsing();
132127
let frame_type = if self.Mozbrowser() { FrameType::MozBrowserIFrame } else { FrameType::IFrame };
133128

134129
let load_info = IFrameLoadInfo {
135130
load_data: load_data,
136131
parent_pipeline_id: window.pipeline_id(),
137-
new_subpage_id: new_subpage_id,
138-
old_subpage_id: old_subpage_id,
132+
old_pipeline_id: old_pipeline_id,
139133
new_pipeline_id: new_pipeline_id,
140134
sandbox: sandboxed,
141135
is_private: private_iframe,
@@ -170,8 +164,7 @@ impl HTMLIFrameElement {
170164
}
171165
}
172166

173-
pub fn update_subpage_id(&self, new_subpage_id: SubpageId, new_pipeline_id: PipelineId) {
174-
self.subpage_id.set(Some(new_subpage_id));
167+
pub fn update_pipeline_id(&self, new_pipeline_id: PipelineId) {
175168
self.pipeline_id.set(Some(new_pipeline_id));
176169

177170
let mut blocker = self.load_blocker.borrow_mut();
@@ -186,7 +179,6 @@ impl HTMLIFrameElement {
186179
HTMLIFrameElement {
187180
htmlelement: HTMLElement::new_inherited(localName, prefix, document),
188181
pipeline_id: Cell::new(None),
189-
subpage_id: Cell::new(None),
190182
sandbox: Default::default(),
191183
sandbox_allowance: Cell::new(None),
192184
load_blocker: DOMRefCell::new(None),
@@ -208,11 +200,6 @@ impl HTMLIFrameElement {
208200
self.pipeline_id.get()
209201
}
210202

211-
#[inline]
212-
pub fn subpage_id(&self) -> Option<SubpageId> {
213-
self.subpage_id.get()
214-
}
215-
216203
pub fn change_visibility_status(&self, visibility: bool) {
217204
if self.visibility.get() != visibility {
218205
self.visibility.set(visibility);
@@ -270,11 +257,11 @@ impl HTMLIFrameElement {
270257
}
271258

272259
pub fn get_content_window(&self) -> Option<Root<Window>> {
273-
self.subpage_id.get().and_then(|subpage_id| {
260+
self.pipeline_id.get().and_then(|pipeline_id| {
274261
let window = window_from_node(self);
275262
let window = window.r();
276263
let browsing_context = window.browsing_context();
277-
browsing_context.find_child_by_subpage(subpage_id)
264+
browsing_context.find_child_by_id(pipeline_id)
278265
})
279266
}
280267

@@ -659,12 +646,11 @@ impl VirtualMethods for HTMLIFrameElement {
659646
receiver.recv().unwrap()
660647
}
661648

662-
// Resetting the subpage id to None is required here so that
649+
// Resetting the pipeline_id to None is required here so that
663650
// if this iframe is subsequently re-added to the document
664651
// the load doesn't think that it's a navigation, but instead
665652
// a new iframe. Without this, the constellation gets very
666653
// confused.
667-
self.subpage_id.set(None);
668654
self.pipeline_id.set(None);
669655
}
670656
}

‎components/script/dom/servohtmlparser.rs‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ use hyper::header::ContentType;
3030
use hyper::mime::{Mime, SubLevel, TopLevel};
3131
use hyper_serde::Serde;
3232
use js::jsapi::JSTracer;
33-
use msg::constellation_msg::{PipelineId, SubpageId};
33+
use msg::constellation_msg::PipelineId;
3434
use net_traits::{AsyncResponseListener, Metadata, NetworkError};
3535
use network_listener::PreInvoke;
3636
use parse::{Parser, ParserRef, TrustedParser};
@@ -67,19 +67,16 @@ pub struct ParserContext {
6767
is_synthesized_document: bool,
6868
/// The pipeline associated with this document.
6969
id: PipelineId,
70-
/// The subpage associated with this document.
71-
subpage: Option<SubpageId>,
7270
/// The URL for this document.
7371
url: Url,
7472
}
7573

7674
impl ParserContext {
77-
pub fn new(id: PipelineId, subpage: Option<SubpageId>, url: Url) -> ParserContext {
75+
pub fn new(id: PipelineId, url: Url) -> ParserContext {
7876
ParserContext {
7977
parser: None,
8078
is_synthesized_document: false,
8179
id: id,
82-
subpage: subpage,
8380
url: url,
8481
}
8582
}
@@ -102,7 +99,6 @@ impl AsyncResponseListener for ParserContext {
10299
let content_type =
103100
metadata.clone().and_then(|meta| meta.content_type).map(Serde::into_inner);
104101
let parser = match ScriptThread::page_headers_available(&self.id,
105-
self.subpage.as_ref(),
106102
metadata) {
107103
Some(parser) => parser,
108104
None => return,

‎components/script/dom/window.rs‎

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ use js::jsval::UndefinedValue;
5151
use js::rust::CompileOptionsWrapper;
5252
use js::rust::Runtime;
5353
use libc;
54-
use msg::constellation_msg::{FrameType, LoadData, PipelineId, SubpageId, WindowSizeType};
54+
use msg::constellation_msg::{FrameType, LoadData, PipelineId, WindowSizeType};
5555
use net_traits::ResourceThreads;
5656
use net_traits::bluetooth_thread::BluetoothMethodMsg;
5757
use net_traits::image_cache_thread::{ImageCacheChan, ImageCacheThread};
@@ -200,16 +200,14 @@ pub struct Window {
200200
/// page changes.
201201
devtools_wants_updates: Cell<bool>,
202202

203-
next_subpage_id: Cell<SubpageId>,
204-
205203
/// Pending resize event, if any.
206204
resize_event: Cell<Option<(WindowSizeData, WindowSizeType)>>,
207205

208206
/// Pipeline id associated with this page.
209207
id: PipelineId,
210208

211-
/// Subpage id associated with this page, if any.
212-
parent_info: Option<(PipelineId, SubpageId, FrameType)>,
209+
/// Parent id associated with this page, if any.
210+
parent_info: Option<(PipelineId, FrameType)>,
213211

214212
/// Global static data related to the DOM.
215213
dom_static: GlobalStaticData,
@@ -335,11 +333,7 @@ impl Window {
335333
self.id
336334
}
337335

338-
pub fn subpage(&self) -> Option<SubpageId> {
339-
self.parent_info.map(|p| p.1)
340-
}
341-
342-
pub fn parent_info(&self) -> Option<(PipelineId, SubpageId, FrameType)> {
336+
pub fn parent_info(&self) -> Option<(PipelineId, FrameType)> {
343337
self.parent_info
344338
}
345339

@@ -1492,13 +1486,6 @@ impl Window {
14921486
WindowProxyHandler(self.dom_static.windowproxy_handler.0)
14931487
}
14941488

1495-
pub fn get_next_subpage_id(&self) -> SubpageId {
1496-
let subpage_id = self.next_subpage_id.get();
1497-
let SubpageId(id_num) = subpage_id;
1498-
self.next_subpage_id.set(SubpageId(id_num + 1));
1499-
subpage_id
1500-
}
1501-
15021489
pub fn get_pending_reflow_count(&self) -> u32 {
15031490
self.pending_reflow_count.get()
15041491
}
@@ -1612,7 +1599,7 @@ impl Window {
16121599
// https://html.spec.whatwg.org/multipage/#top-level-browsing-context
16131600
pub fn is_top_level(&self) -> bool {
16141601
match self.parent_info {
1615-
Some((_, _, FrameType::IFrame)) => false,
1602+
Some((_, FrameType::IFrame)) => false,
16161603
_ => true,
16171604
}
16181605
}
@@ -1676,7 +1663,7 @@ impl Window {
16761663
timer_event_chan: IpcSender<TimerEvent>,
16771664
layout_chan: Sender<Msg>,
16781665
id: PipelineId,
1679-
parent_info: Option<(PipelineId, SubpageId, FrameType)>,
1666+
parent_info: Option<(PipelineId, FrameType)>,
16801667
window_size: Option<WindowSizeData>)
16811668
-> Root<Window> {
16821669
let layout_rpc: Box<LayoutRPC> = {
@@ -1726,7 +1713,6 @@ impl Window {
17261713
page_clip_rect: Cell::new(max_rect()),
17271714
fragment_name: DOMRefCell::new(None),
17281715
resize_event: Cell::new(None),
1729-
next_subpage_id: Cell::new(SubpageId(0)),
17301716
layout_chan: layout_chan,
17311717
layout_rpc: layout_rpc,
17321718
window_size: Cell::new(window_size),

0 commit comments

Comments
 (0)