Skip to content

Commit 93414ce

Browse files
author
bors-servo
authored
Auto merge of #11698 - aneeshusa:remove-subpage-id, r=<try>
Excise SubpageId and use only PipelineIds <!-- Please describe your changes on the following line: --> 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. --- <!-- 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 #11694 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [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="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11698) <!-- Reviewable:end -->
2 parents bb53da6 + 56fbfd4 commit 93414ce

27 files changed

+231
-304
lines changed

‎components/constellation/constellation.rs‎

Lines changed: 50 additions & 74 deletions
Large diffs are not rendered by default.

‎components/constellation/pipeline.rs‎

Lines changed: 11 additions & 13 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 (containing_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 {
156-
containing_pipeline_id: containing_pipeline_id,
155+
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/global.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,10 +69,10 @@ impl<'a> GlobalRef<'a> {
6969
}
7070

7171
/// Get the `PipelineId` for this global scope.
72-
pub fn pipeline(&self) -> PipelineId {
72+
pub fn pipeline_id(&self) -> PipelineId {
7373
match *self {
74-
GlobalRef::Window(window) => window.pipeline(),
75-
GlobalRef::Worker(worker) => worker.pipeline(),
74+
GlobalRef::Window(window) => window.pipeline_id(),
75+
GlobalRef::Worker(worker) => worker.pipeline_id(),
7676
}
7777
}
7878

‎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: 4 additions & 4 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

@@ -152,18 +152,18 @@ impl BrowsingContext {
152152
old
153153
}
154154

155-
pub fn pipeline(&self) -> PipelineId {
155+
pub fn pipeline_id(&self) -> PipelineId {
156156
self.id
157157
}
158158

159159
pub fn push_child_context(&self, context: &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/console.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ impl Console {
1818
if let Some(chan) = global.devtools_chan() {
1919
let console_message = prepare_message(level, message);
2020
let devtools_message = ScriptToDevtoolsControlMsg::ConsoleAPI(
21-
global.pipeline(),
21+
global.pipeline_id(),
2222
console_message,
2323
global.get_worker_id());
2424
chan.send(devtools_message).unwrap();

‎components/script/dom/dedicatedworkerglobalscope.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ impl DedicatedWorkerGlobalScope {
243243
}
244244
}
245245

246-
pub fn pipeline(&self) -> PipelineId {
246+
pub fn pipeline_id(&self) -> PipelineId {
247247
self.id
248248
}
249249

‎components/script/dom/document.rs‎

Lines changed: 12 additions & 20 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};
@@ -641,7 +641,7 @@ impl Document {
641641
// Update the focus state for all elements in the focus chain.
642642
// https://html.spec.whatwg.org/multipage/#focus-chain
643643
if focus_type == FocusType::Element {
644-
let event = ConstellationMsg::Focus(self.window.pipeline());
644+
let event = ConstellationMsg::Focus(self.window.pipeline_id());
645645
self.window.constellation_chan().send(event).unwrap();
646646
}
647647
}
@@ -661,7 +661,7 @@ impl Document {
661661
pub fn send_title_to_compositor(&self) {
662662
let window = self.window();
663663
window.constellation_chan()
664-
.send(ConstellationMsg::SetTitle(window.pipeline(),
664+
.send(ConstellationMsg::SetTitle(window.pipeline_id(),
665665
Some(String::from(self.Title()))))
666666
.unwrap();
667667
}
@@ -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((containing_pipeline_id, subpage_id, _)) = self.window.parent_info() {
1346-
let event = ConstellationMsg::MozBrowserEvent(containing_pipeline_id,
1347-
Some(subpage_id),
1345+
if let Some((parent_pipeline_id, _)) = self.window.parent_info() {
1346+
let event = ConstellationMsg::MozBrowserEvent(parent_pipeline_id,
1347+
Some(self.window.pipeline_id()),
13481348
event);
13491349
self.window.constellation_chan().send(event).unwrap();
13501350
}
@@ -1367,7 +1367,7 @@ impl Document {
13671367
// TODO: Should tick animation only when document is visible
13681368
if !self.running_animation_callbacks.get() {
13691369
let event = ConstellationMsg::ChangeRunningAnimationsState(
1370-
self.window.pipeline(),
1370+
self.window.pipeline_id(),
13711371
AnimationState::AnimationCallbacksPresent);
13721372
self.window.constellation_chan().send(event).unwrap();
13731373
}
@@ -1405,7 +1405,7 @@ impl Document {
14051405
if self.animation_frame_list.borrow().is_empty() {
14061406
mem::swap(&mut *self.animation_frame_list.borrow_mut(),
14071407
&mut animation_frame_list);
1408-
let event = ConstellationMsg::ChangeRunningAnimationsState(self.window.pipeline(),
1408+
let event = ConstellationMsg::ChangeRunningAnimationsState(self.window.pipeline_id(),
14091409
AnimationState::NoAnimationCallbacksPresent);
14101410
self.window.constellation_chan().send(event).unwrap();
14111411
}
@@ -1469,7 +1469,7 @@ impl Document {
14691469
let loader = self.loader.borrow();
14701470
if !loader.is_blocked() && !loader.events_inhibited() {
14711471
let win = self.window();
1472-
let msg = MainThreadScriptMsg::DocumentLoadsComplete(win.pipeline());
1472+
let msg = MainThreadScriptMsg::DocumentLoadsComplete(win.pipeline_id());
14731473
win.main_thread_script_chan().send(msg).unwrap();
14741474
}
14751475
}
@@ -1567,7 +1567,7 @@ impl Document {
15671567
}
15681568

15691569
pub fn notify_constellation_load(&self) {
1570-
let pipeline_id = self.window.pipeline();
1570+
let pipeline_id = self.window.pipeline_id();
15711571
let load_event = ConstellationMsg::LoadComplete(pipeline_id);
15721572
self.window.constellation_chan().send(load_event).unwrap();
15731573
}
@@ -1581,19 +1581,11 @@ impl Document {
15811581
}
15821582

15831583
/// Find an iframe element in the document.
1584-
pub fn find_iframe(&self, subpage_id: SubpageId) -> Option<Root<HTMLIFrameElement>> {
1584+
pub fn find_iframe(&self, pipeline: PipelineId) -> Option<Root<HTMLIFrameElement>> {
15851585
self.upcast::<Node>()
15861586
.traverse_preorder()
15871587
.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>> {
1593-
self.upcast::<Node>()
1594-
.traverse_preorder()
1595-
.filter_map(Root::downcast::<HTMLIFrameElement>)
1596-
.find(|node| node.pipeline() == Some(pipeline))
1588+
.find(|node| node.pipeline_id() == Some(pipeline))
15971589
}
15981590

15991591
pub fn get_dom_loading(&self) -> u64 {

‎components/script/dom/history.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ impl History {
3838

3939
impl History {
4040
fn traverse_history(&self, direction: TraversalDirection) {
41-
let pipeline = self.window.pipeline();
41+
let pipeline = self.window.pipeline_id();
4242
let msg = ConstellationMsg::TraverseHistory(Some(pipeline), direction);
4343
let _ = self.window.constellation_chan().send(msg);
4444
}
@@ -47,7 +47,7 @@ impl History {
4747
impl HistoryMethods for History {
4848
// https://html.spec.whatwg.org/multipage/#dom-history-length
4949
fn Length(&self) -> u32 {
50-
let pipeline = self.window.pipeline();
50+
let pipeline = self.window.pipeline_id();
5151
let (sender, recv) = ipc::channel().expect("Failed to create channel to send jsh length.");
5252
let msg = ConstellationMsg::JointSessionHistoryLength(pipeline, sender);
5353
let _ = self.window.constellation_chan().send(msg);

0 commit comments

Comments
 (0)