refactor: remove electron::WebRequestAPI interface (#48792)

* refactor: remove electron::WebRequestAPI interface

Remove the |electron::WebRequestAPI| interface class.
Use handles to the concrete class |electron::api::WebRequest| instead.

Prerequisite for https://github.com/electron/electron/pull/48762.

Two classes (electron::ProxyingURLLoaderFactory and electron::ProxyingWebSocket)
hold a handle to a WebRequest via |raw_ptr<electron::WebRequestAPI>|.
|electron::WebRequestAPI| is a pure virtual interface whose concrete impl is
|electron::api::WebRequest|.

This is a problem when migrating |electron::api::WebRequest| to cppgc:
we need to change those |raw_ptr<>|s to |cppgc::WeakPersistent<>| but
can't instantiate |cppgc::WeakPersistent<electron::WebRequestAPI>| as-is.
We also can't change it to inherit from |cppgc::GarbageCollectedMixin|,
since that causes problems when |electron::api::WebRequest| inherits from
both |electron::WebRequestAPI| and |cppgc::GarbageCollected|.

* refactor: use name web_request, not web_request_api

* refactor: make ProxyingURLLoaderFactory::web_request() private

* chore: make linter happy by fixing whitespace
This commit is contained in:
Charles Kerr
2025-11-06 02:05:50 -06:00
committed by GitHub
parent 27bea2576e
commit f6ffb55c72
8 changed files with 64 additions and 124 deletions

View File

@@ -88,7 +88,7 @@ ProxyingURLLoaderFactory::InProgressRequest::~InProgressRequest() {
// This is important to ensure that no outstanding blocking requests continue
// to reference state owned by this object.
if (info_) {
factory_->web_request_api()->OnRequestWillBeDestroyed(&info_.value());
factory_->web_request()->OnRequestWillBeDestroyed(&info_.value());
}
if (on_before_send_headers_callback_) {
std::move(on_before_send_headers_callback_)
@@ -147,7 +147,7 @@ void ProxyingURLLoaderFactory::InProgressRequest::RestartInternal() {
weak_factory_.GetWeakPtr());
}
redirect_url_ = GURL();
int result = factory_->web_request_api()->OnBeforeRequest(
int result = factory_->web_request()->OnBeforeRequest(
&info_.value(), request_, continuation, &redirect_url_);
if (result == net::ERR_BLOCKED_BY_CLIENT) {
// The request was cancelled synchronously. Dispatch an error notification
@@ -293,8 +293,8 @@ void ProxyingURLLoaderFactory::InProgressRequest::OnComplete(
}
target_client_->OnComplete(status);
factory_->web_request_api()->OnCompleted(&info_.value(), request_,
status.error_code);
factory_->web_request()->OnCompleted(&info_.value(), request_,
status.error_code);
// Deletes |this|.
factory_->RemoveRequest(network_service_request_id_, request_id_);
@@ -441,7 +441,7 @@ void ProxyingURLLoaderFactory::InProgressRequest::ContinueToBeforeSendHeaders(
auto continuation = base::BindRepeating(
&InProgressRequest::ContinueToSendHeaders, weak_factory_.GetWeakPtr());
// Note: In Electron onBeforeSendHeaders is called for all protocols.
int result = factory_->web_request_api()->OnBeforeSendHeaders(
int result = factory_->web_request()->OnBeforeSendHeaders(
&info_.value(), request_, continuation, &request_.headers);
if (result == net::ERR_BLOCKED_BY_CLIENT) {
@@ -555,8 +555,8 @@ void ProxyingURLLoaderFactory::InProgressRequest::ContinueToSendHeaders(
proxied_client_receiver_.Resume();
// Note: In Electron onSendHeaders is called for all protocols.
factory_->web_request_api()->OnSendHeaders(&info_.value(), request_,
request_.headers);
factory_->web_request()->OnSendHeaders(&info_.value(), request_,
request_.headers);
if (!current_request_uses_header_client_)
ContinueToStartRequest(net::OK);
@@ -598,8 +598,8 @@ void ProxyingURLLoaderFactory::InProgressRequest::
if (info_->response_code == net::HTTP_PROXY_AUTHENTICATION_REQUIRED)
return;
// We notify the completion here, and delete |this|.
factory_->web_request_api()->OnResponseStarted(&info_.value(), request_);
factory_->web_request_api()->OnCompleted(&info_.value(), request_, net::OK);
factory_->web_request()->OnResponseStarted(&info_.value(), request_);
factory_->web_request()->OnCompleted(&info_.value(), request_, net::OK);
factory_->RemoveRequest(network_service_request_id_, request_id_);
return;
@@ -653,7 +653,7 @@ void ProxyingURLLoaderFactory::InProgressRequest::ContinueToResponseStarted(
proxied_client_receiver_.Resume();
factory_->web_request_api()->OnResponseStarted(&info_.value(), request_);
factory_->web_request()->OnResponseStarted(&info_.value(), request_);
target_client_->OnReceiveResponse(current_response_.Clone(),
std::move(current_body_),
std::move(current_cached_metadata_));
@@ -672,8 +672,8 @@ void ProxyingURLLoaderFactory::InProgressRequest::ContinueToBeforeRedirect(
if (proxied_client_receiver_.is_bound())
proxied_client_receiver_.Resume();
factory_->web_request_api()->OnBeforeRedirect(&info_.value(), request_,
redirect_info.new_url);
factory_->web_request()->OnBeforeRedirect(&info_.value(), request_,
redirect_info.new_url);
target_client_->OnReceiveRedirect(redirect_info, current_response_.Clone());
request_.url = redirect_info.new_url;
request_.method = redirect_info.new_method;
@@ -696,7 +696,7 @@ void ProxyingURLLoaderFactory::InProgressRequest::
auto callback_pair = base::SplitOnceCallback(std::move(continuation));
DCHECK(info_.has_value());
int result = factory_->web_request_api()->OnHeadersReceived(
int result = factory_->web_request()->OnHeadersReceived(
&info_.value(), request_, std::move(callback_pair.first),
current_response_->headers.get(), &override_headers_, &redirect_url_);
if (result == net::ERR_BLOCKED_BY_CLIENT) {
@@ -724,15 +724,15 @@ void ProxyingURLLoaderFactory::InProgressRequest::OnRequestError(
const network::URLLoaderCompletionStatus& status) {
if (target_client_)
target_client_->OnComplete(status);
factory_->web_request_api()->OnErrorOccurred(&info_.value(), request_,
status.error_code);
factory_->web_request()->OnErrorOccurred(&info_.value(), request_,
status.error_code);
// Deletes |this|.
factory_->RemoveRequest(network_service_request_id_, request_id_);
}
ProxyingURLLoaderFactory::ProxyingURLLoaderFactory(
WebRequestAPI* web_request_api,
api::WebRequest* web_request,
const HandlersMap& intercepted_handlers,
int render_process_id,
int frame_routing_id,
@@ -744,7 +744,7 @@ ProxyingURLLoaderFactory::ProxyingURLLoaderFactory(
mojo::PendingReceiver<network::mojom::TrustedURLLoaderHeaderClient>
header_client_receiver,
content::ContentBrowserClient::URLLoaderFactoryType loader_factory_type)
: web_request_api_(web_request_api),
: web_request_{web_request},
intercepted_handlers_(intercepted_handlers),
render_process_id_(render_process_id),
frame_routing_id_(frame_routing_id),
@@ -824,7 +824,7 @@ void ProxyingURLLoaderFactory::CreateLoaderAndStart(
return;
}
if (!web_request_api()->HasListener()) {
if (!web_request()->HasListener()) {
// Pass-through to the original factory.
target_factory_->CreateLoaderAndStart(std::move(loader), request_id,
options, request, std::move(client),

View File

@@ -30,8 +30,8 @@
#include "services/network/public/mojom/url_loader_factory.mojom.h"
#include "services/network/public/mojom/url_response_head.mojom-forward.h"
#include "services/network/url_loader_factory.h"
#include "shell/browser/api/electron_api_web_request.h"
#include "shell/browser/net/electron_url_loader_factory.h"
#include "shell/browser/net/web_request_api_interface.h"
#include "url/gurl.h"
namespace mojo {
@@ -197,7 +197,7 @@ class ProxyingURLLoaderFactory
};
ProxyingURLLoaderFactory(
WebRequestAPI* web_request_api,
api::WebRequest* web_request,
const HandlersMap& intercepted_handlers,
int render_process_id,
int frame_routing_id,
@@ -239,11 +239,11 @@ class ProxyingURLLoaderFactory
mojo::PendingReceiver<network::mojom::TrustedHeaderClient> receiver)
override;
WebRequestAPI* web_request_api() { return web_request_api_; }
bool IsForServiceWorkerScript() const;
private:
api::WebRequest* web_request() { return web_request_; }
void OnTargetFactoryError();
void OnProxyBindingError();
void RemoveRequest(int32_t network_service_request_id, uint64_t request_id);
@@ -251,8 +251,7 @@ class ProxyingURLLoaderFactory
bool ShouldIgnoreConnectionsLimit(const network::ResourceRequest& request);
// Passed from api::WebRequest.
raw_ptr<WebRequestAPI> web_request_api_;
raw_ptr<api::WebRequest> web_request_;
// This is passed from api::Protocol.
//

View File

@@ -17,7 +17,7 @@
namespace electron {
ProxyingWebSocket::ProxyingWebSocket(
WebRequestAPI* web_request_api,
api::WebRequest* web_request,
WebSocketFactory factory,
const network::ResourceRequest& request,
mojo::PendingRemote<network::mojom::WebSocketHandshakeClient>
@@ -27,7 +27,7 @@ ProxyingWebSocket::ProxyingWebSocket(
int render_frame_id,
content::BrowserContext* browser_context,
uint64_t* request_id_generator)
: web_request_api_(web_request_api),
: web_request_{web_request},
request_(request),
factory_(std::move(factory)),
forwarding_handshake_client_(std::move(handshake_client)),
@@ -70,8 +70,8 @@ void ProxyingWebSocket::Start() {
weak_factory_.GetWeakPtr());
}
int result = web_request_api_->OnBeforeRequest(&info_, request_, continuation,
&redirect_url_);
int result = web_request_->OnBeforeRequest(&info_, request_, continuation,
&redirect_url_);
if (result == net::ERR_BLOCKED_BY_CLIENT) {
OnError(result);
@@ -97,7 +97,7 @@ void ProxyingWebSocket::ContinueToHeadersReceived() {
base::BindRepeating(&ProxyingWebSocket::OnHeadersReceivedComplete,
weak_factory_.GetWeakPtr());
info_.AddResponseInfoFromResourceResponse(*response_);
int result = web_request_api_->OnHeadersReceived(
int result = web_request_->OnHeadersReceived(
&info_, request_, continuation, response_->headers.get(),
&override_headers_, &redirect_url_);
@@ -152,7 +152,7 @@ void ProxyingWebSocket::OnConnectionEstablished(
void ProxyingWebSocket::ContinueToCompleted() {
DCHECK(forwarding_handshake_client_);
DCHECK(is_done_);
web_request_api_->OnCompleted(&info_, request_, net::ERR_WS_UPGRADE);
web_request_->OnCompleted(&info_, request_, net::ERR_WS_UPGRADE);
forwarding_handshake_client_->OnConnectionEstablished(
std::move(websocket_), std::move(client_receiver_),
std::move(handshake_response_), std::move(readable_),
@@ -180,7 +180,7 @@ void ProxyingWebSocket::OnAuthRequired(
base::BindRepeating(&ProxyingWebSocket::OnHeadersReceivedCompleteForAuth,
weak_factory_.GetWeakPtr(), auth_info);
info_.AddResponseInfoFromResourceResponse(*response_);
int result = web_request_api_->OnHeadersReceived(
int result = web_request_->OnHeadersReceived(
&info_, request_, continuation, response_->headers.get(),
&override_headers_, &redirect_url_);
@@ -219,7 +219,7 @@ void ProxyingWebSocket::OnHeadersReceived(const std::string& headers,
}
void ProxyingWebSocket::StartProxying(
WebRequestAPI* web_request_api,
api::WebRequest* web_request,
WebSocketFactory factory,
const GURL& url,
const net::SiteForCookies& site_for_cookies,
@@ -242,7 +242,7 @@ void ProxyingWebSocket::StartProxying(
request.request_initiator = origin;
auto* proxy = new ProxyingWebSocket(
web_request_api, std::move(factory), request, std::move(handshake_client),
web_request, std::move(factory), request, std::move(handshake_client),
has_extra_headers, process_id, render_frame_id, browser_context,
request_id_generator);
proxy->Start();
@@ -262,8 +262,8 @@ void ProxyingWebSocket::OnBeforeRequestComplete(int error_code) {
weak_factory_.GetWeakPtr());
info_.AddResponseInfoFromResourceResponse(*response_);
int result = web_request_api_->OnBeforeSendHeaders(
&info_, request_, continuation, &request_headers_);
int result = web_request_->OnBeforeSendHeaders(&info_, request_, continuation,
&request_headers_);
if (result == net::ERR_BLOCKED_BY_CLIENT) {
OnError(result);
@@ -296,7 +296,7 @@ void ProxyingWebSocket::OnBeforeSendHeadersComplete(
}
info_.AddResponseInfoFromResourceResponse(*response_);
web_request_api_->OnSendHeaders(&info_, request_, request_headers_);
web_request_->OnSendHeaders(&info_, request_, request_headers_);
if (!receiver_as_header_client_.is_bound())
ContinueToStartRequest(net::OK);
@@ -366,7 +366,7 @@ void ProxyingWebSocket::OnHeadersReceivedComplete(int error_code) {
ResumeIncomingMethodCallProcessing();
info_.AddResponseInfoFromResourceResponse(*response_);
web_request_api_->OnResponseStarted(&info_, request_);
web_request_->OnResponseStarted(&info_, request_);
if (!receiver_as_header_client_.is_bound())
ContinueToCompleted();
@@ -424,7 +424,7 @@ void ProxyingWebSocket::ResumeIncomingMethodCallProcessing() {
void ProxyingWebSocket::OnError(int error_code) {
if (!is_done_) {
is_done_ = true;
web_request_api_->OnErrorOccurred(&info_, request_, error_code);
web_request_->OnErrorOccurred(&info_, request_, error_code);
}
// Deletes |this|.

View File

@@ -19,7 +19,7 @@
#include "services/network/public/cpp/resource_request.h"
#include "services/network/public/mojom/network_context.mojom.h"
#include "services/network/public/mojom/websocket.mojom.h"
#include "shell/browser/net/web_request_api_interface.h"
#include "shell/browser/api/electron_api_web_request.h"
#include "url/gurl.h"
#include "url/origin.h"
@@ -52,7 +52,7 @@ class ProxyingWebSocket : public network::mojom::WebSocketHandshakeClient,
};
ProxyingWebSocket(
WebRequestAPI* web_request_api,
api::WebRequest* web_request,
WebSocketFactory factory,
const network::ResourceRequest& request,
mojo::PendingRemote<network::mojom::WebSocketHandshakeClient>
@@ -97,7 +97,7 @@ class ProxyingWebSocket : public network::mojom::WebSocketHandshakeClient,
OnHeadersReceivedCallback callback) override;
static void StartProxying(
WebRequestAPI* web_request_api,
api::WebRequest* web_request,
WebSocketFactory factory,
const GURL& url,
const net::SiteForCookies& site_for_cookies,
@@ -136,7 +136,7 @@ class ProxyingWebSocket : public network::mojom::WebSocketHandshakeClient,
void OnMojoConnectionError();
// Passed from api::WebRequest.
raw_ptr<WebRequestAPI> web_request_api_;
raw_ptr<api::WebRequest> web_request_;
// Saved to feed the api::WebRequest.
network::ResourceRequest request_;

View File

@@ -1,65 +0,0 @@
// Copyright (c) 2020 GitHub, Inc.
// Use of this source code is governed by the MIT license that can be
// found in the LICENSE file.
#ifndef ELECTRON_SHELL_BROWSER_NET_WEB_REQUEST_API_INTERFACE_H_
#define ELECTRON_SHELL_BROWSER_NET_WEB_REQUEST_API_INTERFACE_H_
#include <set>
#include <string>
#include "net/base/completion_once_callback.h"
#include "services/network/public/cpp/resource_request.h"
namespace extensions {
struct WebRequestInfo;
} // namespace extensions
namespace electron {
// Defines the interface for WebRequest API, implemented by api::WebRequestNS.
class WebRequestAPI {
public:
virtual ~WebRequestAPI() = default;
using BeforeSendHeadersCallback =
base::OnceCallback<void(const std::set<std::string>& removed_headers,
const std::set<std::string>& set_headers,
int error_code)>;
virtual bool HasListener() const = 0;
virtual int OnBeforeRequest(extensions::WebRequestInfo* info,
const network::ResourceRequest& request,
net::CompletionOnceCallback callback,
GURL* new_url) = 0;
virtual int OnBeforeSendHeaders(extensions::WebRequestInfo* info,
const network::ResourceRequest& request,
BeforeSendHeadersCallback callback,
net::HttpRequestHeaders* headers) = 0;
virtual int OnHeadersReceived(
extensions::WebRequestInfo* info,
const network::ResourceRequest& request,
net::CompletionOnceCallback callback,
const net::HttpResponseHeaders* original_response_headers,
scoped_refptr<net::HttpResponseHeaders>* override_response_headers,
GURL* allowed_unsafe_redirect_url) = 0;
virtual void OnSendHeaders(extensions::WebRequestInfo* info,
const network::ResourceRequest& request,
const net::HttpRequestHeaders& headers) = 0;
virtual void OnBeforeRedirect(extensions::WebRequestInfo* info,
const network::ResourceRequest& request,
const GURL& new_location) = 0;
virtual void OnResponseStarted(extensions::WebRequestInfo* info,
const network::ResourceRequest& request) = 0;
virtual void OnErrorOccurred(extensions::WebRequestInfo* info,
const network::ResourceRequest& request,
int net_error) = 0;
virtual void OnCompleted(extensions::WebRequestInfo* info,
const network::ResourceRequest& request,
int net_error) = 0;
virtual void OnRequestWillBeDestroyed(extensions::WebRequestInfo* info) = 0;
};
} // namespace electron
#endif // ELECTRON_SHELL_BROWSER_NET_WEB_REQUEST_API_INTERFACE_H_