Skip to content

Commit

Permalink
perf: avoid protocol registry redundant lookup (#41991)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
ckerr committed May 9, 2024
1 parent 865b049 commit e2acdff
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 51 deletions.
4 changes: 2 additions & 2 deletions shell/browser/api/electron_api_protocol.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
15 changes: 10 additions & 5 deletions shell/browser/protocol_registry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand All @@ -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
9 changes: 6 additions & 3 deletions shell/browser/protocol_registry.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
12 changes: 5 additions & 7 deletions shell/browser/ui/inspectable_web_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand All @@ -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()
Expand Down
64 changes: 30 additions & 34 deletions shell/common/api/electron_api_url_loader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit e2acdff

Please sign in to comment.