Browse Source

perf: avoid protocol registry redundant lookup (#41991)

* perf: avoid redundant map lookup in SimpleURLLoaderWrapper::GetURLLoaderFactoryForURL()

* perf: avoid redundant map lookup in InspectableWebContents::LoadNetworkResource()

* refactor: remove unused ProtocolRegistry::IsProtocolRegistered()

refactor: remove unused ProtocolRegistry::IsProtocolIntercepted()

* refactor: remove unused ProtocolRegistry::handlers()

* refactor: rename ProtocolRegistry::FindIntercepted()

refactor: rename ProtocolRegistry::FindRegistered()

similar semantics to base::Value::Find*()

* chore: follow Google C++ brace style

chore: use same variable names as in main
Charles Kerr 11 months ago
parent
commit
e2acdffe58

+ 2 - 2
shell/browser/api/electron_api_protocol.cc

@@ -232,7 +232,7 @@ bool Protocol::UnregisterProtocol(const std::string& scheme,
 }
 
 bool Protocol::IsProtocolRegistered(const std::string& scheme) {
-  return protocol_registry_->IsProtocolRegistered(scheme);
+  return protocol_registry_->FindRegistered(scheme) != nullptr;
 }
 
 ProtocolError Protocol::InterceptProtocol(ProtocolType type,
@@ -251,7 +251,7 @@ bool Protocol::UninterceptProtocol(const std::string& scheme,
 }
 
 bool Protocol::IsProtocolIntercepted(const std::string& scheme) {
-  return protocol_registry_->IsProtocolIntercepted(scheme);
+  return protocol_registry_->FindIntercepted(scheme) != nullptr;
 }
 
 v8::Local<v8::Promise> Protocol::IsProtocolHandled(const std::string& scheme,

+ 10 - 5
shell/browser/protocol_registry.cc

@@ -4,7 +4,6 @@
 
 #include "shell/browser/protocol_registry.h"
 
-#include "base/stl_util.h"
 #include "content/public/browser/web_contents.h"
 #include "electron/fuses.h"
 #include "shell/browser/electron_browser_context.h"
@@ -75,8 +74,11 @@ bool ProtocolRegistry::UnregisterProtocol(const std::string& scheme) {
   return handlers_.erase(scheme) != 0;
 }
 
-bool ProtocolRegistry::IsProtocolRegistered(const std::string& scheme) {
-  return base::Contains(handlers_, scheme);
+const HandlersMap::mapped_type* ProtocolRegistry::FindRegistered(
+    const std::string& scheme) const {
+  const auto& map = handlers_;
+  const auto iter = map.find(scheme);
+  return iter != std::end(map) ? &iter->second : nullptr;
 }
 
 bool ProtocolRegistry::InterceptProtocol(ProtocolType type,
@@ -89,8 +91,11 @@ bool ProtocolRegistry::UninterceptProtocol(const std::string& scheme) {
   return intercept_handlers_.erase(scheme) != 0;
 }
 
-bool ProtocolRegistry::IsProtocolIntercepted(const std::string& scheme) {
-  return base::Contains(intercept_handlers_, scheme);
+const HandlersMap::mapped_type* ProtocolRegistry::FindIntercepted(
+    const std::string& scheme) const {
+  const auto& map = intercept_handlers_;
+  const auto iter = map.find(scheme);
+  return iter != std::end(map) ? &iter->second : nullptr;
 }
 
 }  // namespace electron

+ 6 - 3
shell/browser/protocol_registry.h

@@ -33,19 +33,22 @@ class ProtocolRegistry {
   CreateNonNetworkNavigationURLLoaderFactory(const std::string& scheme);
 
   const HandlersMap& intercept_handlers() const { return intercept_handlers_; }
-  const HandlersMap& handlers() const { return handlers_; }
 
   bool RegisterProtocol(ProtocolType type,
                         const std::string& scheme,
                         const ProtocolHandler& handler);
   bool UnregisterProtocol(const std::string& scheme);
-  bool IsProtocolRegistered(const std::string& scheme);
+
+  [[nodiscard]] const HandlersMap::mapped_type* FindRegistered(
+      const std::string& scheme) const;
 
   bool InterceptProtocol(ProtocolType type,
                          const std::string& scheme,
                          const ProtocolHandler& handler);
   bool UninterceptProtocol(const std::string& scheme);
-  bool IsProtocolIntercepted(const std::string& scheme);
+
+  [[nodiscard]] const HandlersMap::mapped_type* FindIntercepted(
+      const std::string& scheme) const;
 
  private:
   friend class ElectronBrowserContext;

+ 5 - 7
shell/browser/ui/inspectable_web_contents.cc

@@ -674,7 +674,7 @@ void InspectableWebContents::LoadNetworkResource(DispatchCallback callback,
   resource_request.site_for_cookies = net::SiteForCookies::FromUrl(gurl);
   resource_request.headers.AddHeadersFromString(headers);
 
-  auto* protocol_registry = ProtocolRegistry::FromBrowserContext(
+  const auto* const protocol_registry = ProtocolRegistry::FromBrowserContext(
       GetDevToolsWebContents()->GetBrowserContext());
   NetworkResourceLoader::URLLoaderFactoryHolder url_loader_factory;
   if (gurl.SchemeIsFile()) {
@@ -683,14 +683,12 @@ void InspectableWebContents::LoadNetworkResource(DispatchCallback callback,
     url_loader_factory = network::SharedURLLoaderFactory::Create(
         std::make_unique<network::WrapperPendingSharedURLLoaderFactory>(
             std::move(pending_remote)));
-  } else if (protocol_registry->IsProtocolRegistered(gurl.scheme())) {
-    auto& protocol_handler = protocol_registry->handlers().at(gurl.scheme());
-    mojo::PendingRemote<network::mojom::URLLoaderFactory> pending_remote =
-        ElectronURLLoaderFactory::Create(protocol_handler.first,
-                                         protocol_handler.second);
+  } else if (const auto* const protocol_handler =
+                 protocol_registry->FindRegistered(gurl.scheme())) {
     url_loader_factory = network::SharedURLLoaderFactory::Create(
         std::make_unique<network::WrapperPendingSharedURLLoaderFactory>(
-            std::move(pending_remote)));
+            ElectronURLLoaderFactory::Create(protocol_handler->first,
+                                             protocol_handler->second)));
   } else {
     auto* partition = GetDevToolsWebContents()
                           ->GetBrowserContext()

+ 30 - 34
shell/common/api/electron_api_url_loader.cc

@@ -473,47 +473,43 @@ void SimpleURLLoaderWrapper::Cancel() {
 }
 scoped_refptr<network::SharedURLLoaderFactory>
 SimpleURLLoaderWrapper::GetURLLoaderFactoryForURL(const GURL& url) {
-  if (electron::IsUtilityProcess()) {
+  if (electron::IsUtilityProcess())
     return URLLoaderBundle::GetInstance()->GetSharedURLLoaderFactory();
-  }
+
   CHECK(browser_context_);
-  scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory;
-  auto* protocol_registry =
-      ProtocolRegistry::FromBrowserContext(browser_context_);
   // Explicitly handle intercepted protocols here, even though
   // ProxyingURLLoaderFactory would handle them later on, so that we can
   // correctly intercept file:// scheme URLs.
-  bool bypass_custom_protocol_handlers =
-      request_options_ & kBypassCustomProtocolHandlers;
-  if (!bypass_custom_protocol_handlers &&
-      protocol_registry->IsProtocolIntercepted(url.scheme())) {
-    auto& protocol_handler =
-        protocol_registry->intercept_handlers().at(url.scheme());
-    mojo::PendingRemote<network::mojom::URLLoaderFactory> pending_remote =
-        ElectronURLLoaderFactory::Create(protocol_handler.first,
-                                         protocol_handler.second);
-    url_loader_factory = network::SharedURLLoaderFactory::Create(
-        std::make_unique<network::WrapperPendingSharedURLLoaderFactory>(
-            std::move(pending_remote)));
-  } else if (!bypass_custom_protocol_handlers &&
-             protocol_registry->IsProtocolRegistered(url.scheme())) {
-    auto& protocol_handler = protocol_registry->handlers().at(url.scheme());
-    mojo::PendingRemote<network::mojom::URLLoaderFactory> pending_remote =
-        ElectronURLLoaderFactory::Create(protocol_handler.first,
-                                         protocol_handler.second);
-    url_loader_factory = network::SharedURLLoaderFactory::Create(
-        std::make_unique<network::WrapperPendingSharedURLLoaderFactory>(
-            std::move(pending_remote)));
-  } else if (url.SchemeIsFile()) {
-    mojo::PendingRemote<network::mojom::URLLoaderFactory> pending_remote =
-        AsarURLLoaderFactory::Create();
-    url_loader_factory = network::SharedURLLoaderFactory::Create(
+  if (const bool bypass = request_options_ & kBypassCustomProtocolHandlers;
+      !bypass) {
+    const auto scheme = url.scheme();
+    const auto* const protocol_registry =
+        ProtocolRegistry::FromBrowserContext(browser_context_);
+
+    if (const auto* const protocol_handler =
+            protocol_registry->FindIntercepted(scheme)) {
+      return network::SharedURLLoaderFactory::Create(
+          std::make_unique<network::WrapperPendingSharedURLLoaderFactory>(
+              ElectronURLLoaderFactory::Create(protocol_handler->first,
+                                               protocol_handler->second)));
+    }
+
+    if (const auto* const protocol_handler =
+            protocol_registry->FindRegistered(scheme)) {
+      return network::SharedURLLoaderFactory::Create(
+          std::make_unique<network::WrapperPendingSharedURLLoaderFactory>(
+              ElectronURLLoaderFactory::Create(protocol_handler->first,
+                                               protocol_handler->second)));
+    }
+  }
+
+  if (url.SchemeIsFile()) {
+    return network::SharedURLLoaderFactory::Create(
         std::make_unique<network::WrapperPendingSharedURLLoaderFactory>(
-            std::move(pending_remote)));
-  } else {
-    url_loader_factory = browser_context_->GetURLLoaderFactory();
+            AsarURLLoaderFactory::Create()));
   }
-  return url_loader_factory;
+
+  return browser_context_->GetURLLoaderFactory();
 }
 
 // static