diff --git a/shell/browser/api/atom_api_web_request_ns.cc b/shell/browser/api/atom_api_web_request_ns.cc index b1f6ca768f57..77bceb470f2c 100644 --- a/shell/browser/api/atom_api_web_request_ns.cc +++ b/shell/browser/api/atom_api_web_request_ns.cc @@ -236,6 +236,10 @@ const char* WebRequestNS::GetTypeName() { return "WebRequest"; } +bool WebRequestNS::HasListener() const { + return !(simple_listeners_.empty() && response_listeners_.empty()); +} + int WebRequestNS::OnBeforeRequest(extensions::WebRequestInfo* info, const network::ResourceRequest& request, net::CompletionOnceCallback callback, @@ -316,16 +320,25 @@ template void WebRequestNS::SetListener(Event event, Listeners* listeners, gin::Arguments* args) { + v8::Local arg; + // { urls }. std::set patterns; gin::Dictionary dict(args->isolate()); - args->GetNext(&dict) && dict.Get("urls", &patterns); + if (args->GetNext(&arg) && !arg->IsFunction()) { + // Note that gin treats Function as Dictionary when doing convertions, so we + // have to explicitly check if the argument is Function before trying to + // convert it to Dictionary. + if (gin::ConvertFromV8(args->isolate(), arg, &dict)) { + dict.Get("urls", &patterns); + args->GetNext(&arg); + } + } // Function or null. - v8::Local value; Listener listener; - if (!args->GetNext(&listener) && - !(args->GetNext(&value) && value->IsNull())) { + if (arg.IsEmpty() || + !(gin::ConvertFromV8(args->isolate(), arg, &listener) || arg->IsNull())) { args->ThrowTypeError("Must pass null or a Function"); return; } @@ -340,6 +353,9 @@ template void WebRequestNS::HandleSimpleEvent(SimpleEvent event, extensions::WebRequestInfo* request_info, Args... args) { + if (!base::Contains(simple_listeners_, event)) + return; + const auto& info = simple_listeners_[event]; if (!MatchesFilterCondition(request_info, info.url_patterns)) return; @@ -357,6 +373,9 @@ int WebRequestNS::HandleResponseEvent(ResponseEvent event, net::CompletionOnceCallback callback, Out out, Args... args) { + if (!base::Contains(response_listeners_, event)) + return net::OK; + const auto& info = response_listeners_[event]; if (!MatchesFilterCondition(request_info, info.url_patterns)) return net::OK; @@ -395,7 +414,11 @@ void WebRequestNS::OnListenerResult(uint64_t id, ReadFromResponse(isolate, &dict, out); } - std::move(callbacks_[id]).Run(result); + // The ProxyingURLLoaderFactory expects the callback to be executed + // asynchronously, because it used to work on IO thread before NetworkService. + base::SequencedTaskRunnerHandle::Get()->PostTask( + FROM_HERE, base::BindOnce(std::move(callbacks_[id]), result)); + callbacks_.erase(id); } // static diff --git a/shell/browser/api/atom_api_web_request_ns.h b/shell/browser/api/atom_api_web_request_ns.h index 46f873412ba8..c7bc8dc53f11 100644 --- a/shell/browser/api/atom_api_web_request_ns.h +++ b/shell/browser/api/atom_api_web_request_ns.h @@ -57,6 +57,7 @@ class WebRequestNS : public gin::Wrappable, public WebRequestAPI { ~WebRequestNS() override; // WebRequestAPI: + bool HasListener() const override; int OnBeforeRequest(extensions::WebRequestInfo* info, const network::ResourceRequest& request, net::CompletionOnceCallback callback, diff --git a/shell/browser/net/proxying_url_loader_factory.cc b/shell/browser/net/proxying_url_loader_factory.cc index 79afbcb75674..5a6807e5b67e 100644 --- a/shell/browser/net/proxying_url_loader_factory.cc +++ b/shell/browser/net/proxying_url_loader_factory.cc @@ -697,14 +697,13 @@ void ProxyingURLLoaderFactory::CreateLoaderAndStart( return; } - // Pass-through to the original factory. - target_factory_->CreateLoaderAndStart(std::move(loader), routing_id, - request_id, options, request, - std::move(client), traffic_annotation); - - // TODO(zcbenz): Remove the |CreateLoaderAndStart| call and create - // InProgressRequest when the webRequest API is used. - return; + if (!web_request_api()->HasListener()) { + // Pass-through to the original factory. + target_factory_->CreateLoaderAndStart( + std::move(loader), routing_id, request_id, options, request, + std::move(client), traffic_annotation); + return; + } // The request ID doesn't really matter. It just needs to be unique // per-BrowserContext so extensions can make sense of it. Note that diff --git a/shell/browser/net/proxying_url_loader_factory.h b/shell/browser/net/proxying_url_loader_factory.h index 91271a1a599a..0eb46f84a151 100644 --- a/shell/browser/net/proxying_url_loader_factory.h +++ b/shell/browser/net/proxying_url_loader_factory.h @@ -31,6 +31,7 @@ class WebRequestAPI { const std::set& set_headers, int error_code)>; + virtual bool HasListener() const = 0; virtual int OnBeforeRequest(extensions::WebRequestInfo* info, const network::ResourceRequest& request, net::CompletionOnceCallback callback, diff --git a/spec-main/api-protocol-spec.ts b/spec-main/api-protocol-spec.ts index 842a6b901ca2..a324a7485275 100644 --- a/spec-main/api-protocol-spec.ts +++ b/spec-main/api-protocol-spec.ts @@ -532,7 +532,7 @@ describe('protocol module', () => { expect({ ...qs.parse(r.data) }).to.deep.equal(postData) }) - it.skip('can use custom session', async () => { + it('can use custom session', async () => { const customSession = session.fromPartition('custom-ses', { cache: false }) customSession.webRequest.onBeforeRequest((details, callback) => { expect(details.url).to.equal('http://fake-host/')