diff --git a/atom/browser/api/atom_api_debugger.cc b/atom/browser/api/atom_api_debugger.cc index 59d12a991ec0..7b5c845a0d75 100644 --- a/atom/browser/api/atom_api_debugger.cc +++ b/atom/browser/api/atom_api_debugger.cc @@ -6,16 +6,13 @@ #include -#include "atom/browser/atom_browser_main_parts.h" #include "atom/common/native_mate_converters/callback.h" #include "atom/common/native_mate_converters/value_converter.h" #include "base/json/json_reader.h" #include "base/json/json_writer.h" -#include "base/memory/ptr_util.h" #include "content/public/browser/devtools_agent_host.h" #include "content/public/browser/web_contents.h" #include "native_mate/dictionary.h" -#include "native_mate/object_template_builder.h" #include "atom/common/node_includes.h" @@ -26,20 +23,22 @@ namespace atom { namespace api { Debugger::Debugger(v8::Isolate* isolate, content::WebContents* web_contents) - : web_contents_(web_contents) { + : content::WebContentsObserver(web_contents), web_contents_(web_contents) { Init(isolate); } Debugger::~Debugger() {} void Debugger::AgentHostClosed(DevToolsAgentHost* agent_host) { - std::string detach_reason = "target closed"; - Emit("detach", detach_reason); + DCHECK(agent_host == agent_host_); + agent_host_ = nullptr; + ClearPendingRequests(); + Emit("detach", "target closed"); } void Debugger::DispatchProtocolMessage(DevToolsAgentHost* agent_host, const std::string& message) { - DCHECK(agent_host == agent_host_.get()); + DCHECK(agent_host == agent_host_); v8::Locker locker(isolate()); v8::HandleScope handle_scope(isolate()); @@ -77,42 +76,52 @@ void Debugger::DispatchProtocolMessage(DevToolsAgentHost* agent_host, } } +void Debugger::RenderFrameHostChanged(content::RenderFrameHost* old_rfh, + content::RenderFrameHost* new_rfh) { + if (agent_host_) { + agent_host_->DisconnectWebContents(); + auto* web_contents = content::WebContents::FromRenderFrameHost(new_rfh); + agent_host_->ConnectWebContents(web_contents); + } +} + void Debugger::Attach(mate::Arguments* args) { std::string protocol_version; args->GetNext(&protocol_version); + if (agent_host_) { + args->ThrowError("Debugger is already attached to the target"); + return; + } + if (!protocol_version.empty() && !DevToolsAgentHost::IsSupportedProtocolVersion(protocol_version)) { args->ThrowError("Requested protocol version is not supported"); return; } + agent_host_ = DevToolsAgentHost::GetOrCreateFor(web_contents_); - if (!agent_host_.get()) { + if (!agent_host_) { args->ThrowError("No target available"); return; } - if (agent_host_->IsAttached()) { - args->ThrowError("Another debugger is already attached to this target"); - return; - } agent_host_->AttachClient(this); } bool Debugger::IsAttached() { - return agent_host_.get() ? agent_host_->IsAttached() : false; + return agent_host_ && agent_host_->IsAttached(); } void Debugger::Detach() { - if (!agent_host_.get()) + if (!agent_host_) return; agent_host_->DetachClient(this); AgentHostClosed(agent_host_.get()); - agent_host_ = nullptr; } void Debugger::SendCommand(mate::Arguments* args) { - if (!agent_host_.get()) + if (!agent_host_) return; std::string method; @@ -139,6 +148,16 @@ void Debugger::SendCommand(mate::Arguments* args) { agent_host_->DispatchProtocolMessage(this, json_args); } +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()); +} + // static mate::Handle Debugger::Create(v8::Isolate* isolate, content::WebContents* web_contents) { diff --git a/atom/browser/api/atom_api_debugger.h b/atom/browser/api/atom_api_debugger.h index d3774dae9dc5..9509801e8498 100644 --- a/atom/browser/api/atom_api_debugger.h +++ b/atom/browser/api/atom_api_debugger.h @@ -12,6 +12,7 @@ #include "base/callback.h" #include "base/values.h" #include "content/public/browser/devtools_agent_host_client.h" +#include "content/public/browser/web_contents_observer.h" #include "native_mate/handle.h" namespace content { @@ -28,11 +29,11 @@ namespace atom { namespace api { class Debugger : public mate::TrackableObject, - public content::DevToolsAgentHostClient { + public content::DevToolsAgentHostClient, + public content::WebContentsObserver { public: using SendCommandCallback = - base::Callback; + base::Callback; static mate::Handle Create(v8::Isolate* isolate, content::WebContents* web_contents); @@ -50,6 +51,10 @@ class Debugger : public mate::TrackableObject, void DispatchProtocolMessage(content::DevToolsAgentHost* agent_host, const std::string& message) override; + // content::WebContentsObserver: + void RenderFrameHostChanged(content::RenderFrameHost* old_rfh, + content::RenderFrameHost* new_rfh) override; + private: using PendingRequestMap = std::map; @@ -57,6 +62,7 @@ class Debugger : public mate::TrackableObject, bool IsAttached(); void Detach(); void SendCommand(mate::Arguments* args); + void ClearPendingRequests(); content::WebContents* web_contents_; // Weak Reference. scoped_refptr agent_host_; diff --git a/spec/api-debugger-spec.js b/spec/api-debugger-spec.js index 25dde3bd6b9a..fe7ca0422b10 100644 --- a/spec/api-debugger-spec.js +++ b/spec/api-debugger-spec.js @@ -3,7 +3,7 @@ const dirtyChai = require('dirty-chai') const http = require('http') const path = require('path') const {closeWindow} = require('./window-helpers') -const BrowserWindow = require('electron').remote.BrowserWindow +const {BrowserWindow} = require('electron').remote const {expect} = chai chai.use(dirtyChai) @@ -23,15 +23,16 @@ describe('debugger module', () => { afterEach(() => closeWindow(w).then(() => { w = null })) describe('debugger.attach', () => { - it('fails when devtools is already open', done => { + it('succeeds when devtools is already open', done => { w.webContents.on('did-finish-load', () => { w.webContents.openDevTools() try { w.webContents.debugger.attach() } catch (err) { - expect(w.webContents.debugger.isAttached()).to.be.true() - done() + done(`unexpected error : ${err}`) } + expect(w.webContents.debugger.isAttached()).to.be.true() + done() }) w.webContents.loadFile(path.join(fixtures, 'pages', 'a.html')) }) @@ -144,8 +145,7 @@ describe('debugger module', () => { }) }) - // TODO(alexeykuzmin): [Ch66] Times out. Fix it and enable back. - xit('handles valid unicode characters in message', (done) => { + it('handles valid unicode characters in message', (done) => { try { w.webContents.debugger.attach() } catch (err) { @@ -174,8 +174,7 @@ describe('debugger module', () => { }) }) - // TODO(alexeykuzmin): [Ch66] Times out. Fix it and enable back. - xit('does not crash for invalid unicode characters in message', (done) => { + it('does not crash for invalid unicode characters in message', (done) => { try { w.webContents.debugger.attach() } catch (err) {