feat: promisify cookies api (#16464)

* feat: promisify the Cookie API

* chore: update specs to test promisified cookies

* chore: add deprecate wrapper for cookie callback API

* docs: update docs to cookie promise changes

* chore: remove redundant namespace use

* docs: improve cookie example

* docs: restore docs for cookie callback API

* chore: restore cookie callback tests

* fix: syntax of cookie promise return types

atom/browser/api/atom_api_cookies.cc

@@ -136,6 +136,21 @@ inline net::CookieStore* GetCookieStore(
   return getter->GetURLRequestContext()->cookie_store();
 }
 
+void ResolvePromiseWithCookies(scoped_refptr<util::Promise> promise,
+                               net::CookieList cookieList) {
+  promise->Resolve(cookieList);
+}
+
+void ResolvePromise(scoped_refptr<util::Promise> promise) {
+  promise->Resolve();
+}
+
+// Resolve |promise| in UI thread.
+void ResolvePromiseInUI(scoped_refptr<util::Promise> promise) {
+  base::PostTaskWithTraits(FROM_HERE, {BrowserThread::UI},
+                           base::BindOnce(ResolvePromise, std::move(promise)));
+}
+
 // Run |callback| on UI thread.
 void RunCallbackInUI(const base::Closure& callback) {
   base::PostTaskWithTraits(FROM_HERE, {BrowserThread::UI}, callback);
@@ -143,25 +158,28 @@ void RunCallbackInUI(const base::Closure& callback) {
 
 // Remove cookies from |list| not matching |filter|, and pass it to |callback|.
 void FilterCookies(std::unique_ptr<base::DictionaryValue> filter,
-                   const Cookies::GetCallback& callback,
+                   scoped_refptr<util::Promise> promise,
                    const net::CookieList& list) {
   net::CookieList result;
   for (const auto& cookie : list) {
     if (MatchesCookie(filter.get(), cookie))
       result.push_back(cookie);
   }
-  RunCallbackInUI(base::Bind(callback, Cookies::SUCCESS, result));
+
+  base::PostTaskWithTraits(
+      FROM_HERE, {BrowserThread::UI},
+      base::BindOnce(ResolvePromiseWithCookies, std::move(promise), result));
 }
 
 // Receives cookies matching |filter| in IO thread.
 void GetCookiesOnIO(scoped_refptr<net::URLRequestContextGetter> getter,
                     std::unique_ptr<base::DictionaryValue> filter,
-                    const Cookies::GetCallback& callback) {
+                    scoped_refptr<util::Promise> promise) {
   std::string url;
   filter->GetString("url", &url);
 
   auto filtered_callback =
-      base::Bind(FilterCookies, base::Passed(&filter), callback);
+      base::Bind(FilterCookies, base::Passed(&filter), std::move(promise));
 
   // Empty url will match all url cookies.
   if (url.empty())
@@ -172,31 +190,42 @@ void GetCookiesOnIO(scoped_refptr<net::URLRequestContextGetter> getter,
 }
 
 // Removes cookie with |url| and |name| in IO thread.
-void RemoveCookieOnIOThread(scoped_refptr<net::URLRequestContextGetter> getter,
+void RemoveCookieOnIO(scoped_refptr<net::URLRequestContextGetter> getter,
-                            const GURL& url,
+                      const GURL& url,
-                            const std::string& name,
+                      const std::string& name,
-                            const base::Closure& callback) {
+                      scoped_refptr<util::Promise> promise) {
   GetCookieStore(getter)->DeleteCookieAsync(
-      url, name, base::BindOnce(RunCallbackInUI, callback));
+      url, name, base::BindOnce(ResolvePromiseInUI, promise));
+}
+
+// Resolves/rejects the |promise| in UI thread.
+void SettlePromiseInUI(scoped_refptr<util::Promise> promise,
+                       const std::string& errmsg) {
+  if (errmsg.empty()) {
+    promise->Resolve();
+  } else {
+    promise->RejectWithErrorMessage(errmsg);
+  }
 }
 
 // Callback of SetCookie.
-void OnSetCookie(const Cookies::SetCallback& callback, bool success) {
+void OnSetCookie(scoped_refptr<util::Promise> promise, bool success) {
-  RunCallbackInUI(
+  const std::string errmsg = success ? "" : "Setting cookie failed";
-      base::Bind(callback, success ? Cookies::SUCCESS : Cookies::FAILED));
+  RunCallbackInUI(base::Bind(SettlePromiseInUI, std::move(promise), errmsg));
 }
 
 // Flushes cookie store in IO thread.
 void FlushCookieStoreOnIOThread(
     scoped_refptr<net::URLRequestContextGetter> getter,
-    const base::Closure& callback) {
+    scoped_refptr<util::Promise> promise) {
-  GetCookieStore(getter)->FlushStore(base::BindOnce(RunCallbackInUI, callback));
+  GetCookieStore(getter)->FlushStore(
+      base::BindOnce(ResolvePromiseInUI, promise));
 }
 
 // Sets cookie with |details| in IO thread.
 void SetCookieOnIO(scoped_refptr<net::URLRequestContextGetter> getter,
                    std::unique_ptr<base::DictionaryValue> details,
-                   const Cookies::SetCallback& callback) {
+                   scoped_refptr<util::Promise> promise) {
   std::string url, name, value, domain, path;
   bool secure = false;
   bool http_only = false;
@@ -237,7 +266,7 @@ void SetCookieOnIO(scoped_refptr<net::URLRequestContextGetter> getter,
           GURL(url), name, value, domain, path, creation_time, expiration_time,
           last_access_time, secure, http_only,
           net::CookieSameSite::DEFAULT_MODE, net::COOKIE_PRIORITY_DEFAULT));
-  auto completion_callback = base::BindOnce(OnSetCookie, callback);
+  auto completion_callback = base::BindOnce(OnSetCookie, std::move(promise));
   if (!canonical_cookie || !canonical_cookie->IsCanonical()) {
     std::move(completion_callback).Run(false);
     return;
@@ -267,43 +296,56 @@ Cookies::Cookies(v8::Isolate* isolate, AtomBrowserContext* browser_context)
 
 Cookies::~Cookies() {}
 
-void Cookies::Get(const base::DictionaryValue& filter,
+v8::Local<v8::Promise> Cookies::Get(const base::DictionaryValue& filter) {
-                  const GetCallback& callback) {
+  scoped_refptr<util::Promise> promise = new util::Promise(isolate());
+
   auto copy = base::DictionaryValue::From(
       base::Value::ToUniquePtrValue(filter.Clone()));
   auto* getter = browser_context_->GetRequestContext();
   base::PostTaskWithTraits(
       FROM_HERE, {BrowserThread::IO},
       base::BindOnce(GetCookiesOnIO, base::RetainedRef(getter), std::move(copy),
-                     callback));
+                     promise));
+
+  return promise->GetHandle();
 }
 
-void Cookies::Remove(const GURL& url,
+v8::Local<v8::Promise> Cookies::Remove(const GURL& url,
-                     const std::string& name,
+                                       const std::string& name) {
-                     const base::Closure& callback) {
+  scoped_refptr<util::Promise> promise = new util::Promise(isolate());
+
   auto* getter = browser_context_->GetRequestContext();
   base::PostTaskWithTraits(
       FROM_HERE, {BrowserThread::IO},
-      base::BindOnce(RemoveCookieOnIOThread, base::RetainedRef(getter), url,
+      base::BindOnce(RemoveCookieOnIO, base::RetainedRef(getter), url, name,
-                     name, callback));
+                     promise));
+
+  return promise->GetHandle();
 }
 
-void Cookies::Set(const base::DictionaryValue& details,
+v8::Local<v8::Promise> Cookies::Set(const base::DictionaryValue& details) {
-                  const SetCallback& callback) {
+  scoped_refptr<util::Promise> promise = new util::Promise(isolate());
+
   auto copy = base::DictionaryValue::From(
       base::Value::ToUniquePtrValue(details.Clone()));
   auto* getter = browser_context_->GetRequestContext();
   base::PostTaskWithTraits(
       FROM_HERE, {BrowserThread::IO},
       base::BindOnce(SetCookieOnIO, base::RetainedRef(getter), std::move(copy),
-                     callback));
+                     promise));
+
+  return promise->GetHandle();
 }
 
-void Cookies::FlushStore(const base::Closure& callback) {
+v8::Local<v8::Promise> Cookies::FlushStore() {
+  scoped_refptr<util::Promise> promise = new util::Promise(isolate());
+
   auto* getter = browser_context_->GetRequestContext();
   base::PostTaskWithTraits(FROM_HERE, {BrowserThread::IO},
                            base::BindOnce(FlushCookieStoreOnIOThread,
-                                          base::RetainedRef(getter), callback));
+                                          base::RetainedRef(getter), promise));
+
+  return promise->GetHandle();
 }
 
 void Cookies::OnCookieChanged(const CookieDetails* details) {

atom/browser/api/atom_api_cookies.h

@@ -10,6 +10,7 @@
 
 #include "atom/browser/api/trackable_object.h"
 #include "atom/browser/net/cookie_details.h"
+#include "atom/common/promise_util.h"
 #include "base/callback_list.h"
 #include "native_mate/handle.h"
 #include "net/cookies/canonical_cookie.h"
@@ -35,9 +36,6 @@ class Cookies : public mate::TrackableObject<Cookies> {
     FAILED,
   };
 
-  using GetCallback = base::Callback<void(Error, const net::CookieList&)>;
-  using SetCallback = base::Callback<void(Error)>;
-
   static mate::Handle<Cookies> Create(v8::Isolate* isolate,
                                       AtomBrowserContext* browser_context);
 
@@ -49,12 +47,10 @@ class Cookies : public mate::TrackableObject<Cookies> {
   Cookies(v8::Isolate* isolate, AtomBrowserContext* browser_context);
   ~Cookies() override;
 
-  void Get(const base::DictionaryValue& filter, const GetCallback& callback);
+  v8::Local<v8::Promise> Get(const base::DictionaryValue& filter);
-  void Remove(const GURL& url,
+  v8::Local<v8::Promise> Set(const base::DictionaryValue& details);
-              const std::string& name,
+  v8::Local<v8::Promise> Remove(const GURL& url, const std::string& name);
-              const base::Closure& callback);
+  v8::Local<v8::Promise> FlushStore();
-  void Set(const base::DictionaryValue& details, const SetCallback& callback);
-  void FlushStore(const base::Closure& callback);
 
   // CookieChangeNotifier subscription:
   void OnCookieChanged(const CookieDetails*);

docs/api/cookies.md

@@ -13,21 +13,30 @@ For example:
 const { session } = require('electron')
 
 // Query all cookies.
-session.defaultSession.cookies.get({}, (error, cookies) => {
+session.defaultSession.cookies.get({})
-  console.log(error, cookies)
+  .then((cookies) => {
-})
+    console.log(cookies)
+  }).catch((error) => {
+    console.log(error)
+  })
 
 // Query all cookies associated with a specific url.
-session.defaultSession.cookies.get({ url: 'http://www.github.com' }, (error, cookies) => {
+session.defaultSession.cookies.get({ url: 'http://www.github.com' })
-  console.log(error, cookies)
+  .then((cookies) => {
-})
+    console.log(cookies)
+  }).catch((error) => {
+    console.log(error)
+  })
 
 // Set a cookie with the given cookie data;
 // may overwrite equivalent cookies if they exist.
 const cookie = { url: 'http://www.github.com', name: 'dummy_name', value: 'dummy' }
-session.defaultSession.cookies.set(cookie, (error) => {
+session.defaultSession.cookies.set(cookie)
-  if (error) console.error(error)
+  .then(() => {
-})
+    // success
+  }, (error) => {
+    console.error(error)
+  })
 ```
 
 ### Instance Events
@@ -55,6 +64,23 @@ expired.
 
 The following methods are available on instances of `Cookies`:
 
+#### `cookies.get(filter)`
+
+* `filter` Object
+  * `url` String (optional) - Retrieves cookies which are associated with
+    `url`. Empty implies retrieving cookies of all urls.
+  * `name` String (optional) - Filters cookies by name.
+  * `domain` String (optional) - Retrieves cookies whose domains match or are
+    subdomains of `domains`.
+  * `path` String (optional) - Retrieves cookies whose path matches `path`.
+  * `secure` Boolean (optional) - Filters cookies by their Secure property.
+  * `session` Boolean (optional) - Filters out session or persistent cookies.
+
+Returns `Promise<Cookie[]>` - A promise which resolves an array of cookie objects.
+
+Sends a request to get all cookies matching `filter`, and resolves a promise with
+the response.
+
 #### `cookies.get(filter, callback)`
 
 * `filter` Object
@@ -73,6 +99,28 @@ The following methods are available on instances of `Cookies`:
 Sends a request to get all cookies matching `filter`, `callback` will be called
 with `callback(error, cookies)` on complete.
 
+**[Deprecated Soon](promisification.md)**
+
+#### `cookies.set(details)`
+
+* `details` Object
+  * `url` String - The url to associate the cookie with.
+  * `name` String (optional) - The name of the cookie. Empty by default if omitted.
+  * `value` String (optional) - The value of the cookie. Empty by default if omitted.
+  * `domain` String (optional) - The domain of the cookie. Empty by default if omitted.
+  * `path` String (optional) - The path of the cookie. Empty by default if omitted.
+  * `secure` Boolean (optional) - Whether the cookie should be marked as Secure. Defaults to
+    false.
+  * `httpOnly` Boolean (optional) - Whether the cookie should be marked as HTTP only.
+    Defaults to false.
+  * `expirationDate` Double (optional) - The expiration date of the cookie as the number of
+    seconds since the UNIX epoch. If omitted then the cookie becomes a session
+    cookie and will not be retained between sessions.
+
+Returns `Promise<void>` - A promise which resolves when the cookie has been set
+
+Sets a cookie with `details`.
+
 #### `cookies.set(details, callback)`
 
 * `details` Object
@@ -94,6 +142,17 @@ with `callback(error, cookies)` on complete.
 Sets a cookie with `details`, `callback` will be called with `callback(error)`
 on complete.
 
+**[Deprecated Soon](promisification.md)**
+
+#### `cookies.remove(url, name)`
+
+* `url` String - The URL associated with the cookie.
+* `name` String - The name of cookie to remove.
+
+Returns `Promise<void>` - A promise which resolves when the cookie has been removed
+
+Removes the cookies matching `url` and `name`
+
 #### `cookies.remove(url, name, callback)`
 
 * `url` String - The URL associated with the cookie.
@@ -103,8 +162,18 @@ on complete.
 Removes the cookies matching `url` and `name`, `callback` will called with
 `callback()` on complete.
 
+**[Deprecated Soon](promisification.md)**
+
+#### `cookies.flushStore()`
+
+Returns `Promise<void>` - A promise which resolves when the cookie store has been flushed
+
+Writes any unwritten cookies data to disk.
+
 #### `cookies.flushStore(callback)`
 
 * `callback` Function
 
 Writes any unwritten cookies data to disk.
+
+**[Deprecated Soon](promisification.md)**

docs/api/promisification.md

@@ -18,10 +18,6 @@ When a majority of affected functions are migrated, this flag will be enabled by
 - [contentTracing.stopMonitoring(callback)](https://github.com/electron/electron/blob/master/docs/api/content-tracing.md#stopMonitoring)
 - [contentTracing.captureMonitoringSnapshot(resultFilePath, callback)](https://github.com/electron/electron/blob/master/docs/api/content-tracing.md#captureMonitoringSnapshot)
 - [contentTracing.getTraceBufferUsage(callback)](https://github.com/electron/electron/blob/master/docs/api/content-tracing.md#getTraceBufferUsage)
-- [cookies.get(filter, callback)](https://github.com/electron/electron/blob/master/docs/api/cookies.md#get)
-- [cookies.set(details, callback)](https://github.com/electron/electron/blob/master/docs/api/cookies.md#set)
-- [cookies.remove(url, name, callback)](https://github.com/electron/electron/blob/master/docs/api/cookies.md#remove)
-- [cookies.flushStore(callback)](https://github.com/electron/electron/blob/master/docs/api/cookies.md#flushStore)
 - [debugger.sendCommand(method[, commandParams, callback])](https://github.com/electron/electron/blob/master/docs/api/debugger.md#sendCommand)
 - [dialog.showOpenDialog([browserWindow, ]options[, callback])](https://github.com/electron/electron/blob/master/docs/api/dialog.md#showOpenDialog)
 - [dialog.showSaveDialog([browserWindow, ]options[, callback])](https://github.com/electron/electron/blob/master/docs/api/dialog.md#showSaveDialog)
@@ -52,10 +48,14 @@ When a majority of affected functions are migrated, this flag will be enabled by
 
 ### Converted Functions
 
-- [win.capturePage([rect, ]callback)](https://github.com/electron/electron/blob/master/docs/api/browser-window.md#capturePage)
-- [webviewTag.capturePage([rect, ]callback)](https://github.com/electron/electron/blob/master/docs/api/webview-tag.md#capturePage)
-- [contents.capturePage([rect, ]callback)](https://github.com/electron/electron/blob/master/docs/api/web-contents.md#capturePage)
 - [app.getFileIcon(path[, options], callback)](https://github.com/electron/electron/blob/master/docs/api/app.md#getFileIcon)
-- [shell.openExternal(url[, options, callback])](https://github.com/electron/electron/blob/master/docs/api/shell.md#openExternal)
+- [contents.capturePage([rect, ]callback)](https://github.com/electron/electron/blob/master/docs/api/web-contents.md#capturePage)
-- [protocol.isProtocolHandled(scheme, callback)](https://github.com/electron/electron/blob/master/docs/api/protocol.md#isProtocolHandled)
+- [cookies.flushStore(callback)](https://github.com/electron/electron/blob/master/docs/api/cookies.md#flushStore)
+- [cookies.get(filter, callback)](https://github.com/electron/electron/blob/master/docs/api/cookies.md#get)
+- [cookies.remove(url, name, callback)](https://github.com/electron/electron/blob/master/docs/api/cookies.md#remove)
+- [cookies.set(details, callback)](https://github.com/electron/electron/blob/master/docs/api/cookies.md#set)
 - [desktopCapturer.getSources(options, callback)](https://github.com/electron/electron/blob/master/docs/api/desktop-capturer.md#getSources)
+- [protocol.isProtocolHandled(scheme, callback)](https://github.com/electron/electron/blob/master/docs/api/protocol.md#isProtocolHandled)
+- [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)
+- [win.capturePage([rect, ]callback)](https://github.com/electron/electron/blob/master/docs/api/browser-window.md#capturePage)

lib/browser/api/session.js

@@ -2,7 +2,22 @@
 
 const { EventEmitter } = require('events')
 const { app, deprecate } = require('electron')
-const { fromPartition, Session, Cookies } = process.atomBinding('session')
+const { Session, Cookies } = process.atomBinding('session')
+const realFromPartition = process.atomBinding('session').fromPartition
+
+const wrappedSymbol = Symbol('wrapped-deprecate')
+const fromPartition = (partition) => {
+  const session = realFromPartition(partition)
+  if (!session[wrappedSymbol]) {
+    session[wrappedSymbol] = true
+    const { cookies } = session
+    cookies.flushStore = deprecate.promisify(cookies.flushStore, 0)
+    cookies.get = deprecate.promisify(cookies.get, 1)
+    cookies.remove = deprecate.promisify(cookies.remove, 2)
+    cookies.set = deprecate.promisify(cookies.set, 1)
+  }
+  return session
+}
 
 // Public API.
 Object.defineProperties(exports, {

spec/api-net-spec.js

@@ -550,12 +550,12 @@ describe('net module', () => {
             handleUnexpectedURL(request, response)
         }
       })
+
       customSession.cookies.set({
         url: `${server.url}`,
         name: 'test',
         value: '11111'
-      }, (error) => {
+      }).then(() => { // resolved
-        if (error) return done(error)
         const urlRequest = net.request({
           method: 'GET',
           url: `${server.url}${requestUrl}`,
@@ -575,6 +575,8 @@ describe('net module', () => {
         assert.strictEqual(urlRequest.getHeader(cookieHeaderName),
           cookieHeaderValue)
         urlRequest.end()
+      }, (error) => {
+        done(error)
       })
     })
 

spec/api-session-spec.js

@@ -68,151 +68,212 @@ describe('session module', () => {
   })
 
   describe('ses.cookies', () => {
-    it('should get cookies', (done) => {
+    const name = '0'
+    const value = '0'
+
+    it('should get cookies with promises', (done) => {
       const server = http.createServer((req, res) => {
-        res.setHeader('Set-Cookie', ['0=0'])
+        res.setHeader('Set-Cookie', [`${name}=${value}`])
         res.end('finished')
         server.close()
       })
       server.listen(0, '127.0.0.1', () => {
-        const port = server.address().port
+        w.webContents.once('did-finish-load', async () => {
-        w.webContents.once('did-finish-load', () => {
+          const list = await w.webContents.session.cookies.get({ url })
-          w.webContents.session.cookies.get({ url }, (error, list) => {
+          const cookie = list.find(cookie => cookie.name === name)
-            if (error) return done(error)
+
-            for (let i = 0; i < list.length; i++) {
+          expect(cookie).to.exist.and.to.have.property('value', value)
-              const cookie = list[i]
+          done()
-              if (cookie.name === '0') {
-                if (cookie.value === '0') {
-                  return done()
-                } else {
-                  return done(`cookie value is ${cookie.value} while expecting 0`)
-                }
-              }
-            }
-            done('Can\'t find cookie')
-          })
         })
+        const { port } = server.address()
         w.loadURL(`${url}:${port}`)
       })
     })
 
-    it('calls back with an error when setting a cookie with missing required fields', (done) => {
+    it('should get cookies with callbacks', (done) => {
-      session.defaultSession.cookies.set({
+      const server = http.createServer((req, res) => {
-        url: '',
+        res.setHeader('Set-Cookie', [`${name}=${value}`])
-        name: '1',
+        res.end('finished')
-        value: '1'
+        server.close()
-      }, (error) => {
-        assert(error, 'Should have an error')
-        assert.strictEqual(error.message, 'Setting cookie failed')
-        done()
       })
-    })
+      server.listen(0, '127.0.0.1', () => {
-
+        w.webContents.once('did-finish-load', () => {
-    it('should over-write the existent cookie', (done) => {
+          w.webContents.session.cookies.get({ url }, (error, list) => {
-      session.defaultSession.cookies.set({
-        url,
-        name: '1',
-        value: '1'
-      }, (error) => {
-        if (error) return done(error)
-        session.defaultSession.cookies.get({ url }, (error, list) => {
-          if (error) return done(error)
-          for (let i = 0; i < list.length; i++) {
-            const cookie = list[i]
-            if (cookie.name === '1') {
-              if (cookie.value === '1') {
-                return done()
-              } else {
-                return done(`cookie value is ${cookie.value} while expecting 1`)
-              }
-            }
-          }
-          done('Can\'t find cookie')
-        })
-      })
-    })
-
-    it('should remove cookies', (done) => {
-      session.defaultSession.cookies.set({
-        url: url,
-        name: '2',
-        value: '2'
-      }, (error) => {
-        if (error) return done(error)
-        session.defaultSession.cookies.remove(url, '2', () => {
-          session.defaultSession.cookies.get({ url }, (error, list) => {
             if (error) return done(error)
-            for (let i = 0; i < list.length; i++) {
+            const cookie = list.find(cookie => cookie.name === name)
-              const cookie = list[i]
+            expect(cookie).to.exist.and.to.have.property('value', value)
-              if (cookie.name === '2') return done('Cookie not deleted')
+            done()
-            }
+          })
+        })
+        const { port } = server.address()
+        w.loadURL(`${url}:${port}`)
+      })
+    })
+
+    it('sets cookies with promises', async () => {
+      let error
+      try {
+        const { cookies } = session.defaultSession
+        const name = '1'
+        const value = '1'
+
+        await cookies.set({ url, name, value })
+      } catch (e) {
+        error = e
+      }
+      expect(error).to.be.undefined(error)
+    })
+
+    it('sets cookies with callbacks', (done) => {
+      const { cookies } = session.defaultSession
+      const name = '1'
+      const value = '1'
+      cookies.set({ url, name, value }, (error, list) => done(error))
+    })
+
+    it('yields an error when setting a cookie with missing required fields', async () => {
+      let error
+      try {
+        const { cookies } = session.defaultSession
+        const name = '1'
+        const value = '1'
+        await cookies.set({ url: '', name, value })
+      } catch (e) {
+        error = e
+      }
+      expect(error).is.an('Error')
+      expect(error).to.have.property('message').which.equals('Setting cookie failed')
+    })
+
+    it('should overwrite previous cookies', async () => {
+      let error
+      try {
+        const { cookies } = session.defaultSession
+        const name = 'DidOverwrite'
+        for (const value of [ 'No', 'Yes' ]) {
+          await cookies.set({ url, name, value })
+          const list = await cookies.get({ url })
+
+          assert(list.some(cookie => cookie.name === name && cookie.value === value))
+        }
+      } catch (e) {
+        error = e
+      }
+      expect(error).to.be.undefined(error)
+    })
+
+    it('should remove cookies with promises', async () => {
+      let error
+      try {
+        const { cookies } = session.defaultSession
+        const name = '2'
+        const value = '2'
+
+        await cookies.set({ url, name, value })
+        await cookies.remove(url, name)
+        const list = await cookies.get({ url })
+
+        assert(!list.some(cookie => cookie.name === name && cookie.value === value))
+      } catch (e) {
+        error = e
+      }
+      expect(error).to.be.undefined(error)
+    })
+
+    it('should remove cookies with callbacks', (done) => {
+      const { cookies } = session.defaultSession
+      const name = '2'
+      const value = '2'
+
+      cookies.set({ url, name, value }, (error) => {
+        if (error) return done(error)
+        cookies.remove(url, name, (error) => {
+          if (error) return done(error)
+          cookies.get({ url }, (error, list) => {
+            if (error) return done(error)
+            assert(!list.some(cookie => cookie.name === name))
             done()
           })
         })
       })
     })
 
-    it('should set cookie for standard scheme', (done) => {
+    it('should set cookie for standard scheme', async () => {
-      const standardScheme = remote.getGlobal('standardScheme')
+      let error
-      const origin = standardScheme + '://fake-host'
+      try {
-      session.defaultSession.cookies.set({
+        const { cookies } = session.defaultSession
-        url: origin,
+        const standardScheme = remote.getGlobal('standardScheme')
-        name: 'custom',
+        const domain = 'fake-host'
-        value: '1'
+        const url = `${standardScheme}://${domain}`
-      }, (error) => {
+        const name = 'custom'
-        if (error) return done(error)
+        const value = '1'
-        session.defaultSession.cookies.get({ url: origin }, (error, list) => {
+
-          if (error) return done(error)
+        await cookies.set({ url, name, value })
-          assert.strictEqual(list.length, 1)
+        const list = await cookies.get({ url })
-          assert.strictEqual(list[0].name, 'custom')
+
-          assert.strictEqual(list[0].value, '1')
+        expect(list).to.have.lengthOf(1)
-          assert.strictEqual(list[0].domain, 'fake-host')
+        expect(list[0]).to.have.property('name', name)
-          done()
+        expect(list[0]).to.have.property('value', value)
-        })
+        expect(list[0]).to.have.property('domain', domain)
-      })
+      } catch (e) {
+        error = e
+      }
+
+      expect(error).to.be.undefined(error)
     })
 
