feat: migrate webRequest module to NetworkService (Part 8) (#19841)

* fix: fill uploadData property

* fix: requestHeaders in onBeforeSendHeaders

* fix: responseHeaders in onHeadersReceived

* fix: header keys should not be lowercased

* fix: gin::Dictionary::Get succeeds even though key does not exist...

* fix: throw for invalid filters

* test: re-enable api-web-request-spec

* chore: do not use deprecated base::Value API

shell/browser/api/atom_api_web_request_ns.cc

@@ -9,6 +9,7 @@
 #include <utility>
 
 #include "base/stl_util.h"
+#include "base/values.h"
 #include "gin/converter.h"
 #include "gin/dictionary.h"
 #include "gin/object_template_builder.h"
@@ -99,6 +100,28 @@ bool MatchesFilterCondition(extensions::WebRequestInfo* info,
   return false;
 }
 
+// Convert HttpResponseHeaders to V8.
+//
+// Note that while we already have converters for HttpResponseHeaders, we can
+// not use it because it lowercases the header keys, while the webRequest has
+// to pass the original keys.
+v8::Local<v8::Value> HttpResponseHeadersToV8(
+    net::HttpResponseHeaders* headers) {
+  base::DictionaryValue response_headers;
+  if (headers) {
+    size_t iter = 0;
+    std::string key;
+    std::string value;
+    while (headers->EnumerateHeaderLines(&iter, &key, &value)) {
+      base::Value* values = response_headers.FindListKey(key);
+      if (!values)
+        values = response_headers.SetKey(key, base::ListValue());
+      values->GetList().emplace_back(value);
+    }
+  }
+  return gin::ConvertToV8(v8::Isolate::GetCurrent(), response_headers);
+}
+
 // Overloaded by multiple types to fill the |details| object.
 void ToDictionary(gin::Dictionary* details, extensions::WebRequestInfo* info) {
   details->Set("id", info->id);
@@ -110,9 +133,10 @@ void ToDictionary(gin::Dictionary* details, extensions::WebRequestInfo* info) {
     details->Set("ip", info->response_ip);
   if (info->response_headers) {
     details->Set("fromCache", info->response_from_cache);
-    details->Set("responseHeaders", info->response_headers.get());
     details->Set("statusLine", info->response_headers->GetStatusLine());
     details->Set("statusCode", info->response_headers->response_code());
+    details->Set("responseHeaders",
+                 HttpResponseHeadersToV8(info->response_headers.get()));
   }
 
   auto* web_contents = content::WebContents::FromRenderFrameHost(
@@ -127,6 +151,8 @@ void ToDictionary(gin::Dictionary* details, extensions::WebRequestInfo* info) {
 void ToDictionary(gin::Dictionary* details,
                   const network::ResourceRequest& request) {
   details->Set("referrer", request.referrer);
+  if (request.request_body)
+    details->Set("uploadData", *request.request_body);
 }
 
 void ToDictionary(gin::Dictionary* details,
@@ -165,7 +191,7 @@ void ReadFromResponse(v8::Isolate* isolate,
                       gin::Dictionary* response,
                       net::HttpRequestHeaders* headers) {
   headers->Clear();
-  gin::ConvertFromV8(isolate, gin::ConvertToV8(isolate, *response), headers);
+  response->Get("requestHeaders", headers);
 }
 
 void ReadFromResponse(v8::Isolate* isolate,
@@ -176,7 +202,7 @@ void ReadFromResponse(v8::Isolate* isolate,
   if (!response->Get("statusLine", &status_line))
     status_line = headers.second;
   v8::Local<v8::Value> value;
-  if (response->Get("responseHeaders", &value)) {
+  if (response->Get("responseHeaders", &value) && value->IsObject()) {
     *headers.first = new net::HttpResponseHeaders("");
     (*headers.first)->ReplaceStatusLine(status_line);
     gin::Converter<net::HttpResponseHeaders*>::FromV8(isolate, value,
@@ -256,7 +282,7 @@ int WebRequestNS::OnBeforeSendHeaders(extensions::WebRequestInfo* info,
       kOnBeforeSendHeaders, info,
       base::BindOnce(std::move(callback), std::set<std::string>(),
                      std::set<std::string>()),
-      headers, request);
+      headers, request, *headers);
 }
 
 int WebRequestNS::OnHeadersReceived(
@@ -323,18 +349,35 @@ void WebRequestNS::SetListener(Event event,
   v8::Local<v8::Value> arg;
 
   // { urls }.
-  std::set<URLPattern> patterns;
+  std::set<std::string> filter_patterns;
   gin::Dictionary dict(args->isolate());
   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);
+      if (!dict.Get("urls", &filter_patterns)) {
+        args->ThrowTypeError("Parameter 'filter' must have property 'urls'.");
+        return;
+      }
       args->GetNext(&arg);
     }
   }
 
+  std::set<URLPattern> patterns;
+  for (const std::string& filter_pattern : filter_patterns) {
+    URLPattern pattern(URLPattern::SCHEME_ALL);
+    const URLPattern::ParseResult result = pattern.Parse(filter_pattern);
+    if (result == URLPattern::ParseResult::kSuccess) {
+      patterns.insert(pattern);
+    } else {
+      const char* error_type = URLPattern::GetParseResultString(result);
+      args->ThrowTypeError("Invalid url pattern " + filter_pattern + ": " +
+                           error_type);
+      return;
+    }
+  }
+
   // Function or null.
   Listener listener;
   if (arg.IsEmpty() ||

shell/browser/net/proxying_url_loader_factory.cc

@@ -608,6 +608,8 @@ void ProxyingURLLoaderFactory::InProgressRequest::
   override_headers_ = nullptr;
   redirect_url_ = GURL();
 
+  info_->AddResponseInfoFromResourceResponse(current_response_);
+
   net::CompletionRepeatingCallback copyable_callback =
       base::AdaptCallbackForRepeating(std::move(continuation));
   DCHECK(info_.has_value());

shell/common/gin_converters/net_converter.cc

@@ -160,15 +160,10 @@ v8::Local<v8::Value> Converter<net::HttpResponseHeaders*>::ToV8(
     std::string value;
     while (headers->EnumerateHeaderLines(&iter, &key, &value)) {
       key = base::ToLowerASCII(key);
-      if (response_headers.FindKey(key)) {
+      base::Value* values = response_headers.FindListKey(key);
-        base::ListValue* values = nullptr;
+      if (!values)
-        if (response_headers.GetList(key, &values))
+        values = response_headers.SetKey(key, base::ListValue());
-          values->AppendString(value);
+      values->GetList().emplace_back(value);
-      } else {
-        auto values = std::make_unique<base::ListValue>();
-        values->AppendString(value);
-        response_headers.Set(key, std::move(values));
-      }
     }
   }
   return ConvertToV8(isolate, response_headers);
@@ -251,6 +246,37 @@ bool Converter<net::HttpRequestHeaders>::FromV8(v8::Isolate* isolate,
   return true;
 }
 
+// static
+v8::Local<v8::Value> Converter<network::ResourceRequestBody>::ToV8(
+    v8::Isolate* isolate,
+    const network::ResourceRequestBody& val) {
+  const auto& elements = *val.elements();
+  v8::Local<v8::Array> arr = v8::Array::New(isolate, elements.size());
+  for (size_t i = 0; i < elements.size(); ++i) {
+    const auto& element = elements[i];
+    gin::Dictionary upload_data(isolate, v8::Object::New(isolate));
+    switch (element.type()) {
+      case network::mojom::DataElementType::kFile:
+        upload_data.Set("file", element.path().value());
+        break;
+      case network::mojom::DataElementType::kBytes:
+        upload_data.Set("bytes", node::Buffer::Copy(isolate, element.bytes(),
+                                                    element.length())
+                                     .ToLocalChecked());
+        break;
+      case network::mojom::DataElementType::kBlob:
+        upload_data.Set("blobUUID", element.blob_uuid());
+        break;
+      default:
+        NOTREACHED() << "Found unsupported data element";
+    }
+    arr->Set(isolate->GetCurrentContext(), static_cast<uint32_t>(i),
+             ConvertToV8(isolate, upload_data))
+        .Check();
+  }
+  return arr;
+}
+
 // static
 v8::Local<v8::Value> Converter<network::ResourceRequest>::ToV8(
     v8::Isolate* isolate,
@@ -260,33 +286,8 @@ v8::Local<v8::Value> Converter<network::ResourceRequest>::ToV8(
   dict.Set("url", val.url.spec());
   dict.Set("referrer", val.referrer.spec());
   dict.Set("headers", val.headers);
-  if (val.request_body) {
+  if (val.request_body)
-    const auto& elements = *val.request_body->elements();
+    dict.Set("uploadData", ConvertToV8(isolate, *val.request_body));
-    v8::Local<v8::Array> arr = v8::Array::New(isolate, elements.size());
-    for (size_t i = 0; i < elements.size(); ++i) {
-      const auto& element = elements[i];
-      gin::Dictionary upload_data(isolate, v8::Object::New(isolate));
-      switch (element.type()) {
-        case network::mojom::DataElementType::kFile:
-          upload_data.Set("file", element.path().value());
-          break;
-        case network::mojom::DataElementType::kBytes:
-          upload_data.Set("bytes", node::Buffer::Copy(isolate, element.bytes(),
-                                                      element.length())
-                                       .ToLocalChecked());
-          break;
-        case network::mojom::DataElementType::kBlob:
-          upload_data.Set("blobUUID", element.blob_uuid());
-          break;
-        default:
-          NOTREACHED() << "Found unsupported data element";
-      }
-      arr->Set(isolate->GetCurrentContext(), static_cast<uint32_t>(i),
-               ConvertToV8(isolate, upload_data))
-          .Check();
-    }
-    dict.Set("uploadData", arr);
-  }
   return ConvertToV8(isolate, dict);
 }
 

shell/common/gin_converters/net_converter.h

@@ -69,6 +69,12 @@ struct Converter<net::HttpRequestHeaders> {
                      net::HttpRequestHeaders* out);
 };
 
+template <>
+struct Converter<network::ResourceRequestBody> {
+  static v8::Local<v8::Value> ToV8(v8::Isolate* isolate,
+                                   const network::ResourceRequestBody& val);
+};
+
 template <>
 struct Converter<network::ResourceRequest> {
   static v8::Local<v8::Value> ToV8(v8::Isolate* isolate,

spec/api-web-request-spec.js

@@ -12,7 +12,7 @@ chai.use(dirtyChai)
 /* The whole webRequest API doesn't use standard callbacks */
 /* eslint-disable standard/no-callback-literal */
 
-describe.skip('webRequest module', () => {
+describe('webRequest module', () => {
   const ses = session.defaultSession
   const server = http.createServer((req, res) => {
     if (req.url === '/serverRedirect') {