feat: migrate protocol module to NetworkService (Part 11) (#18706)

* Pipe data into HTTP protocol handlers

* Remove unused parameters

* Remove "sending request of http protocol urls" test

Sending request to "http://" in "file://" violates CORS rules and always
fail, before NetworkService somehow Chromium still sent a request even
though the request failed with CORS error, so the test passes while the
test is not valid. With NetworkService no request is sent at all and the
test jsut fails.

So this is an ancient invalid test, as sending http requests have been
fully covered in other tests, I am removing this test.

atom/browser/net/atom_url_loader_factory.cc

@@ -12,6 +12,7 @@
 #include "atom/browser/atom_browser_context.h"
 #include "atom/browser/net/asar/asar_url_loader.h"
 #include "atom/browser/net/node_stream_loader.h"
+#include "atom/browser/net/url_pipe_loader.h"
 #include "atom/common/atom_constants.h"
 #include "atom/common/native_mate_converters/file_path_converter.h"
 #include "atom/common/native_mate_converters/gurl_converter.h"
@@ -248,8 +249,8 @@ void AtomURLLoaderFactory::StartLoading(
           std::move(loader), routing_id, request_id, options, new_request,
           std::move(client), traffic_annotation);
     } else {
-      StartLoadingHttp(std::move(loader), routing_id, request_id, options,
-                       new_request, std::move(client), traffic_annotation,
+      StartLoadingHttp(std::move(loader), new_request, std::move(client),
+                       traffic_annotation,
                        mate::Dictionary::CreateEmpty(args->isolate()));
     }
     return;
@@ -275,8 +276,8 @@ void AtomURLLoaderFactory::StartLoading(
                        std::move(head), dict, args->isolate(), response);
       break;
     case ProtocolType::kHttp:
-      StartLoadingHttp(std::move(loader), routing_id, request_id, options,
-                       request, std::move(client), traffic_annotation, dict);
+      StartLoadingHttp(std::move(loader), request, std::move(client),
+                       traffic_annotation, dict);
       break;
     case ProtocolType::kStream:
       StartLoadingStream(std::move(loader), std::move(client), std::move(head),
@@ -358,21 +359,18 @@ void AtomURLLoaderFactory::StartLoadingFile(
 // static
 void AtomURLLoaderFactory::StartLoadingHttp(
     network::mojom::URLLoaderRequest loader,
-    int32_t routing_id,
-    int32_t request_id,
-    uint32_t options,
     const network::ResourceRequest& original_request,
     network::mojom::URLLoaderClientPtr client,
     const net::MutableNetworkTrafficAnnotationTag& traffic_annotation,
     const mate::Dictionary& dict) {
-  network::ResourceRequest request;
-  request.headers = original_request.headers;
-  request.cors_exempt_headers = original_request.cors_exempt_headers;
+  auto request = std::make_unique<network::ResourceRequest>();
+  request->headers = original_request.headers;
+  request->cors_exempt_headers = original_request.cors_exempt_headers;
 
-  dict.Get("url", &request.url);
-  dict.Get("referrer", &request.referrer);
-  if (!dict.Get("method", &request.method))
-    request.method = original_request.method;
+  dict.Get("url", &request->url);
+  dict.Get("referrer", &request->referrer);
+  if (!dict.Get("method", &request->method))
+    request->method = original_request.method;
 
   scoped_refptr<AtomBrowserContext> browser_context =
       AtomBrowserContext::From("", false);
@@ -392,9 +390,10 @@ void AtomURLLoaderFactory::StartLoadingHttp(
   scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory =
       content::BrowserContext::GetDefaultStoragePartition(browser_context.get())
           ->GetURLLoaderFactoryForBrowserProcess();
-  url_loader_factory->CreateLoaderAndStart(
-      std::move(loader), routing_id, request_id, options, std::move(request),
-      std::move(client), traffic_annotation);
+  new URLPipeLoader(
+      url_loader_factory, std::move(request), std::move(loader),
+      std::move(client),
+      static_cast<net::NetworkTrafficAnnotationTag>(traffic_annotation));
 }
 
 // static

atom/browser/net/atom_url_loader_factory.h

@@ -81,9 +81,6 @@ class AtomURLLoaderFactory : public network::mojom::URLLoaderFactory {
                                v8::Local<v8::Value> response);
   static void StartLoadingHttp(
       network::mojom::URLLoaderRequest loader,
-      int32_t routing_id,
-      int32_t request_id,
-      uint32_t options,
       const network::ResourceRequest& original_request,
       network::mojom::URLLoaderClientPtr client,
       const net::MutableNetworkTrafficAnnotationTag& traffic_annotation,

atom/browser/net/url_pipe_loader.cc

@@ -0,0 +1,92 @@
+// Copyright (c) 2019 GitHub, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#include "atom/browser/net/url_pipe_loader.h"
+
+#include <utility>
+
+#include "services/network/public/cpp/shared_url_loader_factory.h"
+
+namespace atom {
+
+URLPipeLoader::URLPipeLoader(
+    scoped_refptr<network::SharedURLLoaderFactory> factory,
+    std::unique_ptr<network::ResourceRequest> request,
+    network::mojom::URLLoaderRequest loader,
+    network::mojom::URLLoaderClientPtr client,
+    const net::NetworkTrafficAnnotationTag& annotation)
+    : binding_(this, std::move(loader)),
+      client_(std::move(client)),
+      weak_factory_(this) {
+  binding_.set_connection_error_handler(base::BindOnce(
+      &URLPipeLoader::NotifyComplete, base::Unretained(this), net::ERR_FAILED));
+
+  // PostTask since it might destruct.
+  base::SequencedTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE,
+      base::BindOnce(&URLPipeLoader::Start, weak_factory_.GetWeakPtr(), factory,
+                     std::move(request), annotation));
+}
+
+URLPipeLoader::~URLPipeLoader() = default;
+
+void URLPipeLoader::Start(
+    scoped_refptr<network::SharedURLLoaderFactory> factory,
+    std::unique_ptr<network::ResourceRequest> request,
+    const net::NetworkTrafficAnnotationTag& annotation) {
+  loader_ = network::SimpleURLLoader::Create(std::move(request), annotation);
+  loader_->SetOnResponseStartedCallback(base::Bind(
+      &URLPipeLoader::OnResponseStarted, weak_factory_.GetWeakPtr()));
+  loader_->DownloadAsStream(factory.get(), this);
+}
+
+void URLPipeLoader::NotifyComplete(int result) {
+  client_->OnComplete(network::URLLoaderCompletionStatus(result));
+  delete this;
+}
+
+void URLPipeLoader::OnResponseStarted(
+    const GURL& final_url,
+    const network::ResourceResponseHead& response_head) {
+  mojo::ScopedDataPipeProducerHandle producer;
+  mojo::ScopedDataPipeConsumerHandle consumer;
+  MojoResult rv = mojo::CreateDataPipe(nullptr, &producer, &consumer);
+  if (rv != MOJO_RESULT_OK) {
+    NotifyComplete(net::ERR_INSUFFICIENT_RESOURCES);
+    return;
+  }
+
+  producer_ =
+      std::make_unique<mojo::StringDataPipeProducer>(std::move(producer));
+
+  client_->OnReceiveResponse(response_head);
+  client_->OnStartLoadingResponseBody(std::move(consumer));
+}
+
+void URLPipeLoader::OnWrite(base::OnceClosure resume, MojoResult result) {
+  if (result == MOJO_RESULT_OK)
+    std::move(resume).Run();
+  else
+    NotifyComplete(net::ERR_FAILED);
+}
+
+void URLPipeLoader::OnDataReceived(base::StringPiece string_piece,
+                                   base::OnceClosure resume) {
+  producer_->Write(
+      string_piece,
+      mojo::StringDataPipeProducer::AsyncWritingMode::
+          STRING_STAYS_VALID_UNTIL_COMPLETION,
+      base::BindOnce(&URLPipeLoader::OnWrite, weak_factory_.GetWeakPtr(),
+                     std::move(resume)));
+}
+
+void URLPipeLoader::OnRetry(base::OnceClosure start_retry) {
+  NOTREACHED();
+}
+
+void URLPipeLoader::OnComplete(bool success) {
+  NotifyComplete(loader_->NetError());
+}
+
+}  // namespace atom

atom/browser/net/url_pipe_loader.h

@@ -0,0 +1,80 @@
+// Copyright (c) 2019 GitHub, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#ifndef ATOM_BROWSER_NET_URL_PIPE_LOADER_H_
+#define ATOM_BROWSER_NET_URL_PIPE_LOADER_H_
+
+#include <memory>
+#include <string>
+#include <vector>
+
+#include "mojo/public/cpp/bindings/strong_binding.h"
+#include "mojo/public/cpp/system/string_data_pipe_producer.h"
+#include "services/network/public/cpp/simple_url_loader.h"
+#include "services/network/public/cpp/simple_url_loader_stream_consumer.h"
+#include "services/network/public/mojom/url_loader.mojom.h"
+
+namespace network {
+class SharedURLLoaderFactory;
+}
+
+namespace atom {
+
+// Read data from URL and pipe it to NetworkService.
+//
+// Different from creating a new loader for the URL directly, protocol handlers
+// using this loader can work around CORS restrictions.
+//
+// This class manages its own lifetime and should delete itself when the
+// connection is lost or finished.
+class URLPipeLoader : public network::mojom::URLLoader,
+                      public network::SimpleURLLoaderStreamConsumer {
+ public:
+  URLPipeLoader(scoped_refptr<network::SharedURLLoaderFactory> factory,
+                std::unique_ptr<network::ResourceRequest> request,
+                network::mojom::URLLoaderRequest loader,
+                network::mojom::URLLoaderClientPtr client,
+                const net::NetworkTrafficAnnotationTag& annotation);
+
+ private:
+  ~URLPipeLoader() override;
+
+  void Start(scoped_refptr<network::SharedURLLoaderFactory> factory,
+             std::unique_ptr<network::ResourceRequest> request,
+             const net::NetworkTrafficAnnotationTag& annotation);
+  void NotifyComplete(int result);
+  void OnResponseStarted(const GURL& final_url,
+                         const network::ResourceResponseHead& response_head);
+  void OnWrite(base::OnceClosure resume, MojoResult result);
+
+  // SimpleURLLoaderStreamConsumer:
+  void OnDataReceived(base::StringPiece string_piece,
+                      base::OnceClosure resume) override;
+  void OnComplete(bool success) override;
+  void OnRetry(base::OnceClosure start_retry) override;
+
+  // URLLoader:
+  void FollowRedirect(const std::vector<std::string>& removed_headers,
+                      const net::HttpRequestHeaders& modified_headers,
+                      const base::Optional<GURL>& new_url) override {}
+  void ProceedWithResponse() override {}
+  void SetPriority(net::RequestPriority priority,
+                   int32_t intra_priority_value) override {}
+  void PauseReadingBodyFromNet() override {}
+  void ResumeReadingBodyFromNet() override {}
+
+  mojo::Binding<network::mojom::URLLoader> binding_;
+  network::mojom::URLLoaderClientPtr client_;
+
+  std::unique_ptr<mojo::StringDataPipeProducer> producer_;
+  std::unique_ptr<network::SimpleURLLoader> loader_;
+
+  base::WeakPtrFactory<URLPipeLoader> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(URLPipeLoader);
+};
+
+}  // namespace atom
+
+#endif  // ATOM_BROWSER_NET_URL_PIPE_LOADER_H_

filenames.gni

@@ -253,6 +253,8 @@ filenames = {
     "atom/browser/net/resolve_proxy_helper.h",
     "atom/browser/net/system_network_context_manager.cc",
     "atom/browser/net/system_network_context_manager.h",
+    "atom/browser/net/url_pipe_loader.cc",
+    "atom/browser/net/url_pipe_loader.h",
     "atom/browser/net/url_request_about_job.cc",
     "atom/browser/net/url_request_about_job.h",
     "atom/browser/net/url_request_async_asar_job.cc",

spec/chromium-spec.js

@@ -86,20 +86,6 @@ describe('chromium feature', () => {
     })
   })
 
-  describe('sending request of http protocol urls', () => {
-    it('does not crash', (done) => {
-      const server = http.createServer((req, res) => {
-        res.end()
-        server.close()
-        done()
-      })
-      server.listen(0, '127.0.0.1', () => {
-        const port = server.address().port
-        $.get(`http://127.0.0.1:${port}`)
-      })
-    })
-  })
-
   describe('accessing key names also used as Node.js module names', () => {
     it('does not crash', (done) => {
       w = new BrowserWindow({ show: false })