fix: don't terminate existing sessions when opening devtools

electron · Sep 12, 2018 · 8cb294d · 8cb294d
1 parent 5e81d8d
commit 8cb294d
Show file tree

Hide file tree

Showing 5 changed files with 60 additions and 6 deletions.
diff --git 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
@@ -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
@@ -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<content::DevToolsAgentHost> 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
@@ -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
@@ -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')}`