-    it('emits a changed event when a cookie is added or removed', (done) => {
+    it('emits a changed event when a cookie is added or removed', async () => {
-      const { cookies } = session.fromPartition('cookies-changed')
+      let error
+      const changes = []
 
-      cookies.once('changed', (event, cookie, cause, removed) => {
+      try {
-        assert.strictEqual(cookie.name, 'foo')
+        const { cookies } = session.fromPartition('cookies-changed')
-        assert.strictEqual(cookie.value, 'bar')
+        const name = 'foo'
-        assert.strictEqual(cause, 'explicit')
+        const value = 'bar'
-        assert.strictEqual(removed, false)
+        const eventName = 'changed'
+        const listener = (event, cookie, cause, removed) => { changes.push({ cookie, cause, removed }) }
 
-        cookies.once('changed', (event, cookie, cause, removed) => {
+        cookies.on(eventName, listener)
-          assert.strictEqual(cookie.name, 'foo')
+        await cookies.set({ url, name, value })
-          assert.strictEqual(cookie.value, 'bar')
+        await cookies.remove(url, name)
-          assert.strictEqual(cause, 'explicit')
+        cookies.off(eventName, listener)
-          assert.strictEqual(removed, true)
-          done()
-        })
 
-        cookies.remove(url, 'foo', (error) => {
+        expect(changes).to.have.lengthOf(2)
-          if (error) return done(error)
+        expect(changes.every(change => change.cookie.name === name))
-        })
+        expect(changes.every(change => change.cookie.value === value))
-      })
+        expect(changes.every(change => change.cause === 'explicit'))
-
+        expect(changes[0].removed).to.be.false()
-      cookies.set({
+        expect(changes[1].removed).to.be.true()
-        url: url,
+      } catch (e) {
-        name: 'foo',
+        error = e
-        value: 'bar'
+      }
-      }, (error) => {
+      expect(error).to.be.undefined(error)
-        if (error) return done(error)
-      })
     })
 
-    describe('ses.cookies.flushStore(callback)', () => {
+    describe('ses.cookies.flushStore()', async () => {
-      it('flushes the cookies to disk and invokes the callback when done', (done) => {
+      describe('flushes the cookies to disk and invokes the callback when done', async () => {
-        session.defaultSession.cookies.set({
+        it('with promises', async () => {
-          url: url,
+          let error
-          name: 'foo',
+          try {
-          value: 'bar'
+            const name = 'foo'
-        }, (error) => {
+            const value = 'bar'
-          if (error) return done(error)
+            const { cookies } = session.defaultSession
-          session.defaultSession.cookies.flushStore(() => {
+
-            done()
+            await cookies.set({ url, name, value })
+            await cookies.flushStore()
+          } catch (e) {
+            error = e
+          }
+          expect(error).to.be.undefined(error)
+        })
+        it('with callbacks', (done) => {
+          const name = 'foo'
+          const value = 'bar'
+          const { cookies } = session.defaultSession
+
+          cookies.set({ url, name, value }, error => {
+            if (error) return done(error)
+            cookies.flushStore(error => done(error))
           })
         })
       })
@@ -232,7 +293,7 @@ describe('session module', () => {
             { env: { PHASE: phase, ...process.env } }
           )
 
-          appProcess.stdout.on('data', (data) => { output += data })
+          appProcess.stdout.on('data', data => { output += data })
           appProcess.stdout.on('end', () => {
             output = output.replace(/(\r\n|\n|\r)/gm, '')
             assert.strictEqual(output, result)

spec/fixtures/api/cookie-app/main.js

@@ -3,51 +3,30 @@ const { app, session } = require('electron')
 app.on('ready', async function () {
   const url = 'http://foo.bar'
   const persistentSession = session.fromPartition('persist:ence-test')
+  const name = 'test'
+  const value = 'true'
 
-  const set = () => {
+  const set = () => persistentSession.cookies.set({
-    return new Promise((resolve, reject) => {
+    url,
-      persistentSession.cookies.set({
+    name,
-        url,
+    value,
-        name: 'test',
+    expirationDate: Date.now() + 60000
-        value: 'true',
+  })
-        expirationDate: Date.now() + 60000
-      }, error => {
-        if (error) {
-          reject(error)
-        } else {
-          resolve()
-        }
-      })
-    })
-  }
 
-  const get = () => {
+  const get = () => persistentSession.cookies.get({
-    return new Promise((resolve, reject) => {
+    url
-      persistentSession.cookies.get({ url }, (error, list) => {
+  })
-        if (error) {
-          reject(error)
-        } else {
-          resolve(list)
-        }
-      })
-    })
-  }
 
-  const maybeRemove = (pred) => {
+  const maybeRemove = async (pred) => new Promise(async (resolve, reject) => {
-    return new Promise((resolve, reject) => {
+    try {
       if (pred()) {
-        persistentSession.cookies.remove(url, 'test', error => {
+        await persistentSession.cookies.remove(url, name)
-          if (error) {
-            reject(error)
-          } else {
-            resolve()
-          }
-        })
-      } else {
-        resolve()
       }
-    })
+      resolve()
-  }
+    } catch (error) {
+      reject(error)
+    }
+  })
 
   try {
     await maybeRemove(() => process.env.PHASE === 'one')