From 7e7abc28f5fb69d440188e42f46b7cc7803ee153 Mon Sep 17 00:00:00 2001 From: Milan Burda Date: Tue, 19 Feb 2019 11:48:27 +0100 Subject: [PATCH] feat: promisify netLog.stopLogging (#16862) --- atom/browser/api/atom_api_net_log.cc | 29 +++++++++++++--------- atom/browser/api/atom_api_net_log.h | 6 ++--- atom/browser/api/atom_api_session.cc | 4 ++++ docs/api/net-log.md | 15 ++++++++---- docs/api/promisification.md | 2 +- docs/api/session.md | 7 +++--- lib/browser/api/session.js | 4 +++- spec/api-net-log-spec.js | 36 ++++++++++++++++++++++++++-- spec/fixtures/api/net-log/main.js | 10 +------- 9 files changed, 78 insertions(+), 35 deletions(-) diff --git a/atom/browser/api/atom_api_net_log.cc b/atom/browser/api/atom_api_net_log.cc index 7464b46b1dd1..793731738c1c 100644 --- a/atom/browser/api/atom_api_net_log.cc +++ b/atom/browser/api/atom_api_net_log.cc @@ -20,6 +20,15 @@ #include "atom/common/node_includes.h" +namespace { + +void OnGetFilePathToCompletedLog(scoped_refptr 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 NetLog::StopLogging(mate::Arguments* args) { + scoped_refptr 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(); } diff --git a/atom/browser/api/atom_api_net_log.h b/atom/browser/api/atom_api_net_log.h index 1afe0b43597f..b71677a12867 100644 --- a/atom/browser/api/atom_api_net_log.h +++ b/atom/browser/api/atom_api_net_log.h @@ -10,6 +10,7 @@ #include #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, std::string GetLoggingState() const; bool IsCurrentlyLogging() const; std::string GetCurrentlyLoggingPath() const; - void StopLogging(mate::Arguments* args); + v8::Local StopLogging(mate::Arguments* args); protected: explicit NetLog(v8::Isolate* isolate, AtomBrowserContext* browser_context); @@ -46,8 +47,7 @@ class NetLog : public mate::TrackableObject, private: AtomBrowserContext* browser_context_; net_log::NetExportFileWriter* net_log_writer_; - std::list - stop_callback_queue_; + std::list> stop_callback_queue_; std::unique_ptr net_log_state_; DISALLOW_COPY_AND_ASSIGN(NetLog); diff --git a/atom/browser/api/atom_api_session.cc b/atom/browser/api/atom_api_session.cc index 4ff876c779e3..021c8e862b1c 100644 --- a/atom/browser/api/atom_api_session.cc +++ b/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 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()); diff --git a/docs/api/net-log.md b/docs/api/net-log.md index fb6dd3e0b039..0792d5b0c4bb 100644 --- a/docs/api/net-log.md +++ b/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` - 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` diff --git a/docs/api/promisification.md b/docs/api/promisification.md index d986a7b269c3..096f93a26980 100644 --- a/docs/api/promisification.md +++ b/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) diff --git a/docs/api/session.md b/docs/api/session.md index 9884daf6b0ca..dd1fcab94dcb 100644 --- a/docs/api/session.md +++ b/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) }) ``` diff --git a/lib/browser/api/session.js b/lib/browser/api/session.js index b300bd8ad17e..3f860fae2c16 100644 --- a/lib/browser/api/session.js +++ b/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) diff --git a/spec/api-net-log-spec.js b/spec/api-net-log-spec.js index 033aecb7a58b..aed2b192786c 100644 --- a/spec/api-net-log-spec.js +++ b/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('') diff --git a/spec/fixtures/api/net-log/main.js b/spec/fixtures/api/net-log/main.js index d14671d939a6..bfe38a31bf48 100644 --- a/spec/fixtures/api/net-log/main.js +++ b/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()