refactor: reduce coupling in electron::api::Protocol (#46182)

* refactor: decouple api::Protocol from ElectronBrowserContext

now they do not know about each other

Co-authored-by: Charles Kerr <charles@charleskerr.com>

* refactor: make electron::api::ProtocolError private

Co-authored-by: Charles Kerr <charles@charleskerr.com>

* refactor: remove unused isolate arg in Protocol constructor

Co-authored-by: Charles Kerr <charles@charleskerr.com>

* refactor: use =default for trivial destructor

Co-authored-by: Charles Kerr <charles@charleskerr.com>

---------

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Charles Kerr <charles@charleskerr.com>
This commit is contained in:
trop[bot] 2025-03-21 12:54:11 -05:00 committed by GitHub
parent 3772e267c3
commit 33bde96d73
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 54 additions and 58 deletions

View file

@ -14,7 +14,6 @@
#include "gin/handle.h"
#include "gin/object_template_builder.h"
#include "shell/browser/browser.h"
#include "shell/browser/electron_browser_context.h"
#include "shell/browser/protocol_registry.h"
#include "shell/common/gin_converters/callback_converter.h"
#include "shell/common/gin_converters/net_converter.h"
@ -194,41 +193,39 @@ const char* const kBuiltinSchemes[] = {
"about", "file", "http", "https", "data", "filesystem",
};
} // namespace
Protocol::Protocol(ProtocolRegistry* protocol_registry)
: protocol_registry_{protocol_registry} {}
// Convert error code to string.
constexpr std::string_view ErrorCodeToString(ProtocolError error) {
// static
std::string_view Protocol::ErrorCodeToString(Error error) {
switch (error) {
case ProtocolError::kRegistered:
case Error::kRegistered:
return "The scheme has been registered";
case ProtocolError::kNotRegistered:
case Error::kNotRegistered:
return "The scheme has not been registered";
case ProtocolError::kIntercepted:
case Error::kIntercepted:
return "The scheme has been intercepted";
case ProtocolError::kNotIntercepted:
case Error::kNotIntercepted:
return "The scheme has not been intercepted";
default:
return "Unexpected error";
}
}
} // namespace
Protocol::Protocol(v8::Isolate* isolate, ProtocolRegistry* protocol_registry)
: protocol_registry_(protocol_registry) {}
Protocol::~Protocol() = default;
ProtocolError Protocol::RegisterProtocol(ProtocolType type,
Protocol::Error Protocol::RegisterProtocol(ProtocolType type,
const std::string& scheme,
const ProtocolHandler& handler) {
bool added = protocol_registry_->RegisterProtocol(type, scheme, handler);
return added ? ProtocolError::kOK : ProtocolError::kRegistered;
return added ? Error::kOK : Error::kRegistered;
}
bool Protocol::UnregisterProtocol(const std::string& scheme,
gin::Arguments* args) {
bool removed = protocol_registry_->UnregisterProtocol(scheme);
HandleOptionalCallback(
args, removed ? ProtocolError::kOK : ProtocolError::kNotRegistered);
HandleOptionalCallback(args, removed ? Error::kOK : Error::kNotRegistered);
return removed;
}
@ -236,18 +233,17 @@ bool Protocol::IsProtocolRegistered(const std::string& scheme) {
return protocol_registry_->FindRegistered(scheme) != nullptr;
}
ProtocolError Protocol::InterceptProtocol(ProtocolType type,
Protocol::Error Protocol::InterceptProtocol(ProtocolType type,
const std::string& scheme,
const ProtocolHandler& handler) {
bool added = protocol_registry_->InterceptProtocol(type, scheme, handler);
return added ? ProtocolError::kOK : ProtocolError::kIntercepted;
return added ? Error::kOK : Error::kIntercepted;
}
bool Protocol::UninterceptProtocol(const std::string& scheme,
gin::Arguments* args) {
bool removed = protocol_registry_->UninterceptProtocol(scheme);
HandleOptionalCallback(
args, removed ? ProtocolError::kOK : ProtocolError::kNotIntercepted);
HandleOptionalCallback(args, removed ? Error::kOK : Error::kNotIntercepted);
return removed;
}
@ -275,15 +271,14 @@ v8::Local<v8::Promise> Protocol::IsProtocolHandled(const std::string& scheme,
base::Contains(kBuiltinSchemes, scheme));
}
void Protocol::HandleOptionalCallback(gin::Arguments* args,
ProtocolError error) {
void Protocol::HandleOptionalCallback(gin::Arguments* args, Error error) {
base::RepeatingCallback<void(v8::Local<v8::Value>)> callback;
if (args->GetNext(&callback)) {
util::EmitWarning(
args->isolate(),
"The callback argument of protocol module APIs is no longer needed.",
"ProtocolDeprecateCallback");
if (error == ProtocolError::kOK)
if (error == Error::kOK)
callback.Run(v8::Null(args->isolate()));
else
callback.Run(v8::Exception::Error(
@ -292,11 +287,9 @@ void Protocol::HandleOptionalCallback(gin::Arguments* args,
}
// static
gin::Handle<Protocol> Protocol::Create(
v8::Isolate* isolate,
ElectronBrowserContext* browser_context) {
return gin::CreateHandle(
isolate, new Protocol(isolate, browser_context->protocol_registry()));
gin::Handle<Protocol> Protocol::Create(v8::Isolate* isolate,
ProtocolRegistry* protocol_registry) {
return gin::CreateHandle(isolate, new Protocol{protocol_registry});
}
// static

View file

@ -22,7 +22,6 @@ class Handle;
namespace electron {
class ElectronBrowserContext;
class ProtocolRegistry;
namespace api {
@ -35,21 +34,12 @@ void AddServiceWorkerScheme(const std::string& scheme);
void RegisterSchemesAsPrivileged(gin_helper::ErrorThrower thrower,
v8::Local<v8::Value> val);
// Possible errors.
enum class ProtocolError {
kOK, // no error
kRegistered,
kNotRegistered,
kIntercepted,
kNotIntercepted,
};
// Protocol implementation based on network services.
class Protocol final : public gin::Wrappable<Protocol>,
public gin_helper::Constructible<Protocol> {
public:
static gin::Handle<Protocol> Create(v8::Isolate* isolate,
ElectronBrowserContext* browser_context);
ProtocolRegistry* protocol_registry);
// gin_helper::Constructible
static gin::Handle<Protocol> New(gin_helper::ErrorThrower thrower);
@ -63,21 +53,32 @@ class Protocol final : public gin::Wrappable<Protocol>,
const char* GetTypeName() override;
private:
Protocol(v8::Isolate* isolate, ProtocolRegistry* protocol_registry);
~Protocol() override;
// Possible errors.
enum class Error {
kOK, // no error
kRegistered,
kNotRegistered,
kIntercepted,
kNotIntercepted,
};
// Callback types.
using CompletionCallback =
base::RepeatingCallback<void(v8::Local<v8::Value>)>;
explicit Protocol(ProtocolRegistry* protocol_registry);
~Protocol() override = default;
[[nodiscard]] static std::string_view ErrorCodeToString(Error error);
// JS APIs.
ProtocolError RegisterProtocol(ProtocolType type,
Error RegisterProtocol(ProtocolType type,
const std::string& scheme,
const ProtocolHandler& handler);
bool UnregisterProtocol(const std::string& scheme, gin::Arguments* args);
bool IsProtocolRegistered(const std::string& scheme);
ProtocolError InterceptProtocol(ProtocolType type,
Error InterceptProtocol(ProtocolType type,
const std::string& scheme,
const ProtocolHandler& handler);
bool UninterceptProtocol(const std::string& scheme, gin::Arguments* args);
@ -92,21 +93,21 @@ class Protocol final : public gin::Wrappable<Protocol>,
bool RegisterProtocolFor(const std::string& scheme,
const ProtocolHandler& handler,
gin::Arguments* args) {
auto result = RegisterProtocol(type, scheme, handler);
const auto result = RegisterProtocol(type, scheme, handler);
HandleOptionalCallback(args, result);
return result == ProtocolError::kOK;
return result == Error::kOK;
}
template <ProtocolType type>
bool InterceptProtocolFor(const std::string& scheme,
const ProtocolHandler& handler,
gin::Arguments* args) {
auto result = InterceptProtocol(type, scheme, handler);
const auto result = InterceptProtocol(type, scheme, handler);
HandleOptionalCallback(args, result);
return result == ProtocolError::kOK;
return result == Error::kOK;
}
// Be compatible with old interface, which accepts optional callback.
void HandleOptionalCallback(gin::Arguments* args, ProtocolError error);
void HandleOptionalCallback(gin::Arguments* args, Error error);
// Weak pointer; the lifetime of the ProtocolRegistry is guaranteed to be
// longer than the lifetime of this JS interface.

View file

@ -553,7 +553,9 @@ Session::Session(v8::Isolate* isolate, ElectronBrowserContext* browser_context)
SessionPreferences::CreateForBrowserContext(browser_context);
protocol_.Reset(isolate, Protocol::Create(isolate, browser_context).ToV8());
protocol_.Reset(
isolate,
Protocol::Create(isolate, browser_context->protocol_registry()).ToV8());
browser_context->SetUserData(
kElectronApiSessionKey,