Auto merge of #21559 - mandreyel:pipeline-fields-to-browsingcontext, r=cbrewster

Frame should store some of the data that is currently in Pipeline #14692

<!-- Please describe your changes on the following line: -->
Apologies, meant to land it sooner but deadline at work got hectic.

So I moved the `Pipeline::{visible, is_private, parent_info}` fields (`size` was moved earlier) to `BrowsingContext`, and renamed them where appropriate (and did some minor refactoring on the side, hope that's alright).

This introduced some complications, because when a pipeline is spawned for a browsing context that does not yet exist, the browsing context won't be constructed until after pipeline has made its document active. Thus, values for the fields that used to be in `Pipeline` and are now in `BrowsingContext` could not be easily retrieved when constructing the `BrowsingContext` (since in most cases they were only available when spawning a pipeline).
For this reason, I've put these fields in `SessionHistoryChange` since one is always created and added to `Constellation::pending_changes` when a new pipeline is created, so it provides an easy way to forward the necessary values to new `BrowsingContext`s.

Though frankly I'm not sure I like expanding `SessionHistoryChange`'s purpose to serve as a crutch to construct browsing contexts, so a way to uncouple purposes would be to separately store the values for a to-be-created `BrowsingContext` in a collection of structs in `Constellation` and consume them when a new `BrowsingContext` is created. Here's a PoC: 6fa2160bcc.
I didn't include this by default because it introduces a little overhead. Perhaps `PendingBrowsingContextInfo` could be stored as an `Option<>` next to a `SessionHistoryChange` in `Constellation::pending_changes`? That'd uncouple the two structs but not incur any overhead.

I don't think it's finished, so I've marked some areas where I need input on small matters with `TODO(mandreyel)`, but the general idea is done. I'll be sure to squash commits when no further changes need be done!

---
<!-- 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 #14692.

<!-- Either: -->
- [x] These changes do not require tests because no new features or behaviour were introduced.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- 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/21559)
<!-- Reviewable:end -->

components/constellation/browsingcontext.rs

@@ -8,13 +8,31 @@ use pipeline::Pipeline;
 use std::collections::{HashMap, HashSet};
 use style_traits::CSSPixel;
 
+/// Because a browsing context is only constructed once the document that's
+/// going to be in it becomes active (i.e. not when a pipeline is spawned), some
+/// values needed in browsing context are not easily available at the point of
+/// constructing it. Thus, every time a pipeline is created for a browsing
+/// context which doesn't exist yet, these values needed for the new browsing
+/// context are stored here so that they may be available later.
+pub struct NewBrowsingContextInfo {
+    /// The parent pipeline that contains this browsing context. `None` if this
+    /// is a top level browsing context.
+    pub parent_pipeline_id: Option<PipelineId>,
+
+    /// Whether this browsing context is in private browsing mode.
+    pub is_private: bool,
+
+    /// Whether this browsing context should be treated as visible for the
+    /// purposes of scheduling and resource management.
+    pub is_visible: bool,
+}
+
 /// The constellation's view of a browsing context.
