chore: enable debugger api specs (#14475)

electron · Sep 6, 2018 · 35a1849 · 35a1849
1 parent 9bf1fb3
commit 35a1849
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 27 deletions.
diff --git a/atom/browser/api/atom_api_debugger.cc b/atom/browser/api/atom_api_debugger.cc
@@ -6,16 +6,13 @@
 
 #include <string>
 
-#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> 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
@@ -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<Debugger>,
-                 public content::DevToolsAgentHostClient {
+                 public content::DevToolsAgentHostClient,
+                 public content::WebContentsObserver {
  public:
   using SendCommandCallback =
-      base::Callback<void(const base::DictionaryValue&,
-                          const base::DictionaryValue&)>;
+      base::Callback<void(const base::Value&, const base::Value&)>;
 
   static mate::Handle<Debugger> Create(v8::Isolate* isolate,
                                        content::WebContents* web_contents);
@@ -50,13 +51,18 @@ class Debugger : public mate::TrackableObject<Debugger>,
   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<int, SendCommandCallback>;
 
   void Attach(mate::Arguments* args);
   bool IsAttached();
   void Detach();
   void SendCommand(mate::Arguments* args);
+  void ClearPendingRequests();
 
   content::WebContents* web_contents_;  // Weak Reference.
   scoped_refptr<content::DevToolsAgentHost> agent_host_;

diff --git 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) {