feat: promisify netLog.stopLogging (#16862)

atom/browser/api/atom_api_net_log.cc

@@ -20,6 +20,15 @@
 
 #include "atom/common/node_includes.h"
 
+namespace {
+
+void OnGetFilePathToCompletedLog(scoped_refptr<atom::util::Promise> promise,
+                                 const base::FilePath& file_path) {
+  promise->Resolve(file_path);
+}
+
+}  // namespace
+
 namespace atom {
 
 namespace api {
@@ -87,19 +96,17 @@ std::string NetLog::GetCurrentlyLoggingPath() const {
   return std::string();
 }
 
-void NetLog::StopLogging(mate::Arguments* args) {
-  net_log::NetExportFileWriter::FilePathCallback callback;
-  if (!args->GetNext(&callback)) {
-    args->ThrowError("Invalid callback function");
-    return;
-  }
+v8::Local<v8::Promise> NetLog::StopLogging(mate::Arguments* args) {
+  scoped_refptr<util::Promise> promise = new util::Promise(isolate());
 
   if (IsCurrentlyLogging()) {
-    stop_callback_queue_.emplace_back(callback);
+    stop_callback_queue_.emplace_back(promise);
     net_log_writer_->StopNetLog(nullptr);
   } else {
-    callback.Run(base::FilePath());
+    promise->Resolve(base::FilePath());
   }
+
+  return promise->GetHandle();
 }
 
 void NetLog::OnNewState(const base::DictionaryValue& state) {
@@ -109,9 +116,9 @@ void NetLog::OnNewState(const base::DictionaryValue& state) {
     return;
 
   if (GetLoggingState() == "NOT_LOGGING") {
-    for (auto& callback : stop_callback_queue_) {
-      if (!callback.is_null())
-        net_log_writer_->GetFilePathToCompletedLog(callback);
+    for (auto& promise : stop_callback_queue_) {
+      net_log_writer_->GetFilePathToCompletedLog(
+          base::Bind(&OnGetFilePathToCompletedLog, promise));
     }
     stop_callback_queue_.clear();
   }

atom/browser/api/atom_api_net_log.h

@@ -10,6 +10,7 @@
 #include <string>
 
 #include "atom/browser/api/trackable_object.h"
+#include "atom/common/promise_util.h"
 #include "base/callback.h"
 #include "base/values.h"
 #include "components/net_log/net_export_file_writer.h"
@@ -34,7 +35,7 @@ class NetLog : public mate::TrackableObject<NetLog>,
   std::string GetLoggingState() const;
   bool IsCurrentlyLogging() const;
   std::string GetCurrentlyLoggingPath() const;
-  void StopLogging(mate::Arguments* args);
+  v8::Local<v8::Promise> StopLogging(mate::Arguments* args);
 
  protected:
   explicit NetLog(v8::Isolate* isolate, AtomBrowserContext* browser_context);
@@ -46,8 +47,7 @@ class NetLog : public mate::TrackableObject<NetLog>,
  private:
   AtomBrowserContext* browser_context_;
   net_log::NetExportFileWriter* net_log_writer_;
-  std::list<net_log::NetExportFileWriter::FilePathCallback>
-      stop_callback_queue_;
+  std::list<scoped_refptr<atom::util::Promise>> stop_callback_queue_;
   std::unique_ptr<base::DictionaryValue> net_log_state_;
 
   DISALLOW_COPY_AND_ASSIGN(NetLog);

atom/browser/api/atom_api_session.cc

@@ -785,6 +785,7 @@ void Session::BuildPrototype(v8::Isolate* isolate,
 namespace {
 
 using atom::api::Cookies;
+using atom::api::NetLog;
 using atom::api::Protocol;
 using atom::api::Session;
 
@@ -811,6 +812,9 @@ void Initialize(v8::Local<v8::Object> exports,
   dict.Set(
       "Cookies",
       Cookies::GetConstructor(isolate)->GetFunction(context).ToLocalChecked());
+  dict.Set(
+      "NetLog",
+      NetLog::GetConstructor(isolate)->GetFunction(context).ToLocalChecked());
   dict.Set(
       "Protocol",
       Protocol::GetConstructor(isolate)->GetFunction(context).ToLocalChecked());

docs/api/net-log.md

@@ -7,12 +7,11 @@ Process: [Main](../glossary.md#main-process)
 ```javascript
 const { netLog } = require('electron')
 
-app.on('ready', function () {
+app.on('ready', async function () {
   netLog.startLogging('/path/to/net-log')
   // After some network events
-  netLog.stopLogging(path => {
-    console.log('Net-logs written to', path)
-  })
+  const path = await netLog.stopLogging()
+  console.log('Net-logs written to', path)
 })
 ```
 
@@ -36,6 +35,14 @@ Starts recording network events to `path`.
 
 Stops recording network events. If not called, net logging will automatically end when app quits.
 
+**[Deprecated Soon](promisification.md)**
+
+### `netLog.stopLogging()`
+
+Returns `Promise<String>` - resolves with a file path to which network logs were recorded.
+
+Stops recording network events. If not called, net logging will automatically end when app quits.
+
 ## Properties
 
 ### `netLog.currentlyLogging`

docs/api/promisification.md

@@ -15,7 +15,6 @@ When a majority of affected functions are migrated, this flag will be enabled by
 - [dialog.showCertificateTrustDialog([browserWindow, ]options, callback)](https://github.com/electron/electron/blob/master/docs/api/dialog.md#showCertificateTrustDialog)
 - [inAppPurchase.purchaseProduct(productID, quantity, callback)](https://github.com/electron/electron/blob/master/docs/api/in-app-purchase.md#purchaseProduct)
 - [inAppPurchase.getProducts(productIDs, callback)](https://github.com/electron/electron/blob/master/docs/api/in-app-purchase.md#getProducts)
-- [netLog.stopLogging([callback])](https://github.com/electron/electron/blob/master/docs/api/net-log.md#stopLogging)
 - [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.clearStorageData([options, callback])](https://github.com/electron/electron/blob/master/docs/api/session.md#clearStorageData)
@@ -46,6 +45,7 @@ When a majority of affected functions are migrated, this flag will be enabled by
 - [cookies.set(details, callback)](https://github.com/electron/electron/blob/master/docs/api/cookies.md#set)
 - [debugger.sendCommand(method[, commandParams, callback])](https://github.com/electron/electron/blob/master/docs/api/debugger.md#sendCommand)
 - [desktopCapturer.getSources(options, callback)](https://github.com/electron/electron/blob/master/docs/api/desktop-capturer.md#getSources)
+- [netLog.stopLogging([callback])](https://github.com/electron/electron/blob/master/docs/api/net-log.md#stopLogging)
 - [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)

docs/api/session.md

@@ -462,12 +462,11 @@ A [NetLog](net-log.md) object for this session.
 ```javascript
 const { app, session } = require('electron')
 
-app.on('ready', function () {
+app.on('ready', async function () {
   const netLog = session.fromPartition('some-partition').netLog
   netLog.startLogging('/path/to/net-log')
   // After some network events
-  netLog.stopLogging(path => {
-    console.log('Net-logs written to', path)
-  })
+  const path = await netLog.stopLogging()
+  console.log('Net-logs written to', path)
 })
 ```

lib/browser/api/session.js

@@ -2,7 +2,7 @@
 
 const { EventEmitter } = require('events')
 const { app, deprecate } = require('electron')
-const { fromPartition, Session, Cookies, Protocol } = process.atomBinding('session')
+const { fromPartition, Session, Cookies, NetLog, Protocol } = process.atomBinding('session')
 
 // Public API.
 Object.defineProperties(exports, {
@@ -28,4 +28,6 @@ Cookies.prototype.get = deprecate.promisify(Cookies.prototype.get)
 Cookies.prototype.remove = deprecate.promisify(Cookies.prototype.remove)
 Cookies.prototype.set = deprecate.promisify(Cookies.prototype.set)
 
+NetLog.prototype.stopLogging = deprecate.promisify(NetLog.prototype.stopLogging)
+
 Protocol.prototype.isProtocolHandled = deprecate.promisify(Protocol.prototype.isProtocolHandled)

spec/api-net-log-spec.js

@@ -60,7 +60,26 @@ describe('netLog module', () => {
     }
   })
 
-  it('should begin and end logging to file when .startLogging() and .stopLogging() is called', done => {
+  it('should begin and end logging to file when .startLogging() and .stopLogging() is called', async () => {
+    expect(netLog.currentlyLogging).to.be.false()
+    expect(netLog.currentlyLoggingPath).to.equal('')
+
+    netLog.startLogging(dumpFileDynamic)
+
+    expect(netLog.currentlyLogging).to.be.true()
+    expect(netLog.currentlyLoggingPath).to.equal(dumpFileDynamic)
+
+    const path = await netLog.stopLogging()
+
+    expect(netLog.currentlyLogging).to.be.false()
+    expect(netLog.currentlyLoggingPath).to.equal('')
+
+    expect(path).to.equal(dumpFileDynamic)
+    expect(fs.existsSync(dumpFileDynamic)).to.be.true()
+  })
+
+  // TODO(miniak): remove when promisification is complete
+  it('should begin and end logging to file when .startLogging() and .stopLogging() is called (callback)', done => {
     expect(netLog.currentlyLogging).to.be.false()
     expect(netLog.currentlyLoggingPath).to.equal('')
 
@@ -80,7 +99,20 @@ describe('netLog module', () => {
     })
   })
 
-  it('should silence when .stopLogging() is called without calling .startLogging()', done => {
+  it('should silence when .stopLogging() is called without calling .startLogging()', async () => {
+    expect(netLog.currentlyLogging).to.be.false()
+    expect(netLog.currentlyLoggingPath).to.equal('')
+
+    const path = await netLog.stopLogging()
+
+    expect(netLog.currentlyLogging).to.be.false()
+    expect(netLog.currentlyLoggingPath).to.equal('')
+
+    expect(path).to.equal('')
+  })
+
+  // TODO(miniak): remove when promisification is complete
+  it('should silence when .stopLogging() is called without calling .startLogging() (callback)', done => {
     expect(netLog.currentlyLogging).to.be.false()
     expect(netLog.currentlyLoggingPath).to.equal('')
 

spec/fixtures/api/net-log/main.js

@@ -14,14 +14,6 @@ function request () {
   })
 }
 
-function stopLogging (netLog) {
-  return new Promise((resolve) => {
-    netLog.stopLogging((path) => {
-      resolve()
-    })
-  })
-}
-
 app.on('ready', async () => {
   const netLog = session.defaultSession.netLog
 
@@ -38,7 +30,7 @@ app.on('ready', async () => {
     await request()
 
     if (process.env.TEST_MANUAL_STOP) {
-      await stopLogging(netLog)
+      await netLog.stopLogging()
     }
 
     app.quit()