refactor: rewrite the net module to simplify state tracking (#21244)

docs/api/client-request.md

@@ -32,8 +32,8 @@ the hostname and the port number 'hostname:port'.
   * `redirect` String (optional) - The redirect mode for this request. Should be
 one of `follow`, `error` or `manual`. Defaults to `follow`. When mode is `error`,
 any redirection will be aborted. When mode is `manual` the redirection will be
-deferred until [`request.followRedirect`](#requestfollowredirect) is invoked. Listen for the [`redirect`](#event-redirect) event in
-this mode to get more details about the redirect request.
+cancelled unless [`request.followRedirect`](#requestfollowredirect) is invoked
+synchronously during the [`redirect`](#event-redirect) event.
 
 `options` properties such as `protocol`, `host`, `hostname`, `port` and `path`
 strictly follow the Node.js model as described in the
@@ -136,8 +136,11 @@ Returns:
 * `redirectUrl` String
 * `responseHeaders` Record<String, String[]>
 
-Emitted when there is redirection and the mode is `manual`. Calling
-[`request.followRedirect`](#requestfollowredirect) will continue with the redirection.
+Emitted when the server returns a redirect response (e.g. 301 Moved
+Permanently). Calling [`request.followRedirect`](#requestfollowredirect) will
+continue with the redirection.  If this event is handled,
+[`request.followRedirect`](#requestfollowredirect) must be called
+**synchronously**, otherwise the request will be cancelled.
 
 ### Instance Properties
 
@@ -214,7 +217,8 @@ response object,it will emit the `aborted` event.
 
 #### `request.followRedirect()`
 
-Continues any deferred redirection request when the redirection mode is `manual`.
+Continues any pending redirection. Can only be called during a `'redirect'`
+event.
 
 #### `request.getUploadProgress()`
 

filenames.gni

@@ -98,8 +98,8 @@ filenames = {
     "shell/browser/api/atom_api_top_level_window.h",
     "shell/browser/api/atom_api_tray.cc",
     "shell/browser/api/atom_api_tray.h",
-    "shell/browser/api/atom_api_url_request.cc",
-    "shell/browser/api/atom_api_url_request.h",
+    "shell/browser/api/atom_api_url_loader.cc",
+    "shell/browser/api/atom_api_url_loader.h",
     "shell/browser/api/atom_api_view.cc",
     "shell/browser/api/atom_api_view.h",
     "shell/browser/api/atom_api_web_contents.cc",

lib/browser/api/net.js

@@ -2,17 +2,13 @@
 
 const url = require('url')
 const { EventEmitter } = require('events')
-const { Readable } = require('stream')
+const { Readable, Writable } = require('stream')
 const { app } = require('electron')
 const { Session } = process.electronBinding('session')
-const { net, Net } = process.electronBinding('net')
-const { URLRequest } = net
+const { net, Net, _isValidHeaderName, _isValidHeaderValue } = process.electronBinding('net')
+const { URLLoader } = net
 
-// Net is an EventEmitter.
-Object.setPrototypeOf(Net.prototype, EventEmitter.prototype)
-EventEmitter.call(net)
-
-Object.setPrototypeOf(URLRequest.prototype, EventEmitter.prototype)
+Object.setPrototypeOf(URLLoader.prototype, EventEmitter.prototype)
 
 const kSupportedProtocols = new Set(['http:', 'https:'])
 
@@ -40,32 +36,24 @@ const discardableDuplicateHeaders = new Set([
 ])
 
 class IncomingMessage extends Readable {
-  constructor (urlRequest) {
+  constructor (responseHead) {
     super()
-    this.urlRequest = urlRequest
-    this.shouldPush = false
-    this.data = []
-    this.urlRequest.on('data', (event, chunk) => {
-      this._storeInternalData(chunk)
-      this._pushInternalData()
-    })
-    this.urlRequest.on('end', () => {
-      this._storeInternalData(null)
-      this._pushInternalData()
-    })
+    this._shouldPush = false
+    this._data = []
+    this._responseHead = responseHead
   }
 
   get statusCode () {
-    return this.urlRequest.statusCode
+    return this._responseHead.statusCode
   }
 
   get statusMessage () {
-    return this.urlRequest.statusMessage
+    return this._responseHead.statusMessage
   }
 
   get headers () {
     const filteredHeaders = {}
-    const rawHeaders = this.urlRequest.rawResponseHeaders
+    const rawHeaders = this._responseHead.headers
     Object.keys(rawHeaders).forEach(header => {
       if (header in filteredHeaders && discardableDuplicateHeaders.has(header)) {
         // do nothing with discardable duplicate headers
@@ -88,11 +76,11 @@ class IncomingMessage extends Readable {
   }
 
   get httpVersionMajor () {
-    return this.urlRequest.httpVersionMajor
+    return this._responseHead.httpVersion.major
   }
 
   get httpVersionMinor () {
-    return this.urlRequest.httpVersionMinor
+    return this._responseHead.httpVersion.minor
   }
 
   get rawTrailers () {
@@ -104,170 +92,197 @@ class IncomingMessage extends Readable {
   }
 
   _storeInternalData (chunk) {
-    this.data.push(chunk)
+    this._data.push(chunk)
+    this._pushInternalData()
   }
 
   _pushInternalData () {
-    while (this.shouldPush && this.data.length > 0) {
-      const chunk = this.data.shift()
-      this.shouldPush = this.push(chunk)
+    while (this._shouldPush && this._data.length > 0) {
+      const chunk = this._data.shift()
+      this._shouldPush = this.push(chunk)
     }
   }
 
   _read () {
-    this.shouldPush = true
+    this._shouldPush = true
     this._pushInternalData()
   }
 }
 
-URLRequest.prototype._emitRequestEvent = function (isAsync, ...rest) {
-  if (isAsync) {
-    process.nextTick(() => {
-      this.clientRequest.emit(...rest)
-    })
-  } else {
-    this.clientRequest.emit(...rest)
+/** Writable stream that buffers up everything written to it. */
+class SlurpStream extends Writable {
+  constructor () {
+    super()
+    this._data = Buffer.alloc(0)
   }
-}
-
-URLRequest.prototype._emitResponseEvent = function (isAsync, ...rest) {
-  if (isAsync) {
-    process.nextTick(() => {
-      this._response.emit(...rest)
-    })
-  } else {
-    this._response.emit(...rest)
+  _write (chunk, encoding, callback) {
+    this._data = Buffer.concat([this._data, chunk])
+    callback()
   }
+  data () { return this._data }
 }
 
-class ClientRequest extends EventEmitter {
-  constructor (options, callback) {
+class ChunkedBodyStream extends Writable {
+  constructor (clientRequest) {
     super()
+    this._clientRequest = clientRequest
+  }
 
-    if (!app.isReady()) {
-      throw new Error('net module can only be used after app is ready')
-    }
-
-    if (typeof options === 'string') {
-      options = url.parse(options)
+  _write (chunk, encoding, callback) {
+    if (this._downstream) {
+      this._downstream.write(chunk).then(callback, callback)
     } else {
-      options = Object.assign({}, options)
+      // the contract of _write is that we won't be called again until we call
+      // the callback, so we're good to just save a single chunk.
+      this._pendingChunk = chunk
+      this._pendingCallback = callback
+
+      // The first write to a chunked body stream begins the request.
+      this._clientRequest._startRequest()
     }
+  }
 
-    const method = (options.method || 'GET').toUpperCase()
-    let urlStr = options.url
+  _final (callback) {
+    this._downstream.done()
+    callback()
+  }
 
-    if (!urlStr) {
-      const urlObj = {}
-      const protocol = options.protocol || 'http:'
-      if (!kSupportedProtocols.has(protocol)) {
-        throw new Error('Protocol "' + protocol + '" not supported')
+  startReading (pipe) {
+    if (this._downstream) {
+      throw new Error('two startReading calls???')
+    }
+    this._downstream = pipe
+    if (this._pendingChunk) {
+      const doneWriting = (maybeError) => {
+        const cb = this._pendingCallback
+        delete this._pendingCallback
+        delete this._pendingChunk
+        cb(maybeError)
       }
-      urlObj.protocol = protocol
-
-      if (options.host) {
-        urlObj.host = options.host
-      } else {
-        if (options.hostname) {
-          urlObj.hostname = options.hostname
-        } else {
-          urlObj.hostname = 'localhost'
-        }
+      this._downstream.write(this._pendingChunk).then(doneWriting, doneWriting)
+    }
+  }
+}
 
-        if (options.port) {
-          urlObj.port = options.port
-        }
-      }
+function parseOptions (options) {
+  if (typeof options === 'string') {
+    options = url.parse(options)
+  } else {
+    options = { ...options }
+  }
 
-      if (options.path && / /.test(options.path)) {
-        // The actual regex is more like /[^A-Za-z0-9\-._~!$&'()*+,;=/:@]/
-        // with an additional rule for ignoring percentage-escaped characters
-        // but that's a) hard to capture in a regular expression that performs
-        // well, and b) possibly too restrictive for real-world usage. That's
-        // why it only scans for spaces because those are guaranteed to create
-        // an invalid request.
-        throw new TypeError('Request path contains unescaped characters')
-      }
-      const pathObj = url.parse(options.path || '/')
-      urlObj.pathname = pathObj.pathname
-      urlObj.search = pathObj.search
-      urlObj.hash = pathObj.hash
-      urlStr = url.format(urlObj)
-    }
+  const method = (options.method || 'GET').toUpperCase()
+  let urlStr = options.url
 
-    const redirectPolicy = options.redirect || 'follow'
-    if (!['follow', 'error', 'manual'].includes(redirectPolicy)) {
-      throw new Error('redirect mode should be one of follow, error or manual')
+  if (!urlStr) {
+    const urlObj = {}
+    const protocol = options.protocol || 'http:'
+    if (!kSupportedProtocols.has(protocol)) {
+      throw new Error('Protocol "' + protocol + '" not supported')
     }
+    urlObj.protocol = protocol
 
-    const urlRequestOptions = {
-      method: method,
-      url: urlStr,
-      redirect: redirectPolicy
-    }
-    if (options.session) {
-      if (options.session instanceof Session) {
-        urlRequestOptions.session = options.session
+    if (options.host) {
+      urlObj.host = options.host
+    } else {
+      if (options.hostname) {
+        urlObj.hostname = options.hostname
       } else {
-        throw new TypeError('`session` should be an instance of the Session class')
+        urlObj.hostname = 'localhost'
       }
-    } else if (options.partition) {
-      if (typeof options.partition === 'string') {
-        urlRequestOptions.partition = options.partition
-      } else {
-        throw new TypeError('`partition` should be a string')
+
+      if (options.port) {
+        urlObj.port = options.port
       }
     }
 
-    const urlRequest = new URLRequest(urlRequestOptions)
+    if (options.path && / /.test(options.path)) {
+      // The actual regex is more like /[^A-Za-z0-9\-._~!$&'()*+,;=/:@]/
+      // with an additional rule for ignoring percentage-escaped characters
+      // but that's a) hard to capture in a regular expression that performs
+      // well, and b) possibly too restrictive for real-world usage. That's
+      // why it only scans for spaces because those are guaranteed to create
+      // an invalid request.
+      throw new TypeError('Request path contains unescaped characters')
+    }
+    const pathObj = url.parse(options.path || '/')
+    urlObj.pathname = pathObj.pathname
+    urlObj.search = pathObj.search
+    urlObj.hash = pathObj.hash
+    urlStr = url.format(urlObj)
+  }
 
-    // Set back and forward links.
-    this.urlRequest = urlRequest
-    urlRequest.clientRequest = this
+  const redirectPolicy = options.redirect || 'follow'
+  if (!['follow', 'error', 'manual'].includes(redirectPolicy)) {
+    throw new Error('redirect mode should be one of follow, error or manual')
+  }
 
-    // This is a copy of the extra headers structure held by the native
-    // net::URLRequest. The main reason is to keep the getHeader API synchronous
-    // after the request starts.
-    this.extraHeaders = {}
+  if (options.headers != null && typeof options.headers !== 'object') {
+    throw new TypeError('headers must be an object')
+  }
 
-    if (options.headers) {
-      for (const key in options.headers) {
-        this.setHeader(key, options.headers[key])
-      }
+  const urlLoaderOptions = {
+    method: method,
+    url: urlStr,
+    redirectPolicy,
+    extraHeaders: options.headers || {}
+  }
+  for (const [name, value] of Object.entries(urlLoaderOptions.extraHeaders)) {
+    if (!_isValidHeaderName(name)) {
+      throw new Error(`Invalid header name: '${name}'`)
     }
+    if (!_isValidHeaderValue(value.toString())) {
+      throw new Error(`Invalid value for header '${name}': '${value}'`)
+    }
+  }
+  if (options.session) {
+    if (options.session instanceof Session) {
+      urlLoaderOptions.session = options.session
+    } else {
+      throw new TypeError('`session` should be an instance of the Session class')
+    }
+  } else if (options.partition) {
+    if (typeof options.partition === 'string') {
+      urlLoaderOptions.partition = options.partition
+    } else {
+      throw new TypeError('`partition` should be a string')
+    }
+  }
+  return urlLoaderOptions
+}
 
-    // Set when the request uses chunked encoding. Can be switched
-    // to true only once and never set back to false.
-    this.chunkedEncodingEnabled = false
-
-    urlRequest.on('response', () => {
-      const response = new IncomingMessage(urlRequest)
-      urlRequest._response = response
-      this.emit('response', response)
-    })
+class ClientRequest extends Writable {
+  constructor (options, callback) {
+    super({ autoDestroy: true })
 
-    urlRequest.on('login', (event, authInfo, callback) => {
-      const handled = this.emit('login', authInfo, callback)
-      if (!handled) {
-        // If there were no listeners, cancel the authentication request.
-        callback()
-      }
-    })
+    if (!app.isReady()) {
+      throw new Error('net module can only be used after app is ready')
+    }
 
     if (callback) {
       this.once('response', callback)
     }
-  }
 
-  get chunkedEncoding () {
-    return this.chunkedEncodingEnabled
+    const { redirectPolicy, ...urlLoaderOptions } = parseOptions(options)
+    this._urlLoaderOptions = urlLoaderOptions
+    this._redirectPolicy = redirectPolicy
+    this._started = false
   }
 
   set chunkedEncoding (value) {
-    if (!this.urlRequest.notStarted) {
-      throw new Error('Can\'t set the transfer encoding, headers have been sent')
+    if (this._started) {
+      throw new Error('chunkedEncoding can only be set before the request is started')
+    }
+    if (typeof this._chunkedEncoding !== 'undefined') {
+      throw new Error('chunkedEncoding can only be set once')
+    }
+    this._chunkedEncoding = !!value
+    if (this._chunkedEncoding) {
+      this._body = new ChunkedBodyStream(this)
+      this._urlLoaderOptions.body = (pipe) => {
+        this._body.startReading(pipe)
+      }
     }
-    this.chunkedEncodingEnabled = value
   }
 
   setHeader (name, value) {
@@ -277,13 +292,18 @@ class ClientRequest extends EventEmitter {
     if (value == null) {
       throw new Error('`value` required in setHeader("' + name + '", value)')
     }
-    if (!this.urlRequest.notStarted) {
+    if (this._started || this._firstWrite) {
       throw new Error('Can\'t set headers after they are sent')
     }
+    if (!_isValidHeaderName(name)) {
+      throw new Error(`Invalid header name: '${name}'`)
+    }
+    if (!_isValidHeaderValue(value.toString())) {
+      throw new Error(`Invalid value for header '${name}': '${value}'`)
+    }
 
     const key = name.toLowerCase()
-    this.extraHeaders[key] = value
-    this.urlRequest.setExtraHeader(name, value.toString())
+    this._urlLoaderOptions.extraHeaders[key] = value
   }
 
   getHeader (name) {
@@ -291,12 +311,8 @@ class ClientRequest extends EventEmitter {
       throw new Error('`name` is required for getHeader(name)')
     }
 
-    if (!this.extraHeaders) {
-      return
-    }
-
     const key = name.toLowerCase()
-    return this.extraHeaders[key]
+    return this._urlLoaderOptions.extraHeaders[key]
   }
 
   removeHeader (name) {
@@ -304,91 +320,142 @@ class ClientRequest extends EventEmitter {
       throw new Error('`name` is required for removeHeader(name)')
     }
 
-    if (!this.urlRequest.notStarted) {
+    if (this._started || this._firstWrite) {
       throw new Error('Can\'t remove headers after they are sent')
     }
 
     const key = name.toLowerCase()
-    delete this.extraHeaders[key]
-    this.urlRequest.removeExtraHeader(name)
+    delete this._urlLoaderOptions.extraHeaders[key]
   }
 
-  _write (chunk, encoding, callback, isLast) {
-    const chunkIsString = typeof chunk === 'string'
-    const chunkIsBuffer = chunk instanceof Buffer
-    if (!chunkIsString && !chunkIsBuffer) {
-      throw new TypeError('First argument must be a string or Buffer')
-    }
-
-    if (chunkIsString) {
-      // We convert all strings into binary buffers.
-      chunk = Buffer.from(chunk, encoding)
-    }
-
-    // Since writing to the network is asynchronous, we conservatively
-    // assume that request headers are written after delivering the first
-    // buffer to the network IO thread.
-    if (this.urlRequest.notStarted) {
-      this.urlRequest.setChunkedUpload(this.chunkedEncoding)
-    }
-
-    // Headers are assumed to be sent on first call to _writeBuffer,
-    // i.e. after the first call to write or end.
-    const result = this.urlRequest.write(chunk, isLast)
-
-    // The write callback is fired asynchronously to mimic Node.js.
-    if (callback) {
-      process.nextTick(callback)
+  _write (chunk, encoding, callback) {
+    this._firstWrite = true
+    if (!this._body) {
+      this._body = new SlurpStream()
+      this._body.on('finish', () => {
+        this._urlLoaderOptions.body = this._body.data()
+        this._startRequest()
+      })
     }
-
-    return result
+    // TODO: is this the right way to forward to another stream?
+    this._body.write(chunk, encoding, callback)
   }
 
-  write (data, encoding, callback) {
-    if (this.urlRequest.finished) {
-      const error = new Error('Write after end')
-      process.nextTick(writeAfterEndNT, this, error, callback)
-      return true
+  _final (callback) {
+    if (this._body) {
+      // TODO: is this the right way to forward to another stream?
+      this._body.end(callback)
+    } else {
+      // end() called without a body, go ahead and start the request
+      this._startRequest()
+      callback()
     }
-
-    return this._write(data, encoding, callback, false)
   }
 
-  end (data, encoding, callback) {
-    if (this.urlRequest.finished) {
-      return false
+  _startRequest () {
+    this._started = true
+    const stringifyValues = (obj) => {
+      const ret = {}
+      for (const k in obj) {
+        ret[k] = obj[k].toString()
+      }
+      return ret
     }
+    const opts = { ...this._urlLoaderOptions, extraHeaders: stringifyValues(this._urlLoaderOptions.extraHeaders) }
+    this._urlLoader = new URLLoader(opts)
+    this._urlLoader.on('response-started', (event, finalUrl, responseHead) => {
+      const response = this._response = new IncomingMessage(responseHead)
+      this.emit('response', response)
+    })
+    this._urlLoader.on('data', (event, data) => {
+      this._response._storeInternalData(Buffer.from(data))
+    })
+    this._urlLoader.on('complete', () => {
+      if (this._response) { this._response._storeInternalData(null) }
+    })
+    this._urlLoader.on('error', (event, netErrorString) => {
+      const error = new Error(netErrorString)
+      if (this._response) this._response.destroy(error)
+      this._die(error)
+    })
 
-    if (typeof data === 'function') {
-      callback = data
-      encoding = null
-      data = null
-    } else if (typeof encoding === 'function') {
-      callback = encoding
-      encoding = null
-    }
+    this._urlLoader.on('login', (event, authInfo, callback) => {
+      const handled = this.emit('login', authInfo, callback)
+      if (!handled) {
+        // If there were no listeners, cancel the authentication request.
+        callback()
+      }
+    })
+
+    this._urlLoader.on('redirect', (event, redirectInfo, headers) => {
+      const { statusCode, newMethod, newUrl } = redirectInfo
+      if (this._redirectPolicy === 'error') {
+        this._die(new Error(`Attempted to redirect, but redirect policy was 'error'`))
+      } else if (this._redirectPolicy === 'manual') {
+        let _followRedirect = false
+        this._followRedirectCb = () => { _followRedirect = true }
+        try {
+          this.emit('redirect', statusCode, newMethod, newUrl, headers)
+        } finally {
+          this._followRedirectCb = null
+          if (!_followRedirect) {
+            this._die(new Error('Redirect was cancelled'))
+          }
+        }
+      } else if (this._redirectPolicy === 'follow') {
+        // Calling followRedirect() when the redirect policy is 'follow' is
+        // allowed but does nothing. (Perhaps it should throw an error
+        // though...? Since the redirect will happen regardless.)
+        try {
+          this._followRedirectCb = () => {}
+          this.emit('redirect', statusCode, newMethod, newUrl, headers)
+        } finally {
+          this._followRedirectCb = null
+        }
+      } else {
+        this._die(new Error(`Unexpected redirect policy '${this._redirectPolicy}'`))
+      }
+    })
 
-    data = data || ''
+    this._urlLoader.on('upload-progress', (event, position, total) => {
+      this._uploadProgress = { active: true, started: true, current: position, total }
+      this.emit('upload-progress', position, total) // Undocumented, for now
+    })
 
-    return this._write(data, encoding, callback, true)
+    this._urlLoader.on('download-progress', (event, current) => {
+      if (this._response) {
+        this._response.emit('download-progress', current) // Undocumented, for now
+      }
+    })
   }
 
   followRedirect () {
-    this.urlRequest.followRedirect()
+    if (this._followRedirectCb) {
+      this._followRedirectCb()
+    } else {
+      throw new Error('followRedirect() called, but was not waiting for a redirect')
+    }
   }
 
   abort () {
-    this.urlRequest.cancel()
+    if (!this._aborted) {
+      process.nextTick(() => { this.emit('abort') })
+    }
+    this._aborted = true
+    this._die()
   }
 
-  getUploadProgress () {
-    return this.urlRequest.getUploadProgress()
+  _die (err) {
+    this.destroy(err)
+    if (this._urlLoader) {
+      this._urlLoader.cancel()
+      if (this._response) this._response.destroy(err)
+    }
   }
-}
 
-function writeAfterEndNT (self, error, callback) {
-  self.emit('error', error)
-  if (callback) callback(error)
+  getUploadProgress () {
+    return { ...this._uploadProgress } || { active: false }
+  }
 }
 
 Net.prototype.request = function (options, callback) {

shell/browser/api/atom_api_net.cc

@@ -4,9 +4,11 @@
 
 #include "shell/browser/api/atom_api_net.h"
 
+#include <string>
+
 #include "gin/handle.h"
 #include "services/network/public/cpp/features.h"
-#include "shell/browser/api/atom_api_url_request.h"
+#include "shell/browser/api/atom_api_url_loader.h"
 #include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/gin_helper/object_template_builder.h"
 
@@ -32,12 +34,12 @@ void Net::BuildPrototype(v8::Isolate* isolate,
                          v8::Local<v8::FunctionTemplate> prototype) {
   prototype->SetClassName(gin::StringToV8(isolate, "Net"));
   gin_helper::ObjectTemplateBuilder(isolate, prototype->PrototypeTemplate())
-      .SetProperty("URLRequest", &Net::URLRequest);
+      .SetProperty("URLLoader", &Net::URLLoader);
 }
 
-v8::Local<v8::Value> Net::URLRequest(v8::Isolate* isolate) {
+v8::Local<v8::Value> Net::URLLoader(v8::Isolate* isolate) {
   v8::Local<v8::FunctionTemplate> constructor;
-  constructor = URLRequest::GetConstructor(isolate);
+  constructor = SimpleURLLoaderWrapper::GetConstructor(isolate);
   return constructor->GetFunction(isolate->GetCurrentContext())
       .ToLocalChecked();
 }
@@ -48,8 +50,16 @@ v8::Local<v8::Value> Net::URLRequest(v8::Isolate* isolate) {
 
 namespace {
 
+bool IsValidHeaderName(std::string header_name) {
+  return net::HttpUtil::IsValidHeaderName(header_name);
+}
+
+bool IsValidHeaderValue(std::string header_value) {
+  return net::HttpUtil::IsValidHeaderValue(header_value);
+}
+
 using electron::api::Net;
-using electron::api::URLRequest;
+using electron::api::SimpleURLLoaderWrapper;
 
 void Initialize(v8::Local<v8::Object> exports,
                 v8::Local<v8::Value> unused,
@@ -57,12 +67,15 @@ void Initialize(v8::Local<v8::Object> exports,
                 void* priv) {
   v8::Isolate* isolate = context->GetIsolate();
 
-  URLRequest::SetConstructor(isolate, base::BindRepeating(URLRequest::New));
+  SimpleURLLoaderWrapper::SetConstructor(
+      isolate, base::BindRepeating(SimpleURLLoaderWrapper::New));
 
   gin_helper::Dictionary dict(isolate, exports);
   dict.Set("net", Net::Create(isolate));
   dict.Set("Net",
            Net::GetConstructor(isolate)->GetFunction(context).ToLocalChecked());
+  dict.SetMethod("_isValidHeaderName", &IsValidHeaderName);
+  dict.SetMethod("_isValidHeaderValue", &IsValidHeaderValue);
 }
 
 }  // namespace

shell/browser/api/atom_api_net.h

@@ -18,7 +18,7 @@ class Net : public mate::Wrappable<Net> {
   static void BuildPrototype(v8::Isolate* isolate,
                              v8::Local<v8::FunctionTemplate> prototype);
 
-  v8::Local<v8::Value> URLRequest(v8::Isolate* isolate);
+  v8::Local<v8::Value> URLLoader(v8::Isolate* isolate);
 
  protected:
   explicit Net(v8::Isolate* isolate);

shell/browser/api/atom_api_url_loader.cc

@@ -0,0 +1,448 @@
+// Copyright (c) 2019 Slack Technologies, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#include "shell/browser/api/atom_api_url_loader.h"
+
+#include <algorithm>
+#include <map>
+#include <memory>
+#include <string>
+#include <utility>
+#include <vector>
+
+#include "base/containers/id_map.h"
+#include "gin/handle.h"
+#include "gin/object_template_builder.h"
+#include "gin/wrappable.h"
+#include "mojo/public/cpp/system/data_pipe_producer.h"
+#include "services/network/public/cpp/resource_request.h"
+#include "services/network/public/cpp/simple_url_loader.h"
+#include "services/network/public/mojom/chunked_data_pipe_getter.mojom.h"
+#include "services/network/public/mojom/url_loader_factory.mojom.h"
+#include "shell/browser/api/atom_api_session.h"
+#include "shell/browser/atom_browser_context.h"
+#include "shell/common/gin_converters/callback_converter.h"
+#include "shell/common/gin_converters/gurl_converter.h"
+#include "shell/common/gin_converters/net_converter.h"
+#include "shell/common/gin_helper/dictionary.h"
+#include "shell/common/gin_helper/object_template_builder.h"
+#include "shell/common/node_includes.h"
+
+class BufferDataSource : public mojo::DataPipeProducer::DataSource {
+ public:
+  explicit BufferDataSource(base::span<char> buffer) {
+    buffer_.resize(buffer.size());
+    memcpy(buffer_.data(), buffer.data(), buffer_.size());
+  }
+  ~BufferDataSource() override = default;
+
+ private:
+  // mojo::DataPipeProducer::DataSource:
+  uint64_t GetLength() const override { return buffer_.size(); }
+  ReadResult Read(uint64_t offset, base::span<char> buffer) override {
+    ReadResult result;
+    if (offset <= buffer_.size()) {
+      size_t readable_size = buffer_.size() - offset;
+      size_t writable_size = buffer.size();
+      size_t copyable_size = std::min(readable_size, writable_size);
+      memcpy(buffer.data(), &buffer_[offset], copyable_size);
+      result.bytes_read = copyable_size;
+    } else {
+      NOTREACHED();
+      result.result = MOJO_RESULT_OUT_OF_RANGE;
+    }
+    return result;
+  }
+
+  std::vector<char> buffer_;
+};
+
+class JSChunkedDataPipeGetter : public gin::Wrappable<JSChunkedDataPipeGetter>,
+                                public network::mojom::ChunkedDataPipeGetter {
+ public:
+  static gin::Handle<JSChunkedDataPipeGetter> Create(
+      v8::Isolate* isolate,
+      v8::Local<v8::Function> body_func,
+      mojo::PendingReceiver<network::mojom::ChunkedDataPipeGetter>
+          chunked_data_pipe_getter) {
+    return gin::CreateHandle(
+        isolate, new JSChunkedDataPipeGetter(
+                     isolate, body_func, std::move(chunked_data_pipe_getter)));
+  }
+
+  // gin::Wrappable
+  gin::ObjectTemplateBuilder GetObjectTemplateBuilder(
+      v8::Isolate* isolate) override {
+    return gin::Wrappable<JSChunkedDataPipeGetter>::GetObjectTemplateBuilder(
+               isolate)
+        .SetMethod("write", &JSChunkedDataPipeGetter::WriteChunk)
+        .SetMethod("done", &JSChunkedDataPipeGetter::Done);
+  }
+
+  static gin::WrapperInfo kWrapperInfo;
+  ~JSChunkedDataPipeGetter() override = default;
+
+ private:
+  JSChunkedDataPipeGetter(
+      v8::Isolate* isolate,
+      v8::Local<v8::Function> body_func,
+      mojo::PendingReceiver<network::mojom::ChunkedDataPipeGetter>
+          chunked_data_pipe_getter)
+      : isolate_(isolate), body_func_(isolate, body_func) {
+    receiver_.Bind(std::move(chunked_data_pipe_getter));
+  }
+
+  // network::mojom::ChunkedDataPipeGetter:
+  void GetSize(GetSizeCallback callback) override {
+    size_callback_ = std::move(callback);
+  }
+
+  void StartReading(mojo::ScopedDataPipeProducerHandle pipe) override {
+    DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+    if (body_func_.IsEmpty()) {
+      LOG(ERROR) << "Tried to read twice from a JSChunkedDataPipeGetter";
+      // Drop the handle on the floor.
+      return;
+    }
+    data_producer_ = std::make_unique<mojo::DataPipeProducer>(std::move(pipe));
+
+    v8::HandleScope handle_scope(isolate_);
+    v8::MicrotasksScope script_scope(isolate_,
+                                     v8::MicrotasksScope::kRunMicrotasks);
+    auto maybe_wrapper = GetWrapper(isolate_);
+    v8::Local<v8::Value> wrapper;
+    if (!maybe_wrapper.ToLocal(&wrapper)) {
+      return;
+    }
+    v8::Local<v8::Value> argv[] = {wrapper};
+    node::Environment* env = node::Environment::GetCurrent(isolate_);
+    auto global = env->context()->Global();
+    node::MakeCallback(isolate_, global, body_func_.Get(isolate_),
+                       node::arraysize(argv), argv, {0, 0});
+  }
+
+  v8::Local<v8::Promise> WriteChunk(v8::Local<v8::Value> buffer_val) {
+    gin_helper::Promise<void> promise(isolate_);
+    v8::Local<v8::Promise> handle = promise.GetHandle();
+    if (!buffer_val->IsArrayBufferView()) {
+      promise.RejectWithErrorMessage("Expected an ArrayBufferView");
+      return handle;
+    }
+    if (is_writing_) {
+      promise.RejectWithErrorMessage("Only one write can be pending at a time");
+      return handle;
+    }
+    if (!size_callback_) {
+      promise.RejectWithErrorMessage("Can't write after calling done()");
+      return handle;
+    }
+    auto buffer = buffer_val.As<v8::ArrayBufferView>();
+    is_writing_ = true;
+    bytes_written_ += buffer->ByteLength();
+    auto backing_store = buffer->Buffer()->GetBackingStore();
+    auto buffer_span = base::make_span(
+        static_cast<char*>(backing_store->Data()) + buffer->ByteOffset(),
+        buffer->ByteLength());
+    auto buffer_source = std::make_unique<BufferDataSource>(buffer_span);
+    data_producer_->Write(
+        std::move(buffer_source),
+        base::BindOnce(&JSChunkedDataPipeGetter::OnWriteChunkComplete,
+                       // We're OK to use Unretained here because we own
+                       // |data_producer_|.
+                       base::Unretained(this), std::move(promise)));
+    return handle;
+  }
+
+  void OnWriteChunkComplete(gin_helper::Promise<void> promise,
+                            MojoResult result) {
+    DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+    is_writing_ = false;
+    if (result == MOJO_RESULT_OK) {
+      promise.Resolve();
+    } else {
+      promise.RejectWithErrorMessage("mojo result not ok");
+      Finished();
+    }
+  }
+
+  // TODO(nornagon): accept a net error here to allow the data provider to
+  // cancel the request with an error.
+  void Done() {
+    if (size_callback_) {
+      std::move(size_callback_).Run(net::OK, bytes_written_);
+      Finished();
+    }
+  }
+
+  void Finished() {
+    size_callback_.Reset();
+    body_func_.Reset();
+    receiver_.reset();
+    data_producer_.reset();
+  }
+
+  GetSizeCallback size_callback_;
+  mojo::Receiver<network::mojom::ChunkedDataPipeGetter> receiver_{this};
+  std::unique_ptr<mojo::DataPipeProducer> data_producer_;
+  bool is_writing_ = false;
+  uint64_t bytes_written_ = 0;
+
+  v8::Isolate* isolate_;
+  v8::Global<v8::Function> body_func_;
+};
+
+gin::WrapperInfo JSChunkedDataPipeGetter::kWrapperInfo = {
+    gin::kEmbedderNativeGin};
+
+namespace electron {
+
+namespace api {
+
+namespace {
+
+const net::NetworkTrafficAnnotationTag kTrafficAnnotation =
+    net::DefineNetworkTrafficAnnotation("electron_net_module", R"(
+        semantics {
+          sender: "Electron Net module"
+          description:
+            "Issue HTTP/HTTPS requests using Chromium's native networking "
+            "library."
+          trigger: "Using the Net module"
+          data: "Anything the user wants to send."
+          destination: OTHER
+        }
+        policy {
+          cookies_allowed: YES
+          cookies_store: "user"
+          setting: "This feature cannot be disabled."
+        })");
+
+base::IDMap<SimpleURLLoaderWrapper*>& GetAllRequests() {
+  static base::NoDestructor<base::IDMap<SimpleURLLoaderWrapper*>>
+      s_all_requests;
+  return *s_all_requests;
+}
+
+}  // namespace
+
+SimpleURLLoaderWrapper::SimpleURLLoaderWrapper(
+    std::unique_ptr<network::ResourceRequest> request,
+    network::mojom::URLLoaderFactory* url_loader_factory)
+    : id_(GetAllRequests().Add(this)) {
+  // We slightly abuse the |render_frame_id| field in ResourceRequest so that
+  // we can correlate any authentication events that arrive with this request.
+  request->render_frame_id = id_;
+
+  // SimpleURLLoader wants to control the request body itself. We have other
+  // ideas.
+  auto request_body = std::move(request->request_body);
+  auto* request_ref = request.get();
+  loader_ =
+      network::SimpleURLLoader::Create(std::move(request), kTrafficAnnotation);
+  if (request_body) {
+    request_ref->request_body = std::move(request_body);
+  }
+
+  loader_->SetAllowHttpErrorResults(true);
+  loader_->SetOnResponseStartedCallback(base::BindOnce(
+      &SimpleURLLoaderWrapper::OnResponseStarted, base::Unretained(this)));
+  loader_->SetOnRedirectCallback(base::BindRepeating(
+      &SimpleURLLoaderWrapper::OnRedirect, base::Unretained(this)));
+  loader_->SetOnUploadProgressCallback(base::BindRepeating(
+      &SimpleURLLoaderWrapper::OnUploadProgress, base::Unretained(this)));
+  loader_->SetOnDownloadProgressCallback(base::BindRepeating(
+      &SimpleURLLoaderWrapper::OnDownloadProgress, base::Unretained(this)));
+
+  loader_->DownloadAsStream(url_loader_factory, this);
+}
+
+void SimpleURLLoaderWrapper::Pin() {
+  // Prevent ourselves from being GC'd until the request is complete.
+  // Must be called after InitWithArgs(), otherwise GetWrapper() isn't
+  // initialized.
+  pinned_wrapper_.Reset(isolate(), GetWrapper());
+}
+
+void SimpleURLLoaderWrapper::PinBodyGetter(v8::Local<v8::Value> body_getter) {
+  pinned_chunk_pipe_getter_.Reset(isolate(), body_getter);
+}
+
+SimpleURLLoaderWrapper::~SimpleURLLoaderWrapper() {
+  GetAllRequests().Remove(id_);
+}
+
+// static
+SimpleURLLoaderWrapper* SimpleURLLoaderWrapper::FromID(uint32_t id) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  return GetAllRequests().Lookup(id);
+}
+
+void SimpleURLLoaderWrapper::OnAuthRequired(
+    const GURL& url,
+    bool first_auth_attempt,
+    net::AuthChallengeInfo auth_info,
+    network::mojom::URLResponseHeadPtr head,
+    mojo::PendingRemote<network::mojom::AuthChallengeResponder>
+        auth_challenge_responder) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  mojo::Remote<network::mojom::AuthChallengeResponder> auth_responder(
+      std::move(auth_challenge_responder));
+  // WeakPtr because if we're Cancel()ed while waiting for auth, and the
+  // network service also decides to cancel at the same time and kill this
+  // pipe, we might end up trying to call Cancel again on dead memory.
+  auth_responder.set_disconnect_handler(base::BindOnce(
+      &SimpleURLLoaderWrapper::Cancel, weak_factory_.GetWeakPtr()));
+  auto cb = base::BindOnce(
+      [](mojo::Remote<network::mojom::AuthChallengeResponder> auth_responder,
+         gin::Arguments* args) {
+        base::string16 username_str, password_str;
+        if (!args->GetNext(&username_str) || !args->GetNext(&password_str)) {
+          auth_responder->OnAuthCredentials(base::nullopt);
+          return;
+        }
+        auth_responder->OnAuthCredentials(
+            net::AuthCredentials(username_str, password_str));
+      },
+      std::move(auth_responder));
+  Emit("login", auth_info, base::AdaptCallbackForRepeating(std::move(cb)));
+}
+
+void SimpleURLLoaderWrapper::Cancel() {
+  loader_.reset();
+  pinned_wrapper_.Reset();
+  pinned_chunk_pipe_getter_.Reset();
+  // This ensures that no further callbacks will be called, so there's no need
+  // for additional guards.
+}
+
+// static
+mate::WrappableBase* SimpleURLLoaderWrapper::New(gin::Arguments* args) {
+  gin_helper::Dictionary opts;
+  if (!args->GetNext(&opts)) {
+    args->ThrowTypeError("Expected a dictionary");
+    return nullptr;
+  }
+  auto request = std::make_unique<network::ResourceRequest>();
+  opts.Get("method", &request->method);
+  opts.Get("url", &request->url);
+  std::map<std::string, std::string> extra_headers;
+  if (opts.Get("extraHeaders", &extra_headers)) {
+    for (const auto& it : extra_headers) {
+      if (!net::HttpUtil::IsValidHeaderName(it.first) ||
+          !net::HttpUtil::IsValidHeaderValue(it.second)) {
+        args->ThrowTypeError("Invalid header name or value");
+        return nullptr;
+      }
+      request->headers.SetHeader(it.first, it.second);
+    }
+  }
+
+  v8::Local<v8::Value> body;
+  v8::Local<v8::Value> chunk_pipe_getter;
+  if (opts.Get("body", &body)) {
+    if (body->IsArrayBufferView()) {
+      auto buffer_body = body.As<v8::ArrayBufferView>();
+      auto backing_store = buffer_body->Buffer()->GetBackingStore();
+      request->request_body = network::ResourceRequestBody::CreateFromBytes(
+          static_cast<char*>(backing_store->Data()) + buffer_body->ByteOffset(),
+          buffer_body->ByteLength());
+    } else if (body->IsFunction()) {
+      auto body_func = body.As<v8::Function>();
+
+      mojo::PendingRemote<network::mojom::ChunkedDataPipeGetter>
+          data_pipe_getter;
+      chunk_pipe_getter = JSChunkedDataPipeGetter::Create(
+                              args->isolate(), body_func,
+                              data_pipe_getter.InitWithNewPipeAndPassReceiver())
+                              .ToV8();
+      request->request_body = new network::ResourceRequestBody();
+      request->request_body->SetToChunkedDataPipe(std::move(data_pipe_getter));
+    }
+  }
+
+  std::string partition;
+  gin::Handle<Session> session;
+  if (!opts.Get("session", &session)) {
+    if (opts.Get("partition", &partition))
+      session = Session::FromPartition(args->isolate(), partition);
+    else  // default session
+      session = Session::FromPartition(args->isolate(), "");
+  }
+
+  auto url_loader_factory = session->browser_context()->GetURLLoaderFactory();
+
+  auto* ret =
+      new SimpleURLLoaderWrapper(std::move(request), url_loader_factory.get());
+  ret->InitWithArgs(args);
+  ret->Pin();
+  if (!chunk_pipe_getter.IsEmpty()) {
+    ret->PinBodyGetter(chunk_pipe_getter);
+  }
+  return ret;
+}
+
+void SimpleURLLoaderWrapper::OnDataReceived(base::StringPiece string_piece,
+                                            base::OnceClosure resume) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  v8::HandleScope handle_scope(isolate());
+  auto array_buffer = v8::ArrayBuffer::New(isolate(), string_piece.size());
+  auto backing_store = array_buffer->GetBackingStore();
+  memcpy(backing_store->Data(), string_piece.data(), string_piece.size());
+  Emit("data", array_buffer);
+  std::move(resume).Run();
+}
+
+void SimpleURLLoaderWrapper::OnComplete(bool success) {
+  if (success) {
+    Emit("complete");
+  } else {
+    Emit("error", net::ErrorToString(loader_->NetError()));
+  }
+  loader_.reset();
+  pinned_wrapper_.Reset();
+  pinned_chunk_pipe_getter_.Reset();
+}
+
+void SimpleURLLoaderWrapper::OnRetry(base::OnceClosure start_retry) {}
+
+void SimpleURLLoaderWrapper::OnResponseStarted(
+    const GURL& final_url,
+    const network::mojom::URLResponseHead& response_head) {
+  gin::Dictionary dict = gin::Dictionary::CreateEmpty(isolate());
+  dict.Set("statusCode", response_head.headers->response_code());
+  dict.Set("statusMessage", response_head.headers->GetStatusText());
+  dict.Set("headers", response_head.headers.get());
+  dict.Set("httpVersion", response_head.headers->GetHttpVersion());
+  Emit("response-started", final_url, dict);
+}
+
+void SimpleURLLoaderWrapper::OnRedirect(
+    const net::RedirectInfo& redirect_info,
+    const network::mojom::URLResponseHead& response_head,
+    std::vector<std::string>* removed_headers) {
+  Emit("redirect", redirect_info, response_head.headers.get());
+}
+
+void SimpleURLLoaderWrapper::OnUploadProgress(uint64_t position,
+                                              uint64_t total) {
+  Emit("upload-progress", position, total);
+}
+
+void SimpleURLLoaderWrapper::OnDownloadProgress(uint64_t current) {
+  Emit("download-progress", current);
+}
+
+// static
+void SimpleURLLoaderWrapper::BuildPrototype(
+    v8::Isolate* isolate,
+    v8::Local<v8::FunctionTemplate> prototype) {
+  prototype->SetClassName(gin::StringToV8(isolate, "SimpleURLLoaderWrapper"));
+  gin_helper::ObjectTemplateBuilder(isolate, prototype->PrototypeTemplate())
+      .SetMethod("cancel", &SimpleURLLoaderWrapper::Cancel);
+}
+
+}  // namespace api
+
+}  // namespace electron

shell/browser/api/atom_api_url_loader.h

@@ -0,0 +1,93 @@
+// Copyright (c) 2019 Slack Technologies, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#ifndef SHELL_BROWSER_API_ATOM_API_URL_LOADER_H_
+#define SHELL_BROWSER_API_ATOM_API_URL_LOADER_H_
+
+#include <memory>
+#include <string>
+#include <vector>
+
+#include "base/memory/weak_ptr.h"
+#include "net/base/auth.h"
+#include "services/network/public/cpp/simple_url_loader_stream_consumer.h"
+#include "services/network/public/mojom/network_context.mojom.h"
+#include "services/network/public/mojom/url_loader_factory.mojom-forward.h"
+#include "services/network/public/mojom/url_response_head.mojom.h"
+#include "shell/common/gin_helper/event_emitter.h"
+#include "url/gurl.h"
+#include "v8/include/v8.h"
+
+namespace gin {
+class Arguments;
+}
+
+namespace network {
+class SimpleURLLoader;
+struct ResourceRequest;
+}  // namespace network
+
+namespace electron {
+
+namespace api {
+
+/** Wraps a SimpleURLLoader to make it usable from JavaScript */
+class SimpleURLLoaderWrapper
+    : public gin_helper::EventEmitter<SimpleURLLoaderWrapper>,
+      public network::SimpleURLLoaderStreamConsumer {
+ public:
+  ~SimpleURLLoaderWrapper() override;
+  static mate::WrappableBase* New(gin::Arguments* args);
+
+  static void BuildPrototype(v8::Isolate* isolate,
+                             v8::Local<v8::FunctionTemplate> prototype);
+
+  static SimpleURLLoaderWrapper* FromID(uint32_t id);
+
+  void OnAuthRequired(
+      const GURL& url,
+      bool first_auth_attempt,
+      net::AuthChallengeInfo auth_info,
+      network::mojom::URLResponseHeadPtr head,
+      mojo::PendingRemote<network::mojom::AuthChallengeResponder>
+          auth_challenge_responder);
+
+  void Cancel();
+
+ private:
+  SimpleURLLoaderWrapper(std::unique_ptr<network::ResourceRequest> loader,
+                         network::mojom::URLLoaderFactory* url_loader_factory);
+
+  // SimpleURLLoaderStreamConsumer:
+  void OnDataReceived(base::StringPiece string_piece,
+                      base::OnceClosure resume) override;
+  void OnComplete(bool success) override;
+  void OnRetry(base::OnceClosure start_retry) override;
+
+  // SimpleURLLoader callbacks
+  void OnResponseStarted(const GURL& final_url,
+                         const network::mojom::URLResponseHead& response_head);
+  void OnRedirect(const net::RedirectInfo& redirect_info,
+                  const network::mojom::URLResponseHead& response_head,
+                  std::vector<std::string>* removed_headers);
+  void OnUploadProgress(uint64_t position, uint64_t total);
+  void OnDownloadProgress(uint64_t current);
+
+  void Start();
+  void Pin();
+  void PinBodyGetter(v8::Local<v8::Value>);
+
+  uint32_t id_;
+  std::unique_ptr<network::SimpleURLLoader> loader_;
+  v8::Global<v8::Value> pinned_wrapper_;
+  v8::Global<v8::Value> pinned_chunk_pipe_getter_;
+
+  base::WeakPtrFactory<SimpleURLLoaderWrapper> weak_factory_{this};
+};
+
+}  // namespace api
+
+}  // namespace electron
+
+#endif  // SHELL_BROWSER_API_ATOM_API_URL_LOADER_H_

shell/browser/api/atom_api_url_request.cc

@@ -1,597 +0,0 @@
-// 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 "shell/browser/api/atom_api_url_request.h"
-
-#include <utility>
-
-#include "base/containers/id_map.h"
-#include "gin/handle.h"
-#include "mojo/public/cpp/bindings/receiver_set.h"
-#include "mojo/public/cpp/system/string_data_source.h"
-#include "net/http/http_util.h"
-#include "services/network/public/mojom/chunked_data_pipe_getter.mojom.h"
-#include "shell/browser/api/atom_api_session.h"
-#include "shell/browser/atom_browser_context.h"
-#include "shell/common/gin_converters/callback_converter.h"
-#include "shell/common/gin_converters/gurl_converter.h"
-#include "shell/common/gin_converters/net_converter.h"
-#include "shell/common/gin_helper/dictionary.h"
-#include "shell/common/gin_helper/event_emitter_caller.h"
-#include "shell/common/gin_helper/object_template_builder.h"
-
-#include "shell/common/node_includes.h"
-
-namespace gin {
-
-template <>
-struct Converter<network::mojom::RedirectMode> {
-  static bool FromV8(v8::Isolate* isolate,
-                     v8::Local<v8::Value> val,
-                     network::mojom::RedirectMode* out) {
-    std::string mode;
-    if (!ConvertFromV8(isolate, val, &mode))
-      return false;
-    if (mode == "follow")
-      *out = network::mojom::RedirectMode::kFollow;
-    else if (mode == "error")
-      *out = network::mojom::RedirectMode::kError;
-    else if (mode == "manual")
-      *out = network::mojom::RedirectMode::kManual;
-    else
-      return false;
-    return true;
-  }
-};
-
-}  // namespace gin
-
-namespace electron {
-
-namespace api {
-
-namespace {
-
-base::IDMap<URLRequest*>& GetAllRequests() {
-  static base::NoDestructor<base::IDMap<URLRequest*>> s_all_requests;
-  return *s_all_requests;
-}
-
-// Network state for request and response.
-enum State {
-  STATE_STARTED = 1 << 0,
-  STATE_FINISHED = 1 << 1,
-  STATE_CANCELED = 1 << 2,
-  STATE_FAILED = 1 << 3,
-  STATE_CLOSED = 1 << 4,
-  STATE_ERROR = STATE_CANCELED | STATE_FAILED | STATE_CLOSED,
-};
-
-// Annotation tag passed to NetworkService.
-const net::NetworkTrafficAnnotationTag kTrafficAnnotation =
-    net::DefineNetworkTrafficAnnotation("electron_net_module", R"(
-        semantics {
-          sender: "Electron Net module"
-          description:
-            "Issue HTTP/HTTPS requests using Chromium's native networking "
-            "library."
-          trigger: "Using the Net module"
-          data: "Anything the user wants to send."
-          destination: OTHER
-        }
-        policy {
-          cookies_allowed: YES
-          cookies_store: "user"
-          setting: "This feature cannot be disabled."
-        })");
-
-}  // namespace
-
-// Common class for streaming data.
-class UploadDataPipeGetter {
- public:
-  explicit UploadDataPipeGetter(URLRequest* request) : request_(request) {}
-  virtual ~UploadDataPipeGetter() = default;
-
-  virtual void AttachToRequestBody(network::ResourceRequestBody* body) = 0;
-
- protected:
-  void SetCallback(network::mojom::DataPipeGetter::ReadCallback callback) {
-    request_->size_callback_ = std::move(callback);
-  }
-
-  void SetPipe(mojo::ScopedDataPipeProducerHandle pipe) {
-    request_->producer_ =
-        std::make_unique<mojo::DataPipeProducer>(std::move(pipe));
-    request_->StartWriting();
-  }
-
- private:
-  URLRequest* request_;
-
-  DISALLOW_COPY_AND_ASSIGN(UploadDataPipeGetter);
-};
-
-// Streaming multipart data to NetworkService.
-class MultipartDataPipeGetter : public UploadDataPipeGetter,
-                                public network::mojom::DataPipeGetter {
- public:
-  explicit MultipartDataPipeGetter(URLRequest* request)
-      : UploadDataPipeGetter(request) {}
-  ~MultipartDataPipeGetter() override = default;
-
-  void AttachToRequestBody(network::ResourceRequestBody* body) override {
-    mojo::PendingRemote<network::mojom::DataPipeGetter> data_pipe_getter_remote;
-    receivers_.Add(this,
-                   data_pipe_getter_remote.InitWithNewPipeAndPassReceiver());
-    body->AppendDataPipe(std::move(data_pipe_getter_remote));
-  }
-
- private:
-  // network::mojom::DataPipeGetter:
-  void Read(mojo::ScopedDataPipeProducerHandle pipe,
-            ReadCallback callback) override {
-    SetCallback(std::move(callback));
-    SetPipe(std::move(pipe));
-  }
-
-  void Clone(
-      mojo::PendingReceiver<network::mojom::DataPipeGetter> receiver) override {
-    receivers_.Add(this, std::move(receiver));
-  }
-
-  mojo::ReceiverSet<network::mojom::DataPipeGetter> receivers_;
-};
-
-// Streaming chunked data to NetworkService.
-class ChunkedDataPipeGetter : public UploadDataPipeGetter,
-                              public network::mojom::ChunkedDataPipeGetter {
- public:
-  explicit ChunkedDataPipeGetter(URLRequest* request)
-      : UploadDataPipeGetter(request) {}
-  ~ChunkedDataPipeGetter() override = default;
-
-  void AttachToRequestBody(network::ResourceRequestBody* body) override {
-    mojo::PendingRemote<network::mojom::ChunkedDataPipeGetter>
-        data_pipe_getter_remote;
-    receiver_set_.Add(this,
-                      data_pipe_getter_remote.InitWithNewPipeAndPassReceiver());
-    body->SetToChunkedDataPipe(std::move(data_pipe_getter_remote));
-  }
-
- private:
-  // network::mojom::ChunkedDataPipeGetter:
-  void GetSize(GetSizeCallback callback) override {
-    SetCallback(std::move(callback));
-  }
-
-  void StartReading(mojo::ScopedDataPipeProducerHandle pipe) override {
-    SetPipe(std::move(pipe));
-  }
-
-  mojo::ReceiverSet<network::mojom::ChunkedDataPipeGetter> receiver_set_;
-};
-
-URLRequest::URLRequest(gin::Arguments* args)
-    : id_(GetAllRequests().Add(this)), weak_factory_(this) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  request_ = std::make_unique<network::ResourceRequest>();
-  gin_helper::Dictionary dict;
-  if (args->GetNext(&dict)) {
-    dict.Get("method", &request_->method);
-    dict.Get("url", &request_->url);
-    dict.Get("redirect", &redirect_mode_);
-    request_->redirect_mode = redirect_mode_;
-  }
-
-  request_->render_frame_id = id_;
-
-  std::string partition;
-  gin::Handle<api::Session> session;
-  if (!dict.Get("session", &session)) {
-    if (dict.Get("partition", &partition))
-      session = Session::FromPartition(args->isolate(), partition);
-    else  // default session
-      session = Session::FromPartition(args->isolate(), "");
-  }
-
-  url_loader_factory_ = session->browser_context()->GetURLLoaderFactory();
-
-  InitWithArgs(args);
-}
-
-URLRequest::~URLRequest() = default;
-
-URLRequest* URLRequest::FromID(uint32_t id) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  return GetAllRequests().Lookup(id);
-}
-
-void URLRequest::OnAuthRequired(
-    const GURL& url,
-    bool first_auth_attempt,
-    net::AuthChallengeInfo auth_info,
-    network::mojom::URLResponseHeadPtr head,
-    mojo::PendingRemote<network::mojom::AuthChallengeResponder>
-        auth_challenge_responder) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  mojo::Remote<network::mojom::AuthChallengeResponder> auth_responder(
-      std::move(auth_challenge_responder));
-  auth_responder.set_disconnect_handler(
-      base::BindOnce(&URLRequest::Cancel, weak_factory_.GetWeakPtr()));
-  auto cb = base::BindOnce(
-      [](mojo::Remote<network::mojom::AuthChallengeResponder> auth_responder,
-         gin::Arguments* args) {
-        base::string16 username_str, password_str;
-        if (!args->GetNext(&username_str) || !args->GetNext(&password_str)) {
-          auth_responder->OnAuthCredentials(base::nullopt);
-          return;
-        }
-        auth_responder->OnAuthCredentials(
-            net::AuthCredentials(username_str, password_str));
-      },
-      std::move(auth_responder));
-  Emit("login", auth_info, base::AdaptCallbackForRepeating(std::move(cb)));
-}
-
-bool URLRequest::NotStarted() const {
-  return request_state_ == 0;
-}
-
-bool URLRequest::Finished() const {
-  return request_state_ & STATE_FINISHED;
-}
-
-void URLRequest::Cancel() {
-  // Cancel only once.
-  if (request_state_ & (STATE_CANCELED | STATE_CLOSED))
-    return;
-
-  // Mark as canceled.
-  request_state_ |= STATE_CANCELED;
-  EmitEvent(EventType::kRequest, true, "abort");
-
-  if ((response_state_ & STATE_STARTED) && !(response_state_ & STATE_FINISHED))
-    EmitEvent(EventType::kResponse, true, "aborted");
-
-  Close();
-}
-
-void URLRequest::Close() {
-  if (!(request_state_ & STATE_CLOSED)) {
-    request_state_ |= STATE_CLOSED;
-    if (response_state_ & STATE_STARTED) {
-      // Emit a close event if we really have a response object.
-      EmitEvent(EventType::kResponse, true, "close");
-    }
-    EmitEvent(EventType::kRequest, true, "close");
-  }
-  Unpin();
-  loader_.reset();
-}
-
-bool URLRequest::Write(v8::Local<v8::Value> data, bool is_last) {
-  if (request_state_ & (STATE_FINISHED | STATE_ERROR))
-    return false;
-
-  size_t length = node::Buffer::Length(data);
-
-  if (!loader_) {
-    // Pin on first write.
-    request_state_ = STATE_STARTED;
-    Pin();
-
-    // Create the loader.
-    network::ResourceRequest* request_ref = request_.get();
-    loader_ = network::SimpleURLLoader::Create(std::move(request_),
-                                               kTrafficAnnotation);
-
-    loader_->SetAllowHttpErrorResults(true);
-    loader_->SetOnResponseStartedCallback(
-        base::Bind(&URLRequest::OnResponseStarted, weak_factory_.GetWeakPtr()));
-    loader_->SetOnRedirectCallback(
-        base::Bind(&URLRequest::OnRedirect, weak_factory_.GetWeakPtr()));
-    loader_->SetOnUploadProgressCallback(
-        base::Bind(&URLRequest::OnUploadProgress, weak_factory_.GetWeakPtr()));
-
-    // Create upload data pipe if we have data to write.
-    if (length > 0) {
-      request_ref->request_body = new network::ResourceRequestBody();
-      if (is_chunked_upload_)
-        data_pipe_getter_ = std::make_unique<ChunkedDataPipeGetter>(this);
-      else
-        data_pipe_getter_ = std::make_unique<MultipartDataPipeGetter>(this);
-      data_pipe_getter_->AttachToRequestBody(request_ref->request_body.get());
-    }
-
-    // Start downloading.
-    loader_->DownloadAsStream(url_loader_factory_.get(), this);
-  }
-
-  if (length > 0)
-    pending_writes_.emplace_back(node::Buffer::Data(data), length);
-
-  if (is_last) {
-    // The ElementsUploadDataStream requires the knowledge of content length
-    // before doing upload, while Node's stream does not give us any size
-    // information. So the only option left for us is to keep all the write
-    // data in memory and flush them after the write is done.
-    //
-    // While this looks frustrating, it is actually the behavior of the non-
-    // NetworkService implementation, and we are not breaking anything.
-    if (!pending_writes_.empty()) {
-      last_chunk_written_ = true;
-      StartWriting();
-    }
-
-    request_state_ |= STATE_FINISHED;
-    EmitEvent(EventType::kRequest, true, "finish");
-  }
-  return true;
-}
-
-void URLRequest::FollowRedirect() {
-  if (request_state_ & (STATE_CANCELED | STATE_CLOSED))
-    return;
-  follow_redirect_ = true;
-}
-
-bool URLRequest::SetExtraHeader(const std::string& name,
-                                const std::string& value) {
-  if (!request_)
-    return false;
-  if (!net::HttpUtil::IsValidHeaderName(name))
-    return false;
-  if (!net::HttpUtil::IsValidHeaderValue(value))
-    return false;
-  request_->headers.SetHeader(name, value);
-  return true;
-}
-
-void URLRequest::RemoveExtraHeader(const std::string& name) {
-  if (request_)
-    request_->headers.RemoveHeader(name);
-}
-
-void URLRequest::SetChunkedUpload(bool is_chunked_upload) {
-  if (request_)
-    is_chunked_upload_ = is_chunked_upload;
-}
-
-gin::Dictionary URLRequest::GetUploadProgress() {
-  gin::Dictionary progress = gin::Dictionary::CreateEmpty(isolate());
-  if (loader_) {
-    if (request_)
-      progress.Set("started", false);
-    else
-      progress.Set("started", true);
-    progress.Set("current", upload_position_);
-    progress.Set("total", upload_total_);
-    progress.Set("active", true);
-  } else {
-    progress.Set("active", false);
-  }
-  return progress;
-}
-
-int URLRequest::StatusCode() const {
-  if (response_headers_)
-    return response_headers_->response_code();
-  return -1;
-}
-
-std::string URLRequest::StatusMessage() const {
-  if (response_headers_)
-    return response_headers_->GetStatusText();
-  return "";
-}
-
-net::HttpResponseHeaders* URLRequest::RawResponseHeaders() const {
-  return response_headers_.get();
-}
-
-uint32_t URLRequest::ResponseHttpVersionMajor() const {
-  if (response_headers_)
-    return response_headers_->GetHttpVersion().major_value();
-  return 0;
-}
-
-uint32_t URLRequest::ResponseHttpVersionMinor() const {
-  if (response_headers_)
-    return response_headers_->GetHttpVersion().minor_value();
-  return 0;
-}
-
-void URLRequest::OnDataReceived(base::StringPiece data,
-                                base::OnceClosure resume) {
-  // In case we received an unexpected event from Chromium net, don't emit any
-  // data event after request cancel/error/close.
-  if (!(request_state_ & STATE_ERROR) && !(response_state_ & STATE_ERROR)) {
-    v8::HandleScope handle_scope(isolate());
-    v8::Local<v8::Value> buffer;
-    auto maybe = node::Buffer::Copy(isolate(), data.data(), data.size());
-    if (maybe.ToLocal(&buffer))
-      Emit("data", buffer);
-  }
-  std::move(resume).Run();
-}
-
-void URLRequest::OnRetry(base::OnceClosure start_retry) {}
-
-void URLRequest::OnComplete(bool success) {
-  if (success) {
-    // In case we received an unexpected event from Chromium net, don't emit any
-    // data event after request cancel/error/close.
-    if (!(request_state_ & STATE_ERROR) && !(response_state_ & STATE_ERROR)) {
-      response_state_ |= STATE_FINISHED;
-      Emit("end");
-    }
-  } else {  // failed
-    // If response is started then emit response event, else emit request error.
-    //
-    // Error is only emitted when there is no previous failure. This is to align
-    // with the behavior of non-NetworkService implementation.
-    std::string error = net::ErrorToString(loader_->NetError());
-    if (response_state_ & STATE_STARTED) {
-      if (!(response_state_ & STATE_FAILED))
-        EmitError(EventType::kResponse, error);
-    } else {
-      if (!(request_state_ & STATE_FAILED))
-        EmitError(EventType::kRequest, error);
-    }
-  }
-
-  Close();
-}
-
-void URLRequest::OnResponseStarted(
-    const GURL& final_url,
-    const network::mojom::URLResponseHead& response_head) {
-  // Don't emit any event after request cancel.
-  if (request_state_ & STATE_ERROR)
-    return;
-
-  response_headers_ = response_head.headers;
-  response_state_ |= STATE_STARTED;
-  Emit("response");
-}
-
-void URLRequest::OnRedirect(
-    const net::RedirectInfo& redirect_info,
-    const network::mojom::URLResponseHead& response_head,
-    std::vector<std::string>* to_be_removed_headers) {
-  if (!loader_)
-    return;
-
-  if (request_state_ & (STATE_CLOSED | STATE_CANCELED)) {
-    NOTREACHED();
-    Cancel();
-    return;
-  }
-
-  switch (redirect_mode_) {
-    case network::mojom::RedirectMode::kError:
-      Cancel();
-      EmitError(
-          EventType::kRequest,
-          "Request cannot follow redirect with the current redirect mode");
-      break;
-    case network::mojom::RedirectMode::kManual:
-      // When redirect mode is "manual", the user has to explicitly call the
-      // FollowRedirect method to continue redirecting, otherwise the request
-      // would be cancelled.
-      //
-      // Note that the SimpleURLLoader always calls FollowRedirect and does not
-      // provide a formal way for us to cancel redirection, we have to cancel
-      // the request to prevent the redirection.
-      follow_redirect_ = false;
-      EmitEvent(EventType::kRequest, false, "redirect",
-                redirect_info.status_code, redirect_info.new_method,
-                redirect_info.new_url, response_head.headers.get());
-      if (!follow_redirect_)
-        Cancel();
-      break;
-    case network::mojom::RedirectMode::kFollow:
-      EmitEvent(EventType::kRequest, false, "redirect",
-                redirect_info.status_code, redirect_info.new_method,
-                redirect_info.new_url, response_head.headers.get());
-      break;
-  }
-}
-
-void URLRequest::OnUploadProgress(uint64_t position, uint64_t total) {
-  upload_position_ = position;
-  upload_total_ = total;
-}
-
-void URLRequest::OnWrite(MojoResult result) {
-  if (result != MOJO_RESULT_OK)
-    return;
-
-  // Continue the pending writes.
-  pending_writes_.pop_front();
-  if (!pending_writes_.empty())
-    DoWrite();
-}
-
-void URLRequest::DoWrite() {
-  DCHECK(producer_);
-  DCHECK(!pending_writes_.empty());
-  producer_->Write(
-      std::make_unique<mojo::StringDataSource>(
-          pending_writes_.front(), mojo::StringDataSource::AsyncWritingMode::
-                                       STRING_STAYS_VALID_UNTIL_COMPLETION),
-      base::BindOnce(&URLRequest::OnWrite, weak_factory_.GetWeakPtr()));
-}
-
-void URLRequest::StartWriting() {
-  if (!last_chunk_written_ || size_callback_.is_null())
-    return;
-
-  size_t size = 0;
-  for (const auto& data : pending_writes_)
-    size += data.size();
-  std::move(size_callback_).Run(net::OK, size);
-  DoWrite();
-}
-
-void URLRequest::Pin() {
-  if (wrapper_.IsEmpty()) {
-    wrapper_.Reset(isolate(), GetWrapper());
-  }
-}
-
-void URLRequest::Unpin() {
-  wrapper_.Reset();
-}
-
-void URLRequest::EmitError(EventType type, base::StringPiece message) {
-  if (type == EventType::kRequest)
-    request_state_ |= STATE_FAILED;
-  else
-    response_state_ |= STATE_FAILED;
-  v8::HandleScope handle_scope(isolate());
-  auto error = v8::Exception::Error(gin::StringToV8(isolate(), message));
-  EmitEvent(type, false, "error", error);
-}
-
-template <typename... Args>
-void URLRequest::EmitEvent(EventType type, Args&&... args) {
-  const char* method =
-      type == EventType::kRequest ? "_emitRequestEvent" : "_emitResponseEvent";
-  v8::HandleScope handle_scope(isolate());
-  gin_helper::CustomEmit(isolate(), GetWrapper(), method,
-                         std::forward<Args>(args)...);
-}
-
-// static
-mate::WrappableBase* URLRequest::New(gin::Arguments* args) {
-  return new URLRequest(args);
-}
-
-// static
-void URLRequest::BuildPrototype(v8::Isolate* isolate,
-                                v8::Local<v8::FunctionTemplate> prototype) {
-  prototype->SetClassName(gin::StringToV8(isolate, "URLRequest"));
-  gin_helper::Destroyable::MakeDestroyable(isolate, prototype);
-  gin_helper::ObjectTemplateBuilder(isolate, prototype->PrototypeTemplate())
-      .SetMethod("write", &URLRequest::Write)
-      .SetMethod("cancel", &URLRequest::Cancel)
-      .SetMethod("setExtraHeader", &URLRequest::SetExtraHeader)
-      .SetMethod("removeExtraHeader", &URLRequest::RemoveExtraHeader)
-      .SetMethod("setChunkedUpload", &URLRequest::SetChunkedUpload)
-      .SetMethod("followRedirect", &URLRequest::FollowRedirect)
-      .SetMethod("getUploadProgress", &URLRequest::GetUploadProgress)
-      .SetProperty("notStarted", &URLRequest::NotStarted)
-      .SetProperty("finished", &URLRequest::Finished)
-      .SetProperty("statusCode", &URLRequest::StatusCode)
-      .SetProperty("statusMessage", &URLRequest::StatusMessage)
-      .SetProperty("rawResponseHeaders", &URLRequest::RawResponseHeaders)
-      .SetProperty("httpVersionMajor", &URLRequest::ResponseHttpVersionMajor)
-      .SetProperty("httpVersionMinor", &URLRequest::ResponseHttpVersionMinor);
-}
-
-}  // namespace api
-
-}  // namespace electron

shell/browser/api/atom_api_url_request.h

@@ -1,156 +0,0 @@
-// 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 SHELL_BROWSER_API_ATOM_API_URL_REQUEST_H_
-#define SHELL_BROWSER_API_ATOM_API_URL_REQUEST_H_
-
-#include <list>
-#include <memory>
-#include <string>
-#include <vector>
-
-#include "gin/arguments.h"
-#include "gin/dictionary.h"
-#include "mojo/public/cpp/system/data_pipe_producer.h"
-#include "services/network/public/cpp/shared_url_loader_factory.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/data_pipe_getter.mojom.h"
-#include "services/network/public/mojom/network_context.mojom.h"
-#include "shell/common/gin_helper/event_emitter.h"
-
-namespace electron {
-
-namespace api {
-
-class UploadDataPipeGetter;
-
-class URLRequest : public gin_helper::EventEmitter<URLRequest>,
-                   public network::SimpleURLLoaderStreamConsumer {
- public:
-  static mate::WrappableBase* New(gin::Arguments* args);
-
-  static void BuildPrototype(v8::Isolate* isolate,
-                             v8::Local<v8::FunctionTemplate> prototype);
-  static URLRequest* FromID(uint32_t id);
-
-  void OnAuthRequired(
-      const GURL& url,
-      bool first_auth_attempt,
-      net::AuthChallengeInfo auth_info,
-      network::mojom::URLResponseHeadPtr head,
-      mojo::PendingRemote<network::mojom::AuthChallengeResponder>
-          auth_challenge_responder);
-
- protected:
-  explicit URLRequest(gin::Arguments* args);
-  ~URLRequest() override;
-
-  bool NotStarted() const;
-  bool Finished() const;
-
-  void Cancel();
-  void Close();
-
-  bool Write(v8::Local<v8::Value> data, bool is_last);
-  void FollowRedirect();
-  bool SetExtraHeader(const std::string& name, const std::string& value);
-  void RemoveExtraHeader(const std::string& name);
-  void SetChunkedUpload(bool is_chunked_upload);
-  gin::Dictionary GetUploadProgress();
-  int StatusCode() const;
-  std::string StatusMessage() const;
-  net::HttpResponseHeaders* RawResponseHeaders() const;
-  uint32_t ResponseHttpVersionMajor() const;
-  uint32_t ResponseHttpVersionMinor() const;
-
-  // SimpleURLLoaderStreamConsumer:
-  void OnDataReceived(base::StringPiece string_piece,
-                      base::OnceClosure resume) override;
-  void OnComplete(bool success) override;
-  void OnRetry(base::OnceClosure start_retry) override;
-
- private:
-  friend class UploadDataPipeGetter;
-
-  void OnResponseStarted(const GURL& final_url,
-                         const network::mojom::URLResponseHead& response_head);
-  void OnRedirect(const net::RedirectInfo& redirect_info,
-                  const network::mojom::URLResponseHead& response_head,
-                  std::vector<std::string>* to_be_removed_headers);
-  void OnUploadProgress(uint64_t position, uint64_t total);
-  void OnWrite(MojoResult result);
-
-  // Write the first data of |pending_writes_|.
-  void DoWrite();
-
-  // Start streaming.
-  void StartWriting();
-
-  // Manage lifetime of wrapper.
-  void Pin();
-  void Unpin();
-
-  // Emit events.
-  enum class EventType {
-    kRequest,
-    kResponse,
-  };
-  void EmitError(EventType type, base::StringPiece error);
-  template <typename... Args>
-  void EmitEvent(EventType type, Args&&... args);
-
-  std::unique_ptr<network::ResourceRequest> request_;
-  std::unique_ptr<network::SimpleURLLoader> loader_;
-  scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
-  scoped_refptr<net::HttpResponseHeaders> response_headers_;
-
-  // Redirect mode.
-  //
-  // Note that we store it ourselves instead of reading from the one stored in
-  // |request_|, this is because with multiple redirections, the original one
-  // might be modified.
-  network::mojom::RedirectMode redirect_mode_ =
-      network::mojom::RedirectMode::kFollow;
-
-  // The DataPipeGetter passed to reader.
-  bool is_chunked_upload_ = false;
-  std::unique_ptr<UploadDataPipeGetter> data_pipe_getter_;
-
-  // Passed from DataPipeGetter for streaming data.
-  network::mojom::DataPipeGetter::ReadCallback size_callback_;
-  std::unique_ptr<mojo::DataPipeProducer> producer_;
-
-  // Whether request.end() has been called.
-  bool last_chunk_written_ = false;
-
-  // Whether the redirect should be followed.
-  bool follow_redirect_ = true;
-
-  // Upload progress.
-  uint64_t upload_position_ = 0;
-  uint64_t upload_total_ = 0;
-
-  // Current status.
-  int request_state_ = 0;
-  int response_state_ = 0;
-
-  // Pending writes that not yet sent to NetworkService.
-  std::list<std::string> pending_writes_;
-
-  // Used by pin/unpin to manage lifetime.
-  v8::Global<v8::Object> wrapper_;
-
-  uint32_t id_;
-
-  base::WeakPtrFactory<URLRequest> weak_factory_;
-
-  DISALLOW_COPY_AND_ASSIGN(URLRequest);
-};
-
-}  // namespace api
-
-}  // namespace electron
-
-#endif  // SHELL_BROWSER_API_ATOM_API_URL_REQUEST_H_

shell/browser/atom_browser_context.cc

@@ -33,7 +33,7 @@
 #include "services/network/public/cpp/features.h"
 #include "services/network/public/cpp/wrapper_shared_url_loader_factory.h"
 #include "services/network/public/mojom/network_context.mojom.h"
-#include "shell/browser/api/atom_api_url_request.h"
+#include "shell/browser/api/atom_api_url_loader.h"
 #include "shell/browser/atom_browser_client.h"
 #include "shell/browser/atom_browser_main_parts.h"
 #include "shell/browser/atom_download_manager_delegate.h"
@@ -362,11 +362,12 @@ class AuthResponder : public network::mojom::TrustedAuthClient {
       ::network::mojom::URLResponseHeadPtr head,
       mojo::PendingRemote<network::mojom::AuthChallengeResponder>
           auth_challenge_responder) override {
-    api::URLRequest* url_request = api::URLRequest::FromID(routing_id);
-    if (url_request) {
-      url_request->OnAuthRequired(url, first_auth_attempt, auth_info,
-                                  std::move(head),
-                                  std::move(auth_challenge_responder));
+    api::SimpleURLLoaderWrapper* url_loader =
+        api::SimpleURLLoaderWrapper::FromID(routing_id);
+    if (url_loader) {
+      url_loader->OnAuthRequired(url, first_auth_attempt, auth_info,
+                                 std::move(head),
+                                 std::move(auth_challenge_responder));
     }
   }
 };

shell/common/gin_converters/net_converter.cc

@@ -17,6 +17,8 @@
 #include "net/cert/x509_certificate.h"
 #include "net/cert/x509_util.h"
 #include "net/http/http_response_headers.h"
+#include "net/http/http_version.h"
+#include "net/url_request/redirect_info.h"
 #include "services/network/public/cpp/resource_request.h"
 #include "shell/browser/api/atom_api_data_pipe_holder.h"
 #include "shell/common/gin_converters/gurl_converter.h"
@@ -337,7 +339,7 @@ bool Converter<scoped_refptr<network::ResourceRequestBody>>::FromV8(
 v8::Local<v8::Value> Converter<network::ResourceRequest>::ToV8(
     v8::Isolate* isolate,
     const network::ResourceRequest& val) {
-  gin::Dictionary dict(isolate, v8::Object::New(isolate));
+  gin::Dictionary dict = gin::Dictionary::CreateEmpty(isolate);
   dict.Set("method", val.method);
   dict.Set("url", val.url.spec());
   dict.Set("referrer", val.referrer.spec());
@@ -359,4 +361,32 @@ v8::Local<v8::Value> Converter<electron::VerifyRequestParams>::ToV8(
   return ConvertToV8(isolate, dict);
 }
 
+// static
+v8::Local<v8::Value> Converter<net::HttpVersion>::ToV8(
+    v8::Isolate* isolate,
+    const net::HttpVersion& val) {
+  gin::Dictionary dict = gin::Dictionary::CreateEmpty(isolate);
+  dict.Set("major", static_cast<uint32_t>(val.major_value()));
+  dict.Set("minor", static_cast<uint32_t>(val.minor_value()));
+  return ConvertToV8(isolate, dict);
+}
+
+// static
+v8::Local<v8::Value> Converter<net::RedirectInfo>::ToV8(
+    v8::Isolate* isolate,
+    const net::RedirectInfo& val) {
+  gin::Dictionary dict = gin::Dictionary::CreateEmpty(isolate);
+
+  dict.Set("statusCode", val.status_code);
+  dict.Set("newMethod", val.new_method);
+  dict.Set("newUrl", val.new_url);
+  dict.Set("newSiteForCookies", val.new_site_for_cookies);
+  dict.Set("newReferrer", val.new_referrer);
+  dict.Set("insecureSchemeWasUpgraded", val.insecure_scheme_was_upgraded);
+  dict.Set("isSignedExchangeFallbackRedirect",
+           val.is_signed_exchange_fallback_redirect);
+
+  return ConvertToV8(isolate, dict);
+}
+
 }  // namespace gin

shell/common/gin_converters/net_converter.h

@@ -5,7 +5,10 @@
 #ifndef SHELL_COMMON_GIN_CONVERTERS_NET_CONVERTER_H_
 #define SHELL_COMMON_GIN_CONVERTERS_NET_CONVERTER_H_
 
+#include <string>
+
 #include "gin/converter.h"
+#include "services/network/public/mojom/fetch_api.mojom.h"
 #include "shell/browser/net/cert_verifier_client.h"
 
 namespace base {
@@ -19,6 +22,7 @@ class URLRequest;
 class X509Certificate;
 class HttpResponseHeaders;
 struct CertPrincipal;
+class HttpVersion;
 }  // namespace net
 
 namespace network {
@@ -96,6 +100,18 @@ struct Converter<electron::VerifyRequestParams> {
                                    electron::VerifyRequestParams val);
 };
 
+template <>
+struct Converter<net::HttpVersion> {
+  static v8::Local<v8::Value> ToV8(v8::Isolate* isolate,
+                                   const net::HttpVersion& val);
+};
+
+template <>
+struct Converter<net::RedirectInfo> {
+  static v8::Local<v8::Value> ToV8(v8::Isolate* isolate,
+                                   const net::RedirectInfo& val);
+};
+
 }  // namespace gin
 
 #endif  // SHELL_COMMON_GIN_CONVERTERS_NET_CONVERTER_H_

shell/renderer/electron_api_service_impl.cc

@@ -67,7 +67,7 @@ void InvokeIpcCallback(v8::Local<v8::Context> context,
                           .ToLocalChecked();
   auto callback_value = ipcNative->Get(context, callback_key).ToLocalChecked();
   DCHECK(callback_value->IsFunction());  // set by init.ts
-  auto callback = v8::Local<v8::Function>::Cast(callback_value);
+  auto callback = callback_value.As<v8::Function>();
   ignore_result(callback->Call(context, ipcNative, args.size(), args.data()));
 }
 

spec-main/api-net-spec.ts

@@ -2,7 +2,7 @@ import { expect } from 'chai'
 import { net, session, ClientRequest, BrowserWindow } from 'electron'
 import * as http from 'http'
 import * as url from 'url'
-import { AddressInfo } from 'net'
+import { AddressInfo, Socket } from 'net'
 import { emittedOnce } from './events-helpers'
 
 const kOneKiloByte = 1024
@@ -22,6 +22,13 @@ function randomString (length: number) {
   return buffer.toString()
 }
 
+const cleanupTasks: (() => void)[] = []
+
+function cleanUp () {
+  cleanupTasks.forEach(t => t())
+  cleanupTasks.length = 0
+}
+
 function respondOnce (fn: http.RequestListener): Promise<string> {
   return new Promise((resolve) => {
     const server = http.createServer((request, response) => {
@@ -32,6 +39,12 @@ function respondOnce (fn: http.RequestListener): Promise<string> {
     server.listen(0, '127.0.0.1', () => {
       resolve(`http://127.0.0.1:${(server.address() as AddressInfo).port}`)
     })
+    const sockets: Socket[] = []
+    server.on('connection', s => sockets.push(s))
+    cleanupTasks.push(() => {
+      server.close()
+      sockets.forEach(s => s.destroy())
+    })
   })
 }
 
@@ -57,6 +70,10 @@ respondOnce.toSingleURL = (fn: http.RequestListener) => {
 }
 
 describe('net module', () => {
+  afterEach(cleanUp)
+  afterEach(async () => {
+    await session.defaultSession.clearCache()
+  })
   describe('HTTP basics', () => {
     it('should be able to issue a basic GET request', (done) => {
       respondOnce.toSingleURL((request, response) => {
@@ -325,6 +342,36 @@ describe('net module', () => {
         request.end()
       })
     })
+
+    it('should upload body when 401', async () => {
+      const [user, pass] = ['user', 'pass']
+      const serverUrl = await respondOnce.toSingleURL((request, response) => {
+        if (!request.headers.authorization) {
+          return response.writeHead(401, { 'WWW-Authenticate': 'Basic realm="Foo"' }).end()
+        }
+        response.writeHead(200)
+        request.on('data', (chunk) => { response.write(chunk) })
+        request.on('end', () => {
+          response.end()
+        })
+      })
+      const requestData = randomString(kOneKiloByte)
+      const responseData = await new Promise((resolve, reject) => {
+        const request = net.request({ method: 'GET', url: serverUrl })
+        request.on('response', (response) => {
+          response.on('error', reject)
+          let data = ''
+          response.on('data', (chunk) => { data += chunk.toString() })
+          response.on('end', () => resolve(data))
+        })
+        request.on('login', (authInfo, cb) => {
+          cb(user, pass)
+        })
+        request.on('error', reject)
+        request.end(requestData)
+      })
+      expect(responseData).to.equal(requestData)
+    })
   })
 
   describe('ClientRequest API', () => {
@@ -955,10 +1002,10 @@ describe('net module', () => {
     })
 
     it('should follow redirect when no redirect handler is provided', (done) => {
-      const requestUrl = '/301'
+      const requestUrl = '/302'
       respondOnce.toRoutes({
-        '/301': (request, response) => {
-          response.statusCode = 301
+        '/302': (request, response) => {
+          response.statusCode = 302
           response.setHeader('Location', '/200')
           response.end()
         },
@@ -981,12 +1028,12 @@ describe('net module', () => {
     it('should follow redirect chain when no redirect handler is provided', (done) => {
       respondOnce.toRoutes({
         '/redirectChain': (request, response) => {
-          response.statusCode = 301
-          response.setHeader('Location', '/301')
+          response.statusCode = 302
+          response.setHeader('Location', '/302')
           response.end()
         },
-        '/301': (request, response) => {
-          response.statusCode = 301
+        '/302': (request, response) => {
+          response.statusCode = 302
           response.setHeader('Location', '/200')
           response.end()
         },
@@ -1008,7 +1055,7 @@ describe('net module', () => {
 
     it('should not follow redirect when request is canceled in redirect handler', async () => {
       const serverUrl = await respondOnce.toSingleURL((request, response) => {
-        response.statusCode = 301
+        response.statusCode = 302
         response.setHeader('Location', '/200')
         response.end()
       })
@@ -1017,18 +1064,33 @@ describe('net module', () => {
       })
       urlRequest.end()
       urlRequest.on('redirect', () => { urlRequest.abort() })
+      urlRequest.on('error', () => {})
       await emittedOnce(urlRequest, 'abort')
     })
 
+    it('should not follow redirect when mode is error', async () => {
+      const serverUrl = await respondOnce.toSingleURL((request, response) => {
+        response.statusCode = 302
+        response.setHeader('Location', '/200')
+        response.end()
+      })
+      const urlRequest = net.request({
+        url: serverUrl,
+        redirect: 'error'
+      })
+      urlRequest.end()
+      await emittedOnce(urlRequest, 'error')
+    })
+
     it('should follow redirect when handler calls callback', async () => {
       const serverUrl = await respondOnce.toRoutes({
         '/redirectChain': (request, response) => {
-          response.statusCode = 301
-          response.setHeader('Location', '/301')
+          response.statusCode = 302
+          response.setHeader('Location', '/302')
           response.end()
         },
-        '/301': (request, response) => {
-          response.statusCode = 301
+        '/302': (request, response) => {
+          response.statusCode = 302
           response.setHeader('Location', '/200')
           response.end()
         },
@@ -1037,7 +1099,7 @@ describe('net module', () => {
           response.end()
         }
       })
-      const urlRequest = net.request(`${serverUrl}/redirectChain`)
+      const urlRequest = net.request({ url: `${serverUrl}/redirectChain`, redirect: 'manual' })
       const redirects: string[] = []
       urlRequest.on('redirect', (status, method, url) => {
         redirects.push(url)
@@ -1047,7 +1109,7 @@ describe('net module', () => {
       const [response] = await emittedOnce(urlRequest, 'response')
       expect(response.statusCode).to.equal(200)
       expect(redirects).to.deep.equal([
-        `${serverUrl}/301`,
+        `${serverUrl}/302`,
         `${serverUrl}/200`
       ])
     })
@@ -1276,9 +1338,9 @@ describe('net module', () => {
       await emittedOnce(response, 'end')
     })
 
-    it('should be able to pipe a net response into a writable stream', (done) => {
+    it('should be able to pipe a net response into a writable stream', async () => {
       const bodyData = randomString(kOneKiloByte)
-      Promise.all([
+      const [netServerUrl, nodeServerUrl] = await Promise.all([
         respondOnce.toSingleURL((request, response) => response.end(bodyData)),
         respondOnce.toSingleURL((request, response) => {
           let receivedBodyData = ''
@@ -1293,8 +1355,10 @@ describe('net module', () => {
             response.end()
           })
         })
-      ]).then(([netServerUrl, nodeServerUrl]) => {
-        const netRequest = net.request(netServerUrl)
+      ])
+      const netRequest = net.request(netServerUrl)
+      netRequest.end()
+      await new Promise((resolve) => {
         netRequest.on('response', (netResponse) => {
           const serverUrl = url.parse(nodeServerUrl)
           const nodeOptions = {
@@ -1305,7 +1369,7 @@ describe('net module', () => {
           const nodeRequest = http.request(nodeOptions, res => {
             res.on('data', () => {})
             res.on('end', () => {
-              done()
+              resolve()
             })
           });
           // TODO: IncomingMessage should properly extend ReadableStream in the
@@ -1356,6 +1420,31 @@ describe('net module', () => {
     it('should collect unreferenced, ended requests without crash', (done) => {
       respondOnce.toSingleURL((request, response) => {
         response.end()
+      }).then(serverUrl => {
+        const urlRequest = net.request(serverUrl)
+        urlRequest.on('response', (response) => {
+          response.on('data', () => {})
+          response.on('end', () => { done() })
+        })
+        process.nextTick(() => {
+          const v8Util = process.electronBinding('v8_util')
+          v8Util.requestGarbageCollectionForTesting()
+        })
+        urlRequest.end()
+      })
+    })
+
+    it('should finish sending data when urlRequest is unreferenced', (done) => {
+      respondOnce.toSingleURL((request, response) => {
+        let received = Buffer.alloc(0)
+        request.on('data', (data) => {
+          received = Buffer.concat([received, data])
+        })
+        request.on('end', () => {
+          response.end()
+          expect(received.length).to.equal(kOneMegaByte)
+          done()
+        })
       }).then(serverUrl => {
         const urlRequest = net.request(serverUrl)
         urlRequest.on('response', (response) => {
@@ -1366,10 +1455,61 @@ describe('net module', () => {
           process.nextTick(() => {
             const v8Util = process.electronBinding('v8_util')
             v8Util.requestGarbageCollectionForTesting()
-            done()
           })
         })
-        urlRequest.end()
+        urlRequest.end(randomBuffer(kOneMegaByte))
+      })
+    })
+
+    it('should finish sending data when urlRequest is unreferenced for chunked encoding', (done) => {
+      respondOnce.toSingleURL((request, response) => {
+        let received = Buffer.alloc(0)
+        request.on('data', (data) => {
+          received = Buffer.concat([received, data])
+        })
+        request.on('end', () => {
+          response.end()
+          expect(received.length).to.equal(kOneMegaByte)
+          done()
+        })
+      }).then(serverUrl => {
+        const urlRequest = net.request(serverUrl)
+        urlRequest.on('response', (response) => {
+          response.on('data', () => {})
+          response.on('end', () => {})
+        })
+        urlRequest.on('close', () => {
+          process.nextTick(() => {
+            const v8Util = process.electronBinding('v8_util')
+            v8Util.requestGarbageCollectionForTesting()
+          })
+        })
+        urlRequest.chunkedEncoding = true
+        urlRequest.end(randomBuffer(kOneMegaByte))
+      })
+    })
+
+    it('should finish sending data when urlRequest is unreferenced before close event for chunked encoding', (done) => {
+      respondOnce.toSingleURL((request, response) => {
+        let received = Buffer.alloc(0)
+        request.on('data', (data) => {
+          received = Buffer.concat([received, data])
+        })
+        request.on('end', () => {
+          response.end()
+          expect(received.length).to.equal(kOneMegaByte)
+          done()
+        })
+      }).then(serverUrl => {
+        const urlRequest = net.request(serverUrl)
+        urlRequest.on('response', (response) => {
+          response.on('data', () => {})
+          response.on('end', () => {})
+        })
+        urlRequest.chunkedEncoding = true
+        urlRequest.end(randomBuffer(kOneMegaByte))
+        const v8Util = process.electronBinding('v8_util')
+        v8Util.requestGarbageCollectionForTesting()
       })
     })