Auto merge of #21705 - mandreyel:coalesce-script-to-constellation-msgs, r=paulrouget

Create ScriptMsg::GetBrowsingContextInfo

Script used to send two messages to constellation to first retrieve the id of the browsing context in which a pipeline resides and then its parent pipeline's id. This patch introduces a minor optimization to instead retrieve those fields in a single message.

Also, fixed a potential bug introduced in #21559 where if a browsing context wasn't found for a pipeline in response to `GetParentInfo` (which is now deleted), constellation sent nothing back, presumably causing the script thread to hang on the receiving channel.

---
<!-- 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 #21703 (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [x] These changes do not require tests because no new behaviour was 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/21705)
<!-- Reviewable:end -->

components/constellation/constellation.rs

@@ -1218,28 +1218,14 @@ where
             FromScriptMsg::TouchEventProcessed(result) => self
                 .compositor_proxy
                 .send(ToCompositorMsg::TouchEventProcessed(result)),
-            FromScriptMsg::GetBrowsingContextId(pipeline_id, sender) => {
+            FromScriptMsg::GetBrowsingContextInfo(pipeline_id, sender) => {
                 let result = self
                     .pipelines
                     .get(&pipeline_id)
-                    .map(|pipeline| pipeline.browsing_context_id);
+                    .and_then(|pipeline| self.browsing_contexts.get(&pipeline.browsing_context_id))
+                    .map(|ctx| (ctx.id, ctx.parent_pipeline_id));
                 if let Err(e) = sender.send(result) {
-                    warn!("Sending reply to get browsing context failed ({:?}).", e);
+                    warn!("Sending reply to get browsing context info failed ({:?}).", e);
-                }
-            },
-            // TODO(mandreyel): maybe change semantics of this message to
-            // reflect moving parent_info into BrowsingContext, i.e. message
-            // could be: GetParentInfo(BrowsingContextId, Sender)
-            FromScriptMsg::GetParentInfo(pipeline_id, sender) => {
-                let browsing_context = self.pipelines
-                    .get(&pipeline_id)
-                    .and_then(|pipeline| self.browsing_contexts.get(&pipeline.browsing_context_id));
-                if let Some(browsing_context) = browsing_context {
-                    if let Err(e) = sender.send(browsing_context.parent_pipeline_id) {
-                        warn!("Sending reply to get parent info failed ({:?}).", e);
-                    }
-                } else {
-                    warn!("GetParentInfo for pipeline {} with no browsing context.", pipeline_id)
                 }
             },
             FromScriptMsg::GetTopForBrowsingContext(browsing_context_id, sender) => {

components/script/script_thread.rs

@@ -2065,24 +2065,21 @@ impl ScriptThread {
         }
     }
 
-    fn ask_constellation_for_browsing_context_id(&self, pipeline_id: PipelineId) -> Option<BrowsingContextId> {
+    fn ask_constellation_for_browsing_context_info(
+        &self,
+        pipeline_id: PipelineId
+    ) -> Option<(BrowsingContextId, Option<PipelineId>)> {
         let (result_sender, result_receiver) = ipc::channel().unwrap();
-        let msg = ScriptMsg::GetBrowsingContextId(pipeline_id, result_sender);
+        let msg = ScriptMsg::GetBrowsingContextInfo(pipeline_id, result_sender);
         self.script_sender.send((pipeline_id, msg)).expect("Failed to send to constellation.");
-        result_receiver.recv().expect("Failed to get frame id from constellation.")
+        result_receiver.recv().expect("Failed to get browsing context info from constellation.")
     }
 
-    fn ask_constellation_for_parent_info(&self, pipeline_id: PipelineId) -> Option<PipelineId> {
+    fn ask_constellation_for_top_level_info(
-        let (result_sender, result_receiver) = ipc::channel().unwrap();
+        &self,
-        let msg = ScriptMsg::GetParentInfo(pipeline_id, result_sender);
+        sender_pipeline: PipelineId,
-        self.script_sender.send((pipeline_id, msg)).expect("Failed to send to constellation.");
+        browsing_context_id: BrowsingContextId
-        result_receiver.recv().expect("Failed to get frame id from constellation.")
+    ) -> Option<TopLevelBrowsingContextId> {
-    }
-
-    fn ask_constellation_for_top_level_info(&self,
-                                            sender_pipeline: PipelineId,
-                                            browsing_context_id: BrowsingContextId)
-                                            -> Option<TopLevelBrowsingContextId> {
         let (result_sender, result_receiver) = ipc::channel().unwrap();
         let msg = ScriptMsg::GetTopForBrowsingContext(browsing_context_id, result_sender);
         self.script_sender.send((sender_pipeline, msg)).expect("Failed to send to constellation.");
@@ -2095,25 +2092,29 @@ impl ScriptThread {
     // get the browsing context for the parent if there is one,
     // construct a new dissimilar-origin browsing context, add it
     // to the `window_proxies` map, and return it.
-    fn remote_window_proxy(&self,
+    fn remote_window_proxy(
-                           global_to_clone: &GlobalScope,
+        &self,
-                           top_level_browsing_context_id: TopLevelBrowsingContextId,
+        global_to_clone: &GlobalScope,
-                           pipeline_id: PipelineId,
+        top_level_browsing_context_id: TopLevelBrowsingContextId,
-                           opener: Option<BrowsingContextId>)
+        pipeline_id: PipelineId,
-                           -> Option<DomRoot<WindowProxy>>
+        opener: Option<BrowsingContextId>
-    {
+    ) -> Option<DomRoot<WindowProxy>> {
-        let browsing_context_id = self.ask_constellation_for_browsing_context_id(pipeline_id)?;
+        let (browsing_context_id, parent_pipeline_id) =
+            self.ask_constellation_for_browsing_context_info(pipeline_id)?;
         if let Some(window_proxy) = self.window_proxies.borrow().get(&browsing_context_id) {
             return Some(DomRoot::from_ref(window_proxy));
         }
-        let parent = self.ask_constellation_for_parent_info(pipeline_id).and_then(|parent_id| {
+
+        let parent = parent_pipeline_id.and_then(|parent_id| {
             self.remote_window_proxy(global_to_clone, top_level_browsing_context_id, parent_id, opener)
         });
-        let window_proxy = WindowProxy::new_dissimilar_origin(global_to_clone,
+        let window_proxy = WindowProxy::new_dissimilar_origin(
-                                                              browsing_context_id,
+            global_to_clone,
-                                                              top_level_browsing_context_id,
+            browsing_context_id,
-                                                              parent.r(),
+            top_level_browsing_context_id,
-                                                              opener);
+            parent.r(),
+            opener
+        );
         self.window_proxies.borrow_mut().insert(browsing_context_id, Dom::from_ref(&*window_proxy));
         Some(window_proxy)
     }

components/script_traits/script_msg.rs

@@ -108,15 +108,17 @@ pub enum ScriptMsg {
     Focus,
     /// Requests that the constellation retrieve the current contents of the clipboard
     GetClipboardContents(IpcSender<String>),
-    /// Get the browsing context id for a given pipeline.
-    GetBrowsingContextId(PipelineId, IpcSender<Option<BrowsingContextId>>),
-    /// Get the parent info for a given pipeline.
-    GetParentInfo(PipelineId, IpcSender<Option<PipelineId>>),
     /// Get the top-level browsing context info for a given browsing context.
     GetTopForBrowsingContext(
         BrowsingContextId,
         IpcSender<Option<TopLevelBrowsingContextId>>,
     ),
+    /// Get the browsing context id of the browsing context in which pipeline is
+    /// embedded and the parent pipeline id of that browsing context.
+    GetBrowsingContextInfo(
+        PipelineId,
+        IpcSender<Option<(BrowsingContextId, Option<PipelineId>)>>,
+    ),
     /// Get the nth child browsing context ID for a given browsing context, sorted in tree order.
     GetChildBrowsingContextId(
         BrowsingContextId,
@@ -201,8 +203,7 @@ impl fmt::Debug for ScriptMsg {
             CreateCanvasPaintThread(..) => "CreateCanvasPaintThread",
             Focus => "Focus",
             GetClipboardContents(..) => "GetClipboardContents",
-            GetBrowsingContextId(..) => "GetBrowsingContextId",
+            GetBrowsingContextInfo(..) => "GetBrowsingContextInfo",
-            GetParentInfo(..) => "GetParentInfo",
             GetTopForBrowsingContext(..) => "GetParentBrowsingContext",
             GetChildBrowsingContextId(..) => "GetChildBrowsingContextId",
             LoadComplete => "LoadComplete",