libservo: Refactor ipc-channel default response logic (#35624)

Signed-off-by: Delan Azabani <dazabani@igalia.com>

Cargo.lock

@@ -4330,6 +4330,7 @@ dependencies = [
  "arboard",
  "background_hang_monitor",
  "base",
+ "bincode",
  "bluetooth",
  "bluetooth_traits",
  "canvas",
@@ -4366,6 +4367,7 @@ dependencies = [
  "script",
  "script_layout_interface",
  "script_traits",
+ "serde",
  "servo-media",
  "servo-media-dummy",
  "servo-media-gstreamer",

components/servo/Cargo.toml

@@ -58,6 +58,7 @@ webgpu = [
 [dependencies]
 background_hang_monitor = { path = "../background_hang_monitor" }
 base = { workspace = true }
+bincode = { workspace = true }
 bluetooth = { path = "../bluetooth", optional = true }
 bluetooth_traits = { workspace = true, optional = true }
 canvas = { path = "../canvas", default-features = false }
@@ -91,6 +92,7 @@ profile_traits = { workspace = true }
 script = { path = "../script" }
 script_layout_interface = { workspace = true }
 script_traits = { workspace = true }
+serde = { workspace = true }
 servo-media = { workspace = true }
 servo-media-dummy = { workspace = true }
 servo-media-gstreamer = { workspace = true, optional = true }

components/servo/lib.rs

@@ -766,11 +766,7 @@ impl Servo {
             },
             EmbedderMsg::AllowUnload(webview_id, response_sender) => {
                 if let Some(webview) = self.get_webview_handle(webview_id) {
-                    let request = AllowOrDenyRequest {
-                        response_sender,
-                        response_sent: false,
-                        default_response: AllowOrDeny::Allow,
-                    };
+                    let request = AllowOrDenyRequest::new(response_sender, AllowOrDeny::Allow);
                     webview.delegate().request_unload(webview, request);
                 }
             },
@@ -836,11 +832,7 @@ impl Servo {
                 web_resource_request,
                 response_sender,
             ) => {
-                let web_resource_load = WebResourceLoad {
-                    request: web_resource_request,
-                    response_sender,
-                    intercepted: false,
-                };
+                let web_resource_load = WebResourceLoad::new(web_resource_request, response_sender);
                 match webview_id.and_then(|webview_id| self.get_webview_handle(webview_id)) {
                     Some(webview) => webview
                         .delegate()
@@ -880,12 +872,8 @@ impl Servo {
                 }
             },
             EmbedderMsg::RequestAuthentication(webview_id, url, for_proxy, response_sender) => {
-                let authentication_request = AuthenticationRequest {
-                    url: url.into_url(),
-                    for_proxy,
-                    response_sender,
-                    response_sent: false,
-                };
+                let authentication_request =
+                    AuthenticationRequest::new(url.into_url(), for_proxy, response_sender);
                 if let Some(webview) = self.get_webview_handle(webview_id) {
                     webview
                         .delegate()
@@ -896,11 +884,10 @@ impl Servo {
                 if let Some(webview) = self.get_webview_handle(webview_id) {
                     let permission_request = PermissionRequest {
                         requested_feature,
-                        allow_deny_request: AllowOrDenyRequest {
+                        allow_deny_request: AllowOrDenyRequest::new(
                             response_sender,
-                            response_sent: false,
-                            default_response: AllowOrDeny::Deny,
-                        },
+                            AllowOrDeny::Deny,
+                        ),
                     };
                     webview
                         .delegate()
@@ -942,11 +929,7 @@ impl Servo {
             EmbedderMsg::RequestDevtoolsConnection(response_sender) => {
                 self.delegate().request_devtools_connection(
                     self,
-                    AllowOrDenyRequest {
-                        response_sender,
-                        response_sent: false,
-                        default_response: AllowOrDeny::Deny,
-                    },
+                    AllowOrDenyRequest::new(response_sender, AllowOrDeny::Deny),
                 );
             },
             EmbedderMsg::PlayGamepadHapticEffect(

components/servo/webview_delegate.rs

@@ -14,6 +14,7 @@ use embedder_traits::{
 };
 use ipc_channel::ipc::IpcSender;
 use keyboard_types::KeyboardEvent;
+use serde::Serialize;
 use url::Url;
 use webrender_api::units::{DeviceIntPoint, DeviceIntRect, DeviceIntSize};
 
@@ -60,6 +61,46 @@ impl Drop for NavigationRequest {
     }
 }
 
+/// Sends a response over an IPC channel, or a default response on [`Drop`] if no response was sent.
+pub(crate) struct IpcResponder<T: Serialize> {
+    response_sender: IpcSender<T>,
+    response_sent: bool,
+    /// Always present, except when taken by [`Drop`].
+    default_response: Option<T>,
+}
+
+impl<T: Serialize> IpcResponder<T> {
+    pub(crate) fn new(response_sender: IpcSender<T>, default_response: T) -> Self {
+        Self {
+            response_sender,
+            response_sent: false,
+            default_response: Some(default_response),
+        }
+    }
+
+    pub(crate) fn send(&mut self, response: T) -> bincode::Result<()> {
+        let result = self.response_sender.send(response);
+        self.response_sent = true;
+        result
+    }
+
+    pub(crate) fn into_inner(self) -> IpcSender<T> {
+        self.response_sender.clone()
+    }
+}
+
+impl<T: Serialize> Drop for IpcResponder<T> {
+    fn drop(&mut self) {
+        if !self.response_sent {
+            let response = self
+                .default_response
+                .take()
+                .expect("Guaranteed by inherent impl");
+            let _ = self.response_sender.send(response);
+        }
+    }
+}
+
 /// A permissions request for a [`WebView`] The embedder should allow or deny the request,
 /// either by reading a cached value or querying the user for permission via the user
 /// interface.
@@ -82,29 +123,22 @@ impl PermissionRequest {
     }
 }
 
-pub struct AllowOrDenyRequest {
-    pub(crate) response_sender: IpcSender<AllowOrDeny>,
-    pub(crate) response_sent: bool,
-    pub(crate) default_response: AllowOrDeny,
-}
+pub struct AllowOrDenyRequest(IpcResponder<AllowOrDeny>);
 
 impl AllowOrDenyRequest {
+    pub(crate) fn new(
+        response_sender: IpcSender<AllowOrDeny>,
+        default_response: AllowOrDeny,
+    ) -> Self {
+        Self(IpcResponder::new(response_sender, default_response))
+    }
+
     pub fn allow(mut self) {
-        let _ = self.response_sender.send(AllowOrDeny::Allow);
-        self.response_sent = true;
+        let _ = self.0.send(AllowOrDeny::Allow);
     }
 
     pub fn deny(mut self) {
-        let _ = self.response_sender.send(AllowOrDeny::Deny);
-        self.response_sent = true;
-    }
-}
-
-impl Drop for AllowOrDenyRequest {
-    fn drop(&mut self) {
-        if !self.response_sent {
-            let _ = self.response_sender.send(self.default_response);
-        }
+        let _ = self.0.send(AllowOrDeny::Deny);
     }
 }
 
@@ -114,11 +148,22 @@ impl Drop for AllowOrDenyRequest {
 pub struct AuthenticationRequest {
     pub(crate) url: Url,
     pub(crate) for_proxy: bool,
-    pub(crate) response_sender: IpcSender<Option<AuthenticationResponse>>,
-    pub(crate) response_sent: bool,
+    pub(crate) responder: IpcResponder<Option<AuthenticationResponse>>,
 }
 
 impl AuthenticationRequest {
+    pub(crate) fn new(
+        url: Url,
+        for_proxy: bool,
+        response_sender: IpcSender<Option<AuthenticationResponse>>,
+    ) -> Self {
+        Self {
+            url,
+            for_proxy,
+            responder: IpcResponder::new(response_sender, None),
+        }
+    }
+
     /// The URL of the request that triggered this authentication.
     pub fn url(&self) -> &Url {
         &self.url
@@ -130,17 +175,8 @@ impl AuthenticationRequest {
     /// Respond to the [`AuthenticationRequest`] with the given username and password.
     pub fn authenticate(mut self, username: String, password: String) {
         let _ = self
-            .response_sender
+            .responder
             .send(Some(AuthenticationResponse { username, password }));
-        self.response_sent = true;
-    }
-}
-
-impl Drop for AuthenticationRequest {
-    fn drop(&mut self) {
-        if !self.response_sent {
-            let _ = self.response_sender.send(None);
-        }
     }
 }
 
@@ -149,11 +185,20 @@ impl Drop for AuthenticationRequest {
 /// by calling [`WebResourceLoad::intercept`].
 pub struct WebResourceLoad {
     pub request: WebResourceRequest,
-    pub(crate) response_sender: IpcSender<WebResourceResponseMsg>,
-    pub(crate) intercepted: bool,
+    pub(crate) responder: IpcResponder<WebResourceResponseMsg>,
 }
 
 impl WebResourceLoad {
+    pub(crate) fn new(
+        web_resource_request: WebResourceRequest,
+        response_sender: IpcSender<WebResourceResponseMsg>,
+    ) -> Self {
+        Self {
+            request: web_resource_request,
+            responder: IpcResponder::new(response_sender, WebResourceResponseMsg::DoNotIntercept),
+        }
+    }
+
     /// The [`WebResourceRequest`] associated with this [`WebResourceLoad`].
     pub fn request(&self) -> &WebResourceRequest {
         &self.request
@@ -161,28 +206,15 @@ impl WebResourceLoad {
     /// Intercept this [`WebResourceLoad`] and control the response via the returned
     /// [`InterceptedWebResourceLoad`].
     pub fn intercept(mut self, response: WebResourceResponse) -> InterceptedWebResourceLoad {
-        let _ = self
-            .response_sender
-            .send(WebResourceResponseMsg::Start(response));
-        self.intercepted = true;
+        let _ = self.responder.send(WebResourceResponseMsg::Start(response));
         InterceptedWebResourceLoad {
             request: self.request.clone(),
-            response_sender: self.response_sender.clone(),
+            response_sender: self.responder.into_inner(),
             finished: false,
         }
     }
 }
 
-impl Drop for WebResourceLoad {
-    fn drop(&mut self) {
-        if !self.intercepted {
-            let _ = self
-                .response_sender
-                .send(WebResourceResponseMsg::DoNotIntercept);
-        }
-    }
-}
-
 /// An intercepted web resource load. This struct allows the client to send an alternative response
 /// after calling [`WebResourceLoad::intercept`]. In order to send chunks of body data, the client
 /// must call [`InterceptedWebResourceLoad::send_body_data`]. When the interception is complete, the client