From 3348e5162fe5e6e8e362534c020520562d0ad5e8 Mon Sep 17 00:00:00 2001 From: Robo Date: Wed, 12 Sep 2018 19:47:15 +0530 Subject: [PATCH] fix: don't terminate existing sessions when opening devtools (#14566) --- .../devtools_embedder_message_dispatcher.cc | 1 + .../devtools_embedder_message_dispatcher.h | 1 + .../browser/inspectable_web_contents_impl.cc | 21 +++++++--- .../browser/inspectable_web_contents_impl.h | 1 + spec/api-debugger-spec.js | 42 +++++++++++++++++++ 5 files changed, 60 insertions(+), 6 deletions(-) diff --git a/brightray/browser/devtools_embedder_message_dispatcher.cc b/brightray/browser/devtools_embedder_message_dispatcher.cc index a949eabcb8d1..dffe89bf44ac 100644 --- a/brightray/browser/devtools_embedder_message_dispatcher.cc +++ b/brightray/browser/devtools_embedder_message_dispatcher.cc @@ -212,6 +212,7 @@ DevToolsEmbedderMessageDispatcher::CreateForDevToolsFrontend( d->RegisterHandler("connectionReady", &Delegate::ConnectionReady, delegate); d->RegisterHandler("registerExtensionsAPI", &Delegate::RegisterExtensionsAPI, delegate); + d->RegisterHandlerWithCallback("reattach", &Delegate::Reattach, delegate); return d; } diff --git a/brightray/browser/devtools_embedder_message_dispatcher.h b/brightray/browser/devtools_embedder_message_dispatcher.h index 4df3bdda0948..1ec1ed133d00 100644 --- a/brightray/browser/devtools_embedder_message_dispatcher.h +++ b/brightray/browser/devtools_embedder_message_dispatcher.h @@ -95,6 +95,7 @@ class DevToolsEmbedderMessageDispatcher { virtual void ConnectionReady() = 0; virtual void RegisterExtensionsAPI(const std::string& origin, const std::string& script) = 0; + virtual void Reattach(const DispatchCallback& callback) = 0; }; using DispatchCallback = Delegate::DispatchCallback; diff --git a/brightray/browser/inspectable_web_contents_impl.cc b/brightray/browser/inspectable_web_contents_impl.cc index 5e809552c6bf..b8c805a3437f 100644 --- a/brightray/browser/inspectable_web_contents_impl.cc +++ b/brightray/browser/inspectable_web_contents_impl.cc @@ -271,7 +271,7 @@ content::WebContents* InspectableWebContentsImpl::GetDevToolsWebContents() } void InspectableWebContentsImpl::InspectElement(int x, int y) { - if (agent_host_.get()) + if (agent_host_) agent_host_->InspectElement(web_contents_->GetMainFrame(), x, y); } @@ -354,19 +354,28 @@ bool InspectableWebContentsImpl::IsDevToolsViewShowing() { void InspectableWebContentsImpl::AttachTo( scoped_refptr host) { - if (agent_host_.get()) + if (agent_host_) Detach(); agent_host_ = std::move(host); - // Terminate existing debugging connections and start debugging. - agent_host_->ForceAttachClient(this); + // We could use ForceAttachClient here if problem arises with + // devtools multiple session support. + agent_host_->AttachClient(this); } void InspectableWebContentsImpl::Detach() { - if (agent_host_.get()) + if (agent_host_) agent_host_->DetachClient(this); agent_host_ = nullptr; } +void InspectableWebContentsImpl::Reattach(const DispatchCallback& callback) { + if (agent_host_) { + agent_host_->DetachClient(this); + agent_host_->AttachClient(this); + } + callback.Run(nullptr); +} + void InspectableWebContentsImpl::CallClientFunction( const std::string& function_name, const base::Value* arg1, @@ -620,7 +629,7 @@ void InspectableWebContentsImpl::DispatchProtocolMessageFromDevToolsFrontend( return; } - if (agent_host_.get()) + if (agent_host_) agent_host_->DispatchProtocolMessage(this, message); } diff --git a/brightray/browser/inspectable_web_contents_impl.h b/brightray/browser/inspectable_web_contents_impl.h index 3f0fe0093cf7..03ff1e8482cf 100644 --- a/brightray/browser/inspectable_web_contents_impl.h +++ b/brightray/browser/inspectable_web_contents_impl.h @@ -133,6 +133,7 @@ class InspectableWebContentsImpl void ConnectionReady() override; void RegisterExtensionsAPI(const std::string& origin, const std::string& script) override; + void Reattach(const DispatchCallback& callback) override; // content::DevToolsFrontendHostDelegate: void HandleMessageFromDevToolsFrontend(const std::string& message); diff --git a/spec/api-debugger-spec.js b/spec/api-debugger-spec.js index 672649d601a0..b1098cb56762 100644 --- a/spec/api-debugger-spec.js +++ b/spec/api-debugger-spec.js @@ -72,6 +72,24 @@ describe('debugger module', () => { } w.webContents.debugger.detach() }) + + it('doesn\'t disconnect an active devtools session', done => { + w.webContents.loadURL('about:blank') + try { + w.webContents.debugger.attach() + } catch (err) { + return done(`unexpected error : ${err}`) + } + w.webContents.openDevTools() + w.webContents.once('devtools-opened', () => { + w.webContents.debugger.detach() + }) + w.webContents.debugger.on('detach', (e, reason) => { + expect(w.webContents.debugger.isAttached()).to.be.false() + expect(w.devToolsWebContents.isDestroyed()).to.be.false() + done() + }) + }) }) describe('debugger.sendCommand', () => { @@ -105,6 +123,30 @@ describe('debugger module', () => { w.webContents.debugger.sendCommand('Runtime.evaluate', params, callback) }) + it('returns response when devtools is opened', done => { + w.webContents.loadURL('about:blank') + try { + w.webContents.debugger.attach() + } catch (err) { + return done(`unexpected error : ${err}`) + } + + const callback = (err, res) => { + expect(err.message).to.be.undefined() + expect(res.wasThrown).to.be.undefined() + expect(res.result.value).to.equal(6) + + w.webContents.debugger.detach() + done() + } + + w.webContents.openDevTools() + w.webContents.once('devtools-opened', () => { + const params = {'expression': '4+2'} + w.webContents.debugger.sendCommand('Runtime.evaluate', params, callback) + }) + }) + it('fires message event', done => { const url = process.platform !== 'win32' ? `file://${path.join(fixtures, 'pages', 'a.html')}`