feat: promisify session proxy methods (#17222)

atom/browser/api/atom_api_session.cc

@@ -214,6 +214,7 @@ std::map<uint32_t, v8::Global<v8::Object>> g_sessions;
 void RunCallbackInUI(const base::Callback<void()>& callback) {
   base::PostTaskWithTraits(FROM_HERE, {BrowserThread::UI}, callback);
 }
+
 template <typename... T>
 void RunCallbackInUI(const base::Callback<void(T...)>& callback, T... result) {
   base::PostTaskWithTraits(FROM_HERE, {BrowserThread::UI},
@@ -410,10 +411,20 @@ void Session::OnDownloadCreated(content::DownloadManager* manager,
   }
 }
 
-void Session::ResolveProxy(
+v8::Local<v8::Promise> Session::ResolveProxy(mate::Arguments* args) {
-    const GURL& url,
+  v8::Isolate* isolate = args->isolate();
-    const ResolveProxyHelper::ResolveProxyCallback& callback) {
+  util::Promise promise(isolate);
-  browser_context_->GetResolveProxyHelper()->ResolveProxy(url, callback);
+  v8::Local<v8::Promise> handle = promise.GetHandle();
+
+  GURL url;
+  args->GetNext(&url);
+
+  browser_context_->GetResolveProxyHelper()->ResolveProxy(
+      url,
+      base::Bind(util::CopyablePromise::ResolveCopyablePromise<std::string>,
+                 atom::util::CopyablePromise(promise)));
+
+  return handle;
 }
 
 template <Session::CacheAction action>
@@ -455,11 +466,17 @@ void Session::FlushStorageData() {
   storage_partition->Flush();
 }
 
-void Session::SetProxy(const mate::Dictionary& options,
+v8::Local<v8::Promise> Session::SetProxy(mate::Arguments* args) {
-                       const base::Closure& callback) {
+  v8::Isolate* isolate = args->isolate();
+  util::Promise promise(isolate);
+  v8::Local<v8::Promise> handle = promise.GetHandle();
+
+  mate::Dictionary options;
+  args->GetNext(&options);
+
   if (!browser_context_->in_memory_pref_store()) {
-    callback.Run();
+    promise.Resolve();
-    return;
+    return handle;
   }
 
   std::string proxy_rules, bypass_list, pac_url;
@@ -483,7 +500,11 @@ void Session::SetProxy(const mate::Dictionary& options,
         WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS);
   }
 
-  base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, callback);
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE,
+      base::BindOnce(util::Promise::ResolveEmptyPromise, std::move(promise)));
+
+  return handle;
 }
 
 void Session::SetDownloadPath(const base::FilePath& path) {

atom/browser/api/atom_api_session.h

@@ -62,13 +62,12 @@ class Session : public mate::TrackableObject<Session>,
                              v8::Local<v8::FunctionTemplate> prototype);
 
   // Methods.
-  void ResolveProxy(const GURL& url,
+  v8::Local<v8::Promise> ResolveProxy(mate::Arguments* args);
-                    const ResolveProxyHelper::ResolveProxyCallback& callback);
   template <CacheAction action>
   void DoCacheAction(const net::CompletionCallback& callback);
   v8::Local<v8::Promise> ClearStorageData(mate::Arguments* args);
   void FlushStorageData();
-  void SetProxy(const mate::Dictionary& options, const base::Closure& callback);
+  v8::Local<v8::Promise> SetProxy(mate::Arguments* args);
   void SetDownloadPath(const base::FilePath& path);
   void EnableNetworkEmulation(const mate::Dictionary& options);
   void DisableNetworkEmulation();

atom/common/promise_util.h

@@ -172,7 +172,7 @@ class CopyablePromise {
     if (!content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)) {
       base::PostTaskWithTraits(
           FROM_HERE, {content::BrowserThread::UI},
-          base::BindOnce(Promise::ResolvePromise, promise.GetPromise(),
+          base::BindOnce(Promise::ResolvePromise<T>, promise.GetPromise(),
                          std::move(result)));
     } else {
       promise.GetPromise().Resolve(result);

docs/api/promisification.md

@@ -15,8 +15,6 @@ When a majority of affected functions are migrated, this flag will be enabled by
 - [inAppPurchase.getProducts(productIDs, callback)](https://github.com/electron/electron/blob/master/docs/api/in-app-purchase.md#getProducts)
 - [ses.getCacheSize(callback)](https://github.com/electron/electron/blob/master/docs/api/session.md#getCacheSize)
 - [ses.clearCache(callback)](https://github.com/electron/electron/blob/master/docs/api/session.md#clearCache)
-- [ses.setProxy(config, callback)](https://github.com/electron/electron/blob/master/docs/api/session.md#setProxy)
-- [ses.resolveProxy(url, callback)](https://github.com/electron/electron/blob/master/docs/api/session.md#resolveProxy)
 - [ses.getBlobData(identifier, callback)](https://github.com/electron/electron/blob/master/docs/api/session.md#getBlobData)
 - [ses.clearAuthCache(options[, callback])](https://github.com/electron/electron/blob/master/docs/api/session.md#clearAuthCache)
 - [contents.executeJavaScript(code[, userGesture, callback])](https://github.com/electron/electron/blob/master/docs/api/web-contents.md#executeJavaScript)
@@ -47,6 +45,8 @@ When a majority of affected functions are migrated, this flag will be enabled by
 - [protocol.isProtocolHandled(scheme, callback)](https://github.com/electron/electron/blob/master/docs/api/protocol.md#isProtocolHandled)
 - [ses.clearHostResolverCache([callback])](https://github.com/electron/electron/blob/master/docs/api/session.md#clearHostResolverCache)
 - [ses.clearStorageData([options, callback])](https://github.com/electron/electron/blob/master/docs/api/session.md#clearStorageData)
+- [ses.setProxy(config, callback)](https://github.com/electron/electron/blob/master/docs/api/session.md#setProxy)
+- [ses.resolveProxy(url, callback)](https://github.com/electron/electron/blob/master/docs/api/session.md#resolveProxy)
 - [shell.openExternal(url[, options, callback])](https://github.com/electron/electron/blob/master/docs/api/shell.md#openExternal)
 - [webviewTag.capturePage([rect, ]callback)](https://github.com/electron/electron/blob/master/docs/api/webview-tag.md#capturePage)
 - [webviewTag.printToPDF(options, callback)](https://github.com/electron/electron/blob/master/docs/api/webview-tag.md#printToPDF)

docs/api/session.md

@@ -180,6 +180,85 @@ For example:
 
 The `proxyBypassRules` is a comma separated list of rules described below:
 
+* `[ URL_SCHEME "://" ] HOSTNAME_PATTERN [ ":" <port> ]`
+
+   Match all hostnames that match the pattern HOSTNAME_PATTERN.
+
+   Examples:
+     "foobar.com", "*foobar.com", "*.foobar.com", "*foobar.com:99",
+     "https://x.*.y.com:99"
+
+ * `"." HOSTNAME_SUFFIX_PATTERN [ ":" PORT ]`
+
+   Match a particular domain suffix.
+
+   Examples:
+     ".google.com", ".com", "http://.google.com"
+
+* `[ SCHEME "://" ] IP_LITERAL [ ":" PORT ]`
+
+   Match URLs which are IP address literals.
+
+   Examples:
+     "127.0.1", "[0:0::1]", "[::1]", "http://[::1]:99"
+
+* `IP_LITERAL "/" PREFIX_LENGTH_IN_BITS`
+
+   Match any URL that is to an IP literal that falls between the
+   given range. IP range is specified using CIDR notation.
+
+   Examples:
+     "192.168.1.1/16", "fefe:13::abc/33".
+
+* `<local>`
+
+   Match local addresses. The meaning of `<local>` is whether the
+   host matches one of: "127.0.0.1", "::1", "localhost".
+
+**[Deprecated Soon](promisification.md)**
+
+#### `ses.setProxy(config)`
+
+* `config` Object
+  * `pacScript` String - The URL associated with the PAC file.
+  * `proxyRules` String - Rules indicating which proxies to use.
+  * `proxyBypassRules` String - Rules indicating which URLs should
+    bypass the proxy settings.
+
+Returns `Promise<void>` - Resolves when the proxy setting process is complete.
+
+Sets the proxy settings.
+
+When `pacScript` and `proxyRules` are provided together, the `proxyRules`
+option is ignored and `pacScript` configuration is applied.
+
+The `proxyRules` has to follow the rules below:
+
+```sh
+proxyRules = schemeProxies[";"<schemeProxies>]
+schemeProxies = [<urlScheme>"="]<proxyURIList>
+urlScheme = "http" | "https" | "ftp" | "socks"
+proxyURIList = <proxyURL>[","<proxyURIList>]
+proxyURL = [<proxyScheme>"://"]<proxyHost>[":"<proxyPort>]
+```
+
+For example:
+
+* `http=foopy:80;ftp=foopy2` - Use HTTP proxy `foopy:80` for `http://` URLs, and
+  HTTP proxy `foopy2:80` for `ftp://` URLs.
+* `foopy:80` - Use HTTP proxy `foopy:80` for all URLs.
+* `foopy:80,bar,direct://` - Use HTTP proxy `foopy:80` for all URLs, failing
+  over to `bar` if `foopy:80` is unavailable, and after that using no proxy.
+* `socks4://foopy` - Use SOCKS v4 proxy `foopy:1080` for all URLs.
+* `http=foopy,socks5://bar.com` - Use HTTP proxy `foopy` for http URLs, and fail
+  over to the SOCKS5 proxy `bar.com` if `foopy` is unavailable.
+* `http=foopy,direct://` - Use HTTP proxy `foopy` for http URLs, and use no
+  proxy if `foopy` is unavailable.
+* `http=foopy;socks=foopy2` - Use HTTP proxy `foopy` for http URLs, and use
+  `socks4://foopy2` for all other URLs.
+
+The `proxyBypassRules` is a comma separated list of rules described below:
+
 * `[ URL_SCHEME "://" ] HOSTNAME_PATTERN [ ":" <port> ]`
 
    Match all hostnames that match the pattern HOSTNAME_PATTERN.
@@ -224,6 +303,14 @@ The `proxyBypassRules` is a comma separated list of rules described below:
 Resolves the proxy information for `url`. The `callback` will be called with
 `callback(proxy)` when the request is performed.
 
+**[Deprecated Soon](promisification.md)**
+
+#### `ses.resolveProxy(url)`
+
+* `url` URL
+
+Returns `Promise<string>` - Resolves with the proxy information for `url`.
+
 #### `ses.setDownloadPath(path)`
 
 * `path` String - The download location.

lib/browser/api/session.js

@@ -25,6 +25,8 @@ Session.prototype._init = function () {
 
 Session.prototype.clearStorageData = deprecate.promisify(Session.prototype.clearStorageData)
 Session.prototype.clearHostResolverCache = deprecate.promisify(Session.prototype.clearHostResolverCache)
+Session.prototype.resolveProxy = deprecate.promisify(Session.prototype.resolveProxy)
+Session.prototype.setProxy = deprecate.promisify(Session.prototype.setProxy)
 
 Cookies.prototype.flushStore = deprecate.promisify(Cookies.prototype.flushStore)
 Cookies.prototype.get = deprecate.promisify(Cookies.prototype.get)

spec/api-session-spec.js

@@ -633,7 +633,7 @@ describe('session module', () => {
     })
   })
 
-  describe('ses.setProxy(options, callback)', () => {
+  describe('ses.setProxy(options)', () => {
     let server = null
     let customSession = null
 
@@ -655,30 +655,78 @@ describe('session module', () => {
       }
     })
 
-    it('allows configuring proxy settings', (done) => {
+    it('allows configuring proxy settings', async () => {
+      const config = { proxyRules: 'http=myproxy:80' }
+      await customSession.setProxy(config)
+      const proxy = await customSession.resolveProxy('http://example.com/')
+      assert.strictEqual(proxy, 'PROXY myproxy:80')
+    })
+
+    // TODO(codebytere): remove when Promisification is complete
+    it('allows configuring proxy settings (callback)', (done) => {
       const config = { proxyRules: 'http=myproxy:80' }
       customSession.setProxy(config, () => {
-        customSession.resolveProxy('http://example.com/', (proxy) => {
+        customSession.resolveProxy('http://example.com/', proxy => {
           assert.strictEqual(proxy, 'PROXY myproxy:80')
           done()
         })
       })
     })
 
-    it('allows removing the implicit bypass rules for localhost', (done) => {
+    it('allows removing the implicit bypass rules for localhost', async () => {
       const config = {
         proxyRules: 'http=myproxy:80',
         proxyBypassRules: '<-loopback>'
       }
-      customSession.setProxy(config, () => {
+
-        customSession.resolveProxy('http://localhost', (proxy) => {
+      await customSession.setProxy(config)
+      const proxy = await customSession.resolveProxy('http://localhost')
+      assert.strictEqual(proxy, 'PROXY myproxy:80')
+    })
+
+    // TODO(codebytere): remove when Promisification is complete
+    it('allows removing the implicit bypass rules for localhost (callback)', (done) => {
+      const config = {
+        proxyRules: 'http=myproxy:80',
+        proxyBypassRules: '<-loopback>'
+      }
+      customSession.setProxy(config).then(() => {
+        customSession.resolveProxy('http://localhost').then(proxy => {
           assert.strictEqual(proxy, 'PROXY myproxy:80')
           done()
         })
       })
     })
 
-    it('allows configuring proxy settings with pacScript', (done) => {
+    it('allows configuring proxy settings with pacScript', async () => {
+      server = http.createServer((req, res) => {
+        const pac = `
+          function FindProxyForURL(url, host) {
+            return "PROXY myproxy:8132";
+          }
+        `
+        res.writeHead(200, {
+          'Content-Type': 'application/x-ns-proxy-autoconfig'
+        })
+        res.end(pac)
+      })
+      return new Promise((resolve, reject) => {
+        server.listen(0, '127.0.0.1', async () => {
+          try {
+            const config = { pacScript: `http://127.0.0.1:${server.address().port}` }
+            await customSession.setProxy(config)
+            const proxy = await customSession.resolveProxy('https://google.com')
+            assert.strictEqual(proxy, 'PROXY myproxy:8132')
+            resolve()
+          } catch (error) {
+            reject(error)
+          }
+        })
+      })
+    })
+
+    // TODO(codebytere): reconfigure when Promisification is complete
+    it('allows configuring proxy settings with pacScript (callback)', (done) => {
       server = http.createServer((req, res) => {
         const pac = `
           function FindProxyForURL(url, host) {
@@ -693,7 +741,7 @@ describe('session module', () => {
       server.listen(0, '127.0.0.1', () => {
         const config = { pacScript: `http://127.0.0.1:${server.address().port}` }
         customSession.setProxy(config, () => {
-          customSession.resolveProxy('https://google.com', (proxy) => {
+          customSession.resolveProxy('https://google.com', proxy => {
             assert.strictEqual(proxy, 'PROXY myproxy:8132')
             done()
           })
@@ -701,13 +749,24 @@ describe('session module', () => {
       })
     })
 
-    it('allows bypassing proxy settings', (done) => {
+    it('allows bypassing proxy settings', async () => {
+      const config = {
+        proxyRules: 'http=myproxy:80',
+        proxyBypassRules: '<local>'
+      }
+      await customSession.setProxy(config)
+      const proxy = await customSession.resolveProxy('http://example/')
+      assert.strictEqual(proxy, 'DIRECT')
+    })
+
+    // TODO(codebytere): remove when Promisification is complete
+    it('allows bypassing proxy settings (callback)', (done) => {
       const config = {
         proxyRules: 'http=myproxy:80',
         proxyBypassRules: '<local>'
       }
       customSession.setProxy(config, () => {
-        customSession.resolveProxy('http://example/', (proxy) => {
+        customSession.resolveProxy('http://example/', proxy => {
           assert.strictEqual(proxy, 'DIRECT')
           done()
         })