-/// Each browsing context has a session history, caused by
+/// Each browsing context has a session history, caused by navigation and
-/// navigation and traversing the history. Each browsing context has its
+/// traversing the history. Each browsing context has its current entry, plus
-/// current entry, plus past and future entries. The past is sorted
+/// past and future entries. The past is sorted chronologically, the future is
-/// chronologically, the future is sorted reverse chronologically:
+/// sorted reverse chronologically: in particular prev.pop() is the latest
-/// in particular prev.pop() is the latest past entry, and
+/// past entry, and next.pop() is the earliest future entry.
-/// next.pop() is the earliest future entry.
 pub struct BrowsingContext {
     /// The browsing context id.
     pub id: BrowsingContextId,
@@ -25,9 +43,22 @@ pub struct BrowsingContext {
     /// The size of the frame.
     pub size: Option<TypedSize2D<f32, CSSPixel>>,
 
+    /// Whether this browsing context is in private browsing mode.
+    pub is_private: bool,
+
+    /// Whether this browsing context should be treated as visible for the
+    /// purposes of scheduling and resource management.
+    pub is_visible: bool,
+
     /// The pipeline for the current session history entry.
     pub pipeline_id: PipelineId,
 
+    /// The parent pipeline that contains this browsing context. `None` if this
+    /// is a top level browsing context.
+    pub parent_pipeline_id: Option<PipelineId>,
+
+    /// All the pipelines that have been presented or will be presented in
+    /// this browsing context.
     pub pipelines: HashSet<PipelineId>,
 }
 
@@ -38,6 +69,9 @@ impl BrowsingContext {
         id: BrowsingContextId,
         top_level_id: TopLevelBrowsingContextId,
         pipeline_id: PipelineId,
+        parent_pipeline_id: Option<PipelineId>,
+        is_private: bool,
+        is_visible: bool,
     ) -> BrowsingContext {
         let mut pipelines = HashSet::new();
         pipelines.insert(pipeline_id);
@@ -45,8 +79,11 @@ impl BrowsingContext {
             id: id,
             top_level_id: top_level_id,
             size: None,
+            is_private: is_private,
+            is_visible: is_visible,
             pipeline_id: pipeline_id,
-            pipelines,
+            parent_pipeline_id: parent_pipeline_id,
+            pipelines: pipelines,
         }
     }
 

components/constellation/pipeline.rs

@@ -58,10 +58,6 @@ pub struct Pipeline {
     /// The ID of the top-level browsing context that contains this Pipeline.
     pub top_level_browsing_context_id: TopLevelBrowsingContextId,
 
-    /// The parent pipeline of this one. `None` if this is a root pipeline.
-    /// TODO: move this field to `BrowsingContext`.
-    pub parent_info: Option<PipelineId>,
-
     pub opener: Option<BrowsingContextId>,
 
     /// The event loop handling this pipeline.
@@ -85,14 +81,6 @@ pub struct Pipeline {
     /// The child browsing contexts of this pipeline (these are iframes in the document).
     pub children: Vec<BrowsingContextId>,
 
-    /// Whether this pipeline is in private browsing mode.
-    /// TODO: move this field to `BrowsingContext`.
-    pub is_private: bool,
-
-    /// Whether this pipeline should be treated as visible for the purposes of scheduling and
-    /// resource management.
-    pub visible: bool,
-
     /// The Load Data used to create this pipeline.
     pub load_data: LoadData,
 
@@ -119,7 +107,7 @@ pub struct InitialPipelineState {
 
     /// The ID of the parent pipeline and frame type, if any.
     /// If `None`, this is the root.
-    pub parent_info: Option<PipelineId>,
+    pub parent_pipeline_id: Option<PipelineId>,
 
     pub opener: Option<BrowsingContextId>,
 
@@ -171,8 +159,11 @@ pub struct InitialPipelineState {
     /// The ID of the pipeline namespace for this script thread.
     pub pipeline_namespace_id: PipelineNamespaceId,
 
-    /// Pipeline visibility to be inherited
+    /// Whether the browsing context in which pipeline is embedded is visible
-    pub prev_visibility: Option<bool>,
+    /// for the purposes of scheduling and resource management. This field is
+    /// only used to notify script and compositor threads after spawning
+    /// a pipeline.
+    pub prev_visibility: bool,
 
     /// Webrender api.
     pub webrender_api_sender: webrender_api::RenderApiSender,
@@ -180,9 +171,6 @@ pub struct InitialPipelineState {
     /// The ID of the document processed by this script thread.
     pub webrender_document: webrender_api::DocumentId,
 
-    /// Whether this pipeline is considered private.
-    pub is_private: bool,
-
     /// A channel to the WebGL thread.
     pub webgl_chan: Option<WebGLPipeline>,
 
@@ -216,7 +204,7 @@ impl Pipeline {
         let script_chan = match state.event_loop {
             Some(script_chan) => {
                 let new_layout_info = NewLayoutInfo {
-                    parent_info: state.parent_info,
+                    parent_info: state.parent_pipeline_id,
                     new_pipeline_id: state.id,
                     browsing_context_id: state.browsing_context_id,
                     top_level_browsing_context_id: state.top_level_browsing_context_id,
@@ -270,7 +258,7 @@ impl Pipeline {
                     id: state.id,
                     browsing_context_id: state.browsing_context_id,
                     top_level_browsing_context_id: state.top_level_browsing_context_id,
-                    parent_info: state.parent_info,
+                    parent_pipeline_id: state.parent_pipeline_id,
                     opener: state.opener,
                     script_to_constellation_chan: state.script_to_constellation_chan.clone(),
                     scheduler_chan: state.scheduler_chan,
@@ -317,14 +305,12 @@ impl Pipeline {
             state.id,
             state.browsing_context_id,
             state.top_level_browsing_context_id,
-            state.parent_info,
             state.opener,
             script_chan,
             pipeline_chan,
             state.compositor_proxy,
-            state.is_private,
             url,
-            state.prev_visibility.unwrap_or(true),
+            state.prev_visibility,
             state.load_data,
         ))
     }
@@ -335,21 +321,18 @@ impl Pipeline {
         id: PipelineId,
         browsing_context_id: BrowsingContextId,
         top_level_browsing_context_id: TopLevelBrowsingContextId,
-        parent_info: Option<PipelineId>,
         opener: Option<BrowsingContextId>,
         event_loop: Rc<EventLoop>,
         layout_chan: IpcSender<LayoutControlMsg>,
         compositor_proxy: CompositorProxy,
-        is_private: bool,
         url: ServoUrl,
-        visible: bool,
+        is_visible: bool,
         load_data: LoadData,
     ) -> Pipeline {
         let pipeline = Pipeline {
             id: id,
             browsing_context_id: browsing_context_id,
             top_level_browsing_context_id: top_level_browsing_context_id,
-            parent_info: parent_info,
             opener: opener,
             event_loop: event_loop,
             layout_chan: layout_chan,
@@ -357,14 +340,12 @@ impl Pipeline {
             url: url,
             children: vec![],
             running_animations: false,
-            visible: visible,
-            is_private: is_private,
             load_data: load_data,
             history_state_id: None,
             history_states: HashSet::new(),
         };
 
-        pipeline.notify_visibility();
+        pipeline.notify_visibility(is_visible);
 
         pipeline
     }
@@ -448,25 +429,16 @@ impl Pipeline {
     }
 
     /// Notify the script thread that this pipeline is visible.
-    fn notify_visibility(&self) {
+    pub fn notify_visibility(&self, is_visible: bool) {
         let script_msg =
-            ConstellationControlMsg::ChangeFrameVisibilityStatus(self.id, self.visible);
+            ConstellationControlMsg::ChangeFrameVisibilityStatus(self.id, is_visible);
-        let compositor_msg = CompositorMsg::PipelineVisibilityChanged(self.id, self.visible);
+        let compositor_msg = CompositorMsg::PipelineVisibilityChanged(self.id, is_visible);
         let err = self.event_loop.send(script_msg);
         if let Err(e) = err {
             warn!("Sending visibility change failed ({}).", e);
         }
         self.compositor_proxy.send(compositor_msg);
     }
-
-    /// Change the visibility of this pipeline.
-    pub fn change_visibility(&mut self, visible: bool) {
-        if visible == self.visible {
-            return;
-        }
-        self.visible = visible;
-        self.notify_visibility();
-    }
 }
 
 /// Creating a new pipeline may require creating a new event loop.
@@ -477,7 +449,7 @@ pub struct UnprivilegedPipelineContent {
     id: PipelineId,
     top_level_browsing_context_id: TopLevelBrowsingContextId,
     browsing_context_id: BrowsingContextId,
-    parent_info: Option<PipelineId>,
+    parent_pipeline_id: Option<PipelineId>,
     opener: Option<BrowsingContextId>,
     script_to_constellation_chan: ScriptToConstellationChan,
     layout_to_constellation_chan: IpcSender<LayoutMsg>,
@@ -526,7 +498,7 @@ impl UnprivilegedPipelineContent {
                 id: self.id,
                 browsing_context_id: self.browsing_context_id,
                 top_level_browsing_context_id: self.top_level_browsing_context_id,
-                parent_info: self.parent_info,
+                parent_info: self.parent_pipeline_id,
                 opener: self.opener,
                 control_chan: self.script_chan.clone(),
                 control_port: self.script_port,
@@ -553,7 +525,7 @@ impl UnprivilegedPipelineContent {
             self.id,
             self.top_level_browsing_context_id,
             self.load_data.url,
-            self.parent_info.is_some(),
+            self.parent_pipeline_id.is_some(),
             layout_pair,
             self.pipeline_port,
             self.layout_to_constellation_chan,

components/constellation/session_history.rs

@@ -2,6 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+use browsingcontext::NewBrowsingContextInfo;
 use msg::constellation_msg::{BrowsingContextId, HistoryStateId, PipelineId, TopLevelBrowsingContextId};
 use script_traits::LoadData;
 use servo_url::ServoUrl;
@@ -113,6 +114,10 @@ pub struct SessionHistoryChange {
 
     /// The old pipeline that the new pipeline should replace.
     pub replace: Option<NeedsToReload>,
+
+    /// Holds data for not-yet constructed browsing contexts that are not
+    /// easily available when they need to be constructed.
+    pub new_browsing_context_info: Option<NewBrowsingContextInfo>,
 }
 
 /// Represents a pipeline or discarded pipeline in a history entry.