libservo: Add WebView immediately to the Compositor (#35662)

When creating a `WebView`, let the Compositor know synchronously that
it exists. This allows the embedder to immediately call methods like
`WebView::focus()`. In addition remove messages associated with the
`WebViewDelegate::notify_ready_to_show()` method (and the method
itself), as now `WebView`s can be shown immediately.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>

components/compositing/compositor.rs

@@ -29,7 +29,7 @@ use euclid::{Box2D, Point2D, Rect, Scale, Size2D, Transform3D, Vector2D};
 use fnv::{FnvHashMap, FnvHashSet};
 use ipc_channel::ipc::{self, IpcSharedMemory};
 use libc::c_void;
-use log::{debug, error, info, trace, warn};
+use log::{debug, info, trace, warn};
 use pixels::{CorsStatus, Image, PixelFormat};
 use profile_traits::time::{self as profile_time, ProfilerCategory};
 use profile_traits::time_profile;
@@ -59,7 +59,7 @@ use webrender_traits::{
 };
 
 use crate::touch::{TouchHandler, TouchMoveAction, TouchMoveAllowed, TouchSequenceState};
-use crate::webview::{UnknownWebView, WebView, WebViewAlreadyExists, WebViewManager};
+use crate::webview::{UnknownWebView, WebView, WebViewManager};
 use crate::windowing::{self, EmbedderCoordinates, WebRenderDebugOption, WindowMethods};
 use crate::InitialCompositorState;
 
@@ -1007,42 +1007,33 @@ impl IOCompositor {
         }
     }
 
+    pub fn add_webview(&mut self, webview_id: WebViewId) {
+        let size = self.rendering_context.size2d().to_f32();
+        self.global.webviews.entry(webview_id).or_insert(WebView {
+            pipeline_id: None,
+            rect: Box2D::from_origin_and_size(Point2D::origin(), size),
+        });
+    }
+
     fn set_frame_tree_for_webview(&mut self, frame_tree: &SendableFrameTree) {
         debug!("{}: Setting frame tree for webview", frame_tree.pipeline.id);
 
-        let top_level_browsing_context_id = frame_tree.pipeline.top_level_browsing_context_id;
-        if let Some(webview) = self.global.webviews.get_mut(top_level_browsing_context_id) {
-            let new_pipeline_id = Some(frame_tree.pipeline.id);
-            if new_pipeline_id != webview.pipeline_id {
-                debug!(
-                    "{:?}: Updating webview from pipeline {:?} to {:?}",
-                    top_level_browsing_context_id, webview.pipeline_id, new_pipeline_id
-                );
-            }
-            webview.pipeline_id = new_pipeline_id;
-        } else {
-            let top_level_browsing_context_id = frame_tree.pipeline.top_level_browsing_context_id;
-            let pipeline_id = Some(frame_tree.pipeline.id);
-            debug!(
-                "{:?}: Creating new webview with pipeline {:?}",
-                top_level_browsing_context_id, pipeline_id
+        let webview_id = frame_tree.pipeline.top_level_browsing_context_id;
+        let Some(webview) = self.global.webviews.get_mut(webview_id) else {
+            warn!(
+                "Attempted to set frame tree on unknown WebView (perhaps closed?): {webview_id:?}"
+            );
+            return;
+        };
+
+        let new_pipeline_id = Some(frame_tree.pipeline.id);
+        if new_pipeline_id != webview.pipeline_id {
+            debug!(
+                "{webview_id:?}: Updating webview from pipeline {:?} to {new_pipeline_id:?}",
+                webview.pipeline_id
             );
-            let size = self.rendering_context.size2d().to_f32();
-            if let Err(WebViewAlreadyExists(webview_id)) = self.global.webviews.add(
-                top_level_browsing_context_id,
-                WebView {
-                    pipeline_id,
-                    rect: Box2D::from_origin_and_size(Point2D::origin(), size),
-                },
-            ) {
-                error!("{webview_id}: Creating webview that already exists");
-                return;
-            }
-            let msg = ConstellationMsg::WebViewOpened(top_level_browsing_context_id);
-            if let Err(e) = self.global.constellation_sender.send(msg) {
-                warn!("Sending event to constellation failed ({:?}).", e);
-            }
         }
+        webview.pipeline_id = new_pipeline_id;
 
         self.send_root_pipeline_display_list();
         self.create_or_update_pipeline_details_with_frame_tree(frame_tree, None);

components/compositing/webview.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 https://mozilla.org/MPL/2.0/. */
 
+use std::collections::hash_map::Entry;
 use std::collections::HashMap;
 
 use base::id::{PipelineId, WebViewId};
@@ -23,24 +24,10 @@ pub struct WebViewManager<WebView> {
     pub(crate) painting_order: Vec<WebViewId>,
 }
 
-#[derive(Clone, Copy, Debug, PartialEq)]
-pub struct WebViewAlreadyExists(pub WebViewId);
-
 #[derive(Clone, Copy, Debug, PartialEq)]
 pub struct UnknownWebView(pub WebViewId);
 
 impl<WebView> WebViewManager<WebView> {
-    pub fn add(
-        &mut self,
-        webview_id: WebViewId,
-        webview: WebView,
-    ) -> Result<&mut WebView, WebViewAlreadyExists> {
-        if self.webviews.contains_key(&webview_id) {
-            return Err(WebViewAlreadyExists(webview_id));
-        }
-        Ok(self.webviews.entry(webview_id).or_insert(webview))
-    }
-
     pub fn remove(&mut self, webview_id: WebViewId) -> Result<WebView, UnknownWebView> {
         self.painting_order.retain(|b| *b != webview_id);
         self.webviews
@@ -107,6 +94,10 @@ impl<WebView> WebViewManager<WebView> {
             .iter()
             .flat_map(move |webview_id| self.get(*webview_id).map(|b| (webview_id, b)))
     }
+
+    pub fn entry(&mut self, webview_id: WebViewId) -> Entry<'_, WebViewId, WebView> {
+        self.webviews.entry(webview_id)
+    }
 }
 
 #[cfg(test)]

components/constellation/constellation.rs

@@ -1370,12 +1370,6 @@ where
                     None,
                 );
             },
-            // A top level browsing context is created and opened in both constellation and
-            // compositor.
-            FromCompositorMsg::WebViewOpened(top_level_browsing_context_id) => {
-                self.embedder_proxy
-                    .send(EmbedderMsg::WebViewOpened(top_level_browsing_context_id));
-            },
             // Close a top level browsing context.
             FromCompositorMsg::CloseWebView(top_level_browsing_context_id) => {
                 self.handle_close_top_level_browsing_context(top_level_browsing_context_id);

components/constellation/tracing.rs

@@ -78,7 +78,6 @@ mod from_compositor {
                 Self::Reload(..) => target!("Reload"),
                 Self::LogEntry(..) => target!("LogEntry"),
                 Self::NewWebView(..) => target!("NewWebView"),
-                Self::WebViewOpened(..) => target!("WebViewOpened"),
                 Self::CloseWebView(..) => target!("CloseWebView"),
                 Self::SendError(..) => target!("SendError"),
                 Self::FocusWebView(..) => target!("FocusWebView"),
@@ -207,7 +206,6 @@ mod from_script {
                 Self::ShowContextMenu(..) => target_variant!("ShowContextMenu"),
                 Self::AllowNavigationRequest(..) => target_variant!("AllowNavigationRequest"),
                 Self::AllowOpeningWebView(..) => target_variant!("AllowOpeningWebView"),
-                Self::WebViewOpened(..) => target_variant!("WebViewOpened"),
                 Self::WebViewClosed(..) => target_variant!("WebViewClosed"),
                 Self::WebViewFocused(..) => target_variant!("WebViewFocused"),
                 Self::WebViewBlurred => target_variant!("WebViewBlurred"),

components/servo/examples/winit_minimal.rs

@@ -49,17 +49,6 @@ struct AppState {
 }
 
 impl ::servo::WebViewDelegate for AppState {
-    fn notify_ready_to_show(&self, webview: WebView) {
-        let rect = self
-            .window_delegate
-            .get_coordinates()
-            .get_viewport()
-            .to_f32();
-        webview.focus();
-        webview.move_resize(rect);
-        webview.raise_to_top(true);
-    }
-
     fn notify_new_frame_ready(&self, _: WebView) {
         self.window_delegate.window.request_redraw();
     }
@@ -67,6 +56,8 @@ impl ::servo::WebViewDelegate for AppState {
     fn request_open_auxiliary_webview(&self, parent_webview: WebView) -> Option<WebView> {
         let webview = self.servo.new_auxiliary_webview();
         webview.set_delegate(parent_webview.delegate());
+        webview.focus();
+        webview.raise_to_top(true);
         self.webviews.borrow_mut().push(webview.clone());
         Some(webview)
     }
@@ -124,10 +115,13 @@ impl ApplicationHandler<WakerEvent> for App {
             // Make a new WebView and assign the `AppState` as the delegate.
             let url = Url::parse("https://demo.servo.org/experiments/twgl-tunnel/")
                 .expect("Guaranteed by argument");
+
             let webview = app_state.servo.new_webview(url);
             webview.set_delegate(app_state.clone());
-            app_state.webviews.borrow_mut().push(webview);
+            webview.focus();
+            webview.raise_to_top(true);
 
+            app_state.webviews.borrow_mut().push(webview);
             *self = Self::Running(app_state);
         }
     }

components/servo/lib.rs

@@ -739,11 +739,6 @@ impl Servo {
                     let _ = response_sender.send(new_webview.map(|webview| webview.id()));
                 }
             },
-            EmbedderMsg::WebViewOpened(webview_id) => {
-                if let Some(webview) = self.get_webview_handle(webview_id) {
-                    webview.delegate().notify_ready_to_show(webview);
-                }
-            },
             EmbedderMsg::WebViewClosed(webview_id) => {
                 if let Some(webview) = self.get_webview_handle(webview_id) {
                     webview.delegate().notify_closed(webview);

components/servo/webview.rs

@@ -96,8 +96,10 @@ impl WebView {
         constellation_proxy: &ConstellationProxy,
         compositor: Rc<RefCell<IOCompositor>>,
     ) -> Self {
+        let id = WebViewId::new();
+        compositor.borrow_mut().add_webview(id);
         Self(Rc::new(RefCell::new(WebViewInner {
-            id: WebViewId::new(),
+            id,
             constellation_proxy: constellation_proxy.clone(),
             compositor,
             delegate: Rc::new(DefaultWebViewDelegate),

components/servo/webview_delegate.rs

@@ -283,8 +283,6 @@ pub trait WebViewDelegate {
     /// favicon [`Url`] can accessed via [`WebView::favicon_url`].
     fn notify_favicon_url_changed(&self, _webview: WebView, _: Url) {}
 
-    /// A [`WebView`] was created and is now ready to show in the user interface.
-    fn notify_ready_to_show(&self, _webview: WebView) {}
     /// Notify the embedder that it needs to present a new frame.
     fn notify_new_frame_ready(&self, _webview: WebView) {}
     /// The history state has changed.

components/shared/compositing/constellation_msg.rs

@@ -53,8 +53,6 @@ pub enum ConstellationMsg {
     LogEntry(Option<TopLevelBrowsingContextId>, Option<String>, LogEntry),
     /// Create a new top level browsing context.
     NewWebView(ServoUrl, TopLevelBrowsingContextId),
-    /// A top level browsing context is created in both constellation and compositor.
-    WebViewOpened(TopLevelBrowsingContextId),
     /// Close a top level browsing context.
     CloseWebView(TopLevelBrowsingContextId),
     /// Panic a top level browsing context.
@@ -103,7 +101,6 @@ impl ConstellationMsg {
             Reload(..) => "Reload",
             LogEntry(..) => "LogEntry",
             NewWebView(..) => "NewWebView",
-            WebViewOpened(..) => "WebViewOpened",
             CloseWebView(..) => "CloseWebView",
             FocusWebView(..) => "FocusWebView",
             BlurWebView => "BlurWebView",

components/shared/embedder/lib.rs

@@ -197,8 +197,6 @@ pub enum EmbedderMsg {
     AllowNavigationRequest(WebViewId, PipelineId, ServoUrl),
     /// Whether or not to allow script to open a new tab/browser
     AllowOpeningWebView(WebViewId, IpcSender<Option<WebViewId>>),
-    /// A webview was created.
-    WebViewOpened(WebViewId),
     /// A webview was destroyed.
     WebViewClosed(WebViewId),
     /// A webview gained focus for keyboard events.
@@ -307,7 +305,6 @@ impl Debug for EmbedderMsg {
             EmbedderMsg::ShowIME(..) => write!(f, "ShowIME"),
             EmbedderMsg::HideIME(..) => write!(f, "HideIME"),
             EmbedderMsg::AllowOpeningWebView(..) => write!(f, "AllowOpeningWebView"),
-            EmbedderMsg::WebViewOpened(..) => write!(f, "WebViewOpened"),
             EmbedderMsg::WebViewClosed(..) => write!(f, "WebViewClosed"),
             EmbedderMsg::WebViewFocused(..) => write!(f, "WebViewFocused"),
             EmbedderMsg::WebViewBlurred => write!(f, "WebViewBlurred"),

ports/servoshell/desktop/app_state.rs

@@ -109,6 +109,10 @@ impl RunningAppState {
     pub(crate) fn new_toplevel_webview(self: &Rc<Self>, url: Url) {
         let webview = self.servo().new_webview(url);
         webview.set_delegate(self.clone());
+
+        webview.focus();
+        webview.raise_to_top(true);
+
         self.add(webview);
     }
 
@@ -464,13 +468,12 @@ impl WebViewDelegate for RunningAppState {
     ) -> Option<servo::WebView> {
         let webview = self.servo.new_auxiliary_webview();
         webview.set_delegate(parent_webview.delegate());
-        self.add(webview.clone());
-        Some(webview)
-    }
 
-    fn notify_ready_to_show(&self, webview: servo::WebView) {
         webview.focus();
         webview.raise_to_top(true);
+
+        self.add(webview.clone());
+        Some(webview)
     }
 
     fn notify_closed(&self, webview: servo::WebView) {

ports/servoshell/egl/app_state.rs

@@ -127,10 +127,6 @@ impl WebViewDelegate for RunningAppState {
             .notify_load_status_changed(load_status);
     }
 
-    fn notify_ready_to_show(&self, webview: WebView) {
-        webview.focus();
-    }
-
     fn notify_closed(&self, webview: WebView) {
         {
             let mut inner_mut = self.inner_mut();
@@ -308,6 +304,7 @@ impl RunningAppState {
     pub(crate) fn new_toplevel_webview(self: &Rc<Self>, url: Url) {
         let webview = self.servo.new_webview(url);
         webview.set_delegate(self.clone());
+        webview.focus();
         self.add(webview.clone());
     }