diff --git a/atom/browser/api/atom_api_debugger.cc b/atom/browser/api/atom_api_debugger.cc index 616cc827a231e..d5ed58bed4d34 100644 --- a/atom/browser/api/atom_api_debugger.cc +++ b/atom/browser/api/atom_api_debugger.cc @@ -61,23 +61,26 @@ void Debugger::DispatchProtocolMessage(DevToolsAgentHost* agent_host, params.Swap(params_value); Emit("message", method, params); } else { - auto send_command_callback = pending_requests_[id]; - pending_requests_.erase(id); - if (send_command_callback.is_null()) + auto it = pending_requests_.find(id); + if (it == pending_requests_.end()) return; - base::DictionaryValue* error_body = nullptr; - base::DictionaryValue error; - bool has_error; - if ((has_error = dict->GetDictionary("error", &error_body))) { - error.Swap(error_body); - } - base::DictionaryValue* result_body = nullptr; - base::DictionaryValue result; - if (dict->GetDictionary("result", &result_body)) - result.Swap(result_body); - send_command_callback.Run(has_error ? error.Clone() : base::Value(), - result); + scoped_refptr promise = it->second; + pending_requests_.erase(it); + + base::DictionaryValue* error = nullptr; + if (dict->GetDictionary("error", &error)) { + std::string message; + error->GetString("message", &message); + promise->RejectWithErrorMessage(message); + } else { + base::DictionaryValue* result_body = nullptr; + base::DictionaryValue result; + if (dict->GetDictionary("result", &result_body)) { + result.Swap(result_body); + } + promise->Resolve(result); + } } } @@ -125,23 +128,26 @@ void Debugger::Detach() { AgentHostClosed(agent_host_.get()); } -void Debugger::SendCommand(mate::Arguments* args) { - if (!agent_host_) - return; +v8::Local Debugger::SendCommand(mate::Arguments* args) { + scoped_refptr promise = new util::Promise(isolate()); + + if (!agent_host_) { + promise->RejectWithErrorMessage("No target available"); + return promise->GetHandle(); + } std::string method; if (!args->GetNext(&method)) { - args->ThrowError(); - return; + promise->RejectWithErrorMessage("Invalid method"); + return promise->GetHandle(); } + base::DictionaryValue command_params; args->GetNext(&command_params); - SendCommandCallback callback; - args->GetNext(&callback); base::DictionaryValue request; int request_id = ++previous_request_id_; - pending_requests_[request_id] = callback; + pending_requests_[request_id] = promise; request.SetInteger("id", request_id); request.SetString("method", method); if (!command_params.empty()) @@ -151,16 +157,13 @@ void Debugger::SendCommand(mate::Arguments* args) { std::string json_args; base::JSONWriter::Write(request, &json_args); agent_host_->DispatchProtocolMessage(this, json_args); + + return promise->GetHandle(); } void Debugger::ClearPendingRequests() { - if (pending_requests_.empty()) - return; - base::Value error(base::Value::Type::DICTIONARY); - base::Value error_msg("target closed while handling command"); - error.SetKey("message", std::move(error_msg)); for (const auto& it : pending_requests_) - it.second.Run(error, base::Value()); + it.second->RejectWithErrorMessage("target closed while handling command"); } // static diff --git a/atom/browser/api/atom_api_debugger.h b/atom/browser/api/atom_api_debugger.h index 9509801e84982..b44ab8721c85c 100644 --- a/atom/browser/api/atom_api_debugger.h +++ b/atom/browser/api/atom_api_debugger.h @@ -9,6 +9,7 @@ #include #include "atom/browser/api/trackable_object.h" +#include "atom/common/promise_util.h" #include "base/callback.h" #include "base/values.h" #include "content/public/browser/devtools_agent_host_client.h" @@ -32,9 +33,6 @@ class Debugger : public mate::TrackableObject, public content::DevToolsAgentHostClient, public content::WebContentsObserver { public: - using SendCommandCallback = - base::Callback; - static mate::Handle Create(v8::Isolate* isolate, content::WebContents* web_contents); @@ -56,12 +54,12 @@ class Debugger : public mate::TrackableObject, content::RenderFrameHost* new_rfh) override; private: - using PendingRequestMap = std::map; + using PendingRequestMap = std::map>; void Attach(mate::Arguments* args); bool IsAttached(); void Detach(); - void SendCommand(mate::Arguments* args); + v8::Local SendCommand(mate::Arguments* args); void ClearPendingRequests(); content::WebContents* web_contents_; // Weak Reference. diff --git a/docs/api/debugger.md b/docs/api/debugger.md index ca803e0924836..d41d4d28a5eed 100644 --- a/docs/api/debugger.md +++ b/docs/api/debugger.md @@ -60,6 +60,20 @@ Detaches the debugger from the `webContents`. Send given command to the debugging target. +**[Deprecated Soon](promisification.md)** + +#### `debugger.sendCommand(method[, commandParams])` + +* `method` String - Method name, should be one of the methods defined by the + [remote debugging protocol][rdp]. +* `commandParams` Object (optional) - JSON object with request parameters. + +Returns `Promise` - A promise that resolves with the response defined by +the 'returns' attribute of the command description in the remote debugging protocol +or is rejected indicating the failure of the command. + +Send given command to the debugging target. + ### Instance Events #### Event: 'detach' diff --git a/docs/api/promisification.md b/docs/api/promisification.md index c2b2fb965ea75..5aac18b25c781 100644 --- a/docs/api/promisification.md +++ b/docs/api/promisification.md @@ -10,7 +10,6 @@ When a majority of affected functions are migrated, this flag will be enabled by - [app.importCertificate(options, callback)](https://github.com/electron/electron/blob/master/docs/api/app.md#importCertificate) - [contentTracing.getTraceBufferUsage(callback)](https://github.com/electron/electron/blob/master/docs/api/content-tracing.md#getTraceBufferUsage) -- [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) - [dialog.showMessageBox([browserWindow, ]options[, callback])](https://github.com/electron/electron/blob/master/docs/api/dialog.md#showMessageBox) @@ -49,6 +48,7 @@ When a majority of affected functions are migrated, this flag will be enabled by - [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) +- [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) - [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) diff --git a/lib/browser/api/web-contents.js b/lib/browser/api/web-contents.js index 82b16299b1a84..00797de65ea66 100644 --- a/lib/browser/api/web-contents.js +++ b/lib/browser/api/web-contents.js @@ -501,6 +501,8 @@ WebContents.prototype._init = function () { // JavaScript wrapper of Debugger. const { Debugger } = process.atomBinding('debugger') +Debugger.prototype.sendCommand = deprecate.promisify(Debugger.prototype.sendCommand) + Object.setPrototypeOf(Debugger.prototype, EventEmitter.prototype) // Public APIs. diff --git a/spec/api-debugger-spec.js b/spec/api-debugger-spec.js index 662d79acdaaeb..16f38985e12f3 100644 --- a/spec/api-debugger-spec.js +++ b/spec/api-debugger-spec.js @@ -2,6 +2,7 @@ const chai = require('chai') const dirtyChai = require('dirty-chai') const http = require('http') const path = require('path') +const { emittedOnce } = require('./events-helpers') const { closeWindow } = require('./window-helpers') const { BrowserWindow } = require('electron').remote @@ -102,7 +103,21 @@ describe('debugger module', () => { } }) - it('returns response', done => { + it('returns response', async () => { + w.webContents.loadURL('about:blank') + w.webContents.debugger.attach() + + const params = { 'expression': '4+2' } + const res = await w.webContents.debugger.sendCommand('Runtime.evaluate', params) + + expect(res.wasThrown).to.be.undefined() + expect(res.result.value).to.equal(6) + + w.webContents.debugger.detach() + }) + + // TODO(miniak): remove when promisification is complete + it('returns response (callback)', done => { w.webContents.loadURL('about:blank') try { w.webContents.debugger.attach() @@ -123,7 +138,24 @@ describe('debugger module', () => { w.webContents.debugger.sendCommand('Runtime.evaluate', params, callback) }) - it('returns response when devtools is opened', done => { + it('returns response when devtools is opened', async () => { + w.webContents.loadURL('about:blank') + w.webContents.debugger.attach() + + w.webContents.openDevTools() + await emittedOnce(w.webContents, 'devtools-opened') + + const params = { 'expression': '4+2' } + const res = await w.webContents.debugger.sendCommand('Runtime.evaluate', params) + + expect(res.wasThrown).to.be.undefined() + expect(res.result.value).to.equal(6) + + w.webContents.debugger.detach() + }) + + // TODO(miniak): remove when promisification is complete + it('returns response when devtools is opened (callback)', done => { w.webContents.loadURL('about:blank') try { w.webContents.debugger.attach() @@ -169,7 +201,18 @@ describe('debugger module', () => { w.webContents.debugger.sendCommand('Console.enable') }) - it('returns error message when command fails', done => { + it('returns error message when command fails', async () => { + w.webContents.loadURL('about:blank') + w.webContents.debugger.attach() + + const promise = w.webContents.debugger.sendCommand('Test') + await expect(promise).to.be.eventually.rejectedWith(Error, "'Test' wasn't found") + + w.webContents.debugger.detach() + }) + + // TODO(miniak): remove when promisification is complete + it('returns error message when command fails (callback)', done => { w.webContents.loadURL('about:blank') try { w.webContents.debugger.attach() @@ -177,9 +220,8 @@ describe('debugger module', () => { done(`unexpected error : ${err}`) } - w.webContents.debugger.sendCommand('Test', err => { - expect(err).to.not.be.null() - expect(err.message).to.equal("'Test' wasn't found") + w.webContents.debugger.sendCommand('Test', (err, res) => { + expect(err).to.be.an.instanceOf(Error).with.property('message', "'Test' wasn't found") w.webContents.debugger.detach() done() }) diff --git a/spec/chromium-spec.js b/spec/chromium-spec.js index 7c5e09d42f50b..352ab6ac69687 100644 --- a/spec/chromium-spec.js +++ b/spec/chromium-spec.js @@ -1515,11 +1515,7 @@ describe('font fallback', () => { try { await w.loadURL(`data:text/html,${html}`) w.webContents.debugger.attach() - const sendCommand = (...args) => new Promise((resolve, reject) => { - w.webContents.debugger.sendCommand(...args, (e, r) => { - if (e) { reject(e) } else { resolve(r) } - }) - }) + const sendCommand = (...args) => w.webContents.debugger.sendCommand(...args) const { nodeId } = (await sendCommand('DOM.getDocument')).root.children[0] await sendCommand('CSS.enable') const { fonts } = await sendCommand('CSS.getPlatformFontsForNode', { nodeId })