From cf59aa1948384e7fbdbeecc892a40e21dedfae39 Mon Sep 17 00:00:00 2001
From: Jonathan Schwender <55576758+jschwe@users.noreply.github.com>
Date: Fri, 25 Apr 2025 12:02:46 +0200
Subject: [PATCH] Return Result from ProtocolRegistry::register() (#36688)

Instead of returning true / false it's better to return a Result (even
if we continue ignoring possible error).

Testing: Doesn't change any behavior

Signed-off-by: Jonathan Schwender <schwenderjonathan@gmail.com>
---
 components/net/protocols/mod.rs | 32 +++++++++++++++++++++++++-------
 ports/servoshell/desktop/app.rs |  7 ++++---
 2 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/components/net/protocols/mod.rs b/components/net/protocols/mod.rs
index f8b989b9623..af1557f63e2 100644
--- a/components/net/protocols/mod.rs
+++ b/components/net/protocols/mod.rs
@@ -54,27 +54,45 @@ pub struct ProtocolRegistry {
     pub(crate) handlers: HashMap<String, Box<dyn ProtocolHandler>>, // Maps scheme -> handler
 }
 
+#[derive(Clone, Copy, Debug)]
+pub enum ProtocolRegisterError {
+    ForbiddenScheme,
+    SchemeAlreadyRegistered,
+}
+
 impl ProtocolRegistry {
     pub fn with_internal_protocols() -> Self {
         let mut registry = Self::default();
-        registry.register("data", DataProtocolHander::default());
-        registry.register("blob", BlobProtocolHander::default());
-        registry.register("file", FileProtocolHander::default());
+        // We just created a new registry, and know that we aren't using
+        // any forbidden schemes, so this should never panic.
+        registry
+            .register("data", DataProtocolHander::default())
+            .expect("Infallible");
+        registry
+            .register("blob", BlobProtocolHander::default())
+            .expect("Infallible");
+        registry
+            .register("file", FileProtocolHander::default())
+            .expect("Infallible");
         registry
     }
 
-    pub fn register(&mut self, scheme: &str, handler: impl ProtocolHandler + 'static) -> bool {
+    pub fn register(
+        &mut self,
+        scheme: &str,
+        handler: impl ProtocolHandler + 'static,
+    ) -> Result<(), ProtocolRegisterError> {
         if FORBIDDEN_SCHEMES.contains(&scheme) {
             error!("Protocol handler for '{scheme}' is not allowed to be registered.");
-            return false;
+            return Err(ProtocolRegisterError::ForbiddenScheme);
         }
 
         if let Entry::Vacant(entry) = self.handlers.entry(scheme.into()) {
             entry.insert(Box::new(handler));
-            true
+            Ok(())
         } else {
             error!("Protocol handler for '{scheme}' is already registered.");
-            false
+            Err(ProtocolRegisterError::SchemeAlreadyRegistered)
         }
     }
 
diff --git a/ports/servoshell/desktop/app.rs b/ports/servoshell/desktop/app.rs
index 4dde169fb4b..7bbac47cda3 100644
--- a/ports/servoshell/desktop/app.rs
+++ b/ports/servoshell/desktop/app.rs
@@ -122,12 +122,13 @@ impl App {
         }
 
         let mut protocol_registry = ProtocolRegistry::default();
-        protocol_registry.register(
+        let _ = protocol_registry.register(
             "urlinfo",
             protocols::urlinfo::UrlInfoProtocolHander::default(),
         );
-        protocol_registry.register("servo", protocols::servo::ServoProtocolHandler::default());
-        protocol_registry.register(
+        let _ =
+            protocol_registry.register("servo", protocols::servo::ServoProtocolHandler::default());
+        let _ = protocol_registry.register(
             "resource",
             protocols::resource::ResourceProtocolHandler::default(),
         );