Fixing code review issues: adding some test cases for partition/session options.

atom/browser/api/atom_api_url_request.cc

@@ -173,9 +173,9 @@ URLRequest::~URLRequest() {
 
 // static
 mate::WrappableBase* URLRequest::New(mate::Arguments* args) {
+  auto isolate = args->isolate();
   v8::Local<v8::Object> options;
   args->GetNext(&options);
-  auto isolate = args->isolate();
   mate::Dictionary dict(isolate, options);
   std::string method;
   dict.Get("method", &method);

atom/browser/api/atom_api_url_request.h

@@ -86,9 +86,10 @@ namespace api {
 // more complex.
 //
 // We chose to split the implementation into two classes linked via a
-// strong/weak pointers. A URLRequest instance is deleted if it is unpinned and
+// reference counted/raw pointers. A URLRequest instance is deleted if it is
-// the corresponding JS wrapper object is garbage collected. On the other hand,
+// unpinned and the corresponding JS wrapper object is garbage collected. On the
-// an AtmURLRequest instance lifetime is totally governed by reference counting.
+// other hand, an AtmURLRequest instance lifetime is totally governed by
+// reference counting.
 //
 class URLRequest : public mate::EventEmitter<URLRequest> {
  public:
@@ -179,16 +180,6 @@ class URLRequest : public mate::EventEmitter<URLRequest> {
   uint32_t ResponseHttpVersionMajor() const;
   uint32_t ResponseHttpVersionMinor() const;
 
-  // template <typename... ArgTypes>
-  // std::array<v8::Local<v8::Value>, sizeof...(ArgTypes)> BuildArgsArray(
-  //    ArgTypes... args) const;
-
-  // template <typename... ArgTypes>
-  // void EmitRequestEvent(ArgTypes... args);
-
-  // template <typename... ArgTypes>
-  // void EmitResponseEvent(ArgTypes... args);
-
   void Close();
   void Pin();
   void Unpin();

lib/browser/api/net.js

@@ -5,7 +5,7 @@ const {EventEmitter} = require('events')
 const util = require('util')
 const {Readable} = require('stream')
 const {app} = require('electron')
-const {session} = require('electron')
+const {Session} = process.atomBinding('session')
 const {net, Net} = process.atomBinding('net')
 const {URLRequest} = net
 
@@ -161,7 +161,7 @@ class ClientRequest extends EventEmitter {
       url: urlStr
     }
     if (options.session) {
-      if (options.session instanceof session.Session) {
+      if (options.session instanceof Session) {
         urlRequestOptions.session = options.session
       } else {
         throw new TypeError('`session` should be an instance of the Session class.')

spec/api-net-spec.js

@@ -797,7 +797,79 @@ describe('net module', function () {
       urlRequest.end()
     })
 
-    it('should to able to create and intercept a request using a custom parition name', function (done) {
+    it('should to able to create and intercept a request using a custom session object', function (done) {
+      const requestUrl = '/requestUrl'
+      const redirectUrl = '/redirectUrl'
+      const customPartitionName = 'custom-partition'
+      let requestIsRedirected = false
+      server.on('request', function (request, response) {
+        switch (request.url) {
+          case requestUrl:
+            assert(false)
+            break
+          case redirectUrl:
+            requestIsRedirected = true
+            response.end()
+            break
+          default:
+            assert(false)
+        }
+      })
+
+      session.defaultSession.webRequest.onBeforeRequest(
+        function (details, callback) {
+          assert(false, 'Request should not be intercepted by the default session')
+        })
+
+      let customSession = session.fromPartition(customPartitionName, {
+        cache: false
+      })
+      let requestIsIntercepted = false
+      customSession.webRequest.onBeforeRequest(
+        function (details, callback) {
+          if (details.url === `${server.url}${requestUrl}`) {
+            requestIsIntercepted = true
+            callback({
+              redirectURL: `${server.url}${redirectUrl}`
+            })
+          } else {
+            callback({
+              cancel: false
+            })
+          }
+        })
+
+      const urlRequest = net.request({
+        url: `${server.url}${requestUrl}`,
+        session: customSession
+      })
+      urlRequest.on('response', function (response) {
+        assert.equal(response.statusCode, 200)
+        response.pause()
+        response.on('data', function (chunk) {
+        })
+        response.on('end', function () {
+          assert(requestIsRedirected, 'The server should receive a request to the forward URL')
+          assert(requestIsIntercepted, 'The request should be intercepted by the webRequest module')
+          done()
+        })
+        response.resume()
+      })
+      urlRequest.end()
+    })
+
+    it('should throw if given an invalid session option', function (done) {
+      try {
+        const urlRequest = net.request({
+          url: `${server.url}${requestUrl}`,
+          session: 1
+        })
+      } catch (exception) {
+        done()
+      }
+    })
+
+    it('should to able to create and intercept a request using a custom partition name', function (done) {
       const requestUrl = '/requestUrl'
       const redirectUrl = '/redirectUrl'
       const customPartitionName = 'custom-partition'
@@ -858,6 +930,17 @@ describe('net module', function () {
       urlRequest.end()
     })
 
+    it('should throw if given an invalid partition option', function (done) {
+      try {
+        const urlRequest = net.request({
+          url: `${server.url}${requestUrl}`,
+          partition: 1
+        })
+      } catch (exception) {
+        done()
+      }
+    })
+
     it('should be able to create a request with options', function (done) {
       const requestUrl = '/'
       const customHeaderName = 'Some-Custom-Header-Name'