Misc debugger enhancements and bug fixes....

src/debug-agent.cc

Index: src/debug-agent.cc
===================================================================
--- src/debug-agent.cc	(revision 6117)
+++ src/debug-agent.cc	(working copy)
@@ -27,9 +27,11 @@
 
 
 #include "v8.h"
+#include "debug.h"
 #include "debug-agent.h"
 
 #ifdef ENABLE_DEBUGGER_SUPPORT
+
 namespace v8 {
 namespace internal {
 
@@ -167,22 +169,46 @@
   while (true) {
     // Read data from the debugger front end.
     SmartPointer<char> message = DebuggerAgentUtil::ReceiveMessage(client_);
-    if (*message == NULL) {
-      // Session is closed.
-      agent_->OnSessionClosed(this);
-      return;
+
+    const char *msg = *message;

[Søren Thygesen Gjesse, 2011/01/03 08:56:07]

Please avoid mixed case variables, see
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Variable_Names.

[marklam, 2011/01/04 20:12:13]

Done.

+    bool isBreakRequest = false;
+    bool isClosingSession = (msg == NULL);
+
+    if (msg == NULL) {
+      // If we lost the connection, then simulate a disconnect msg:
+      msg = "{\"seq\":1,\"type\":\"request\",\"command\":\"disconnect\"}";
+
+    } else {
+      // Check if we're getting a break request:

[Søren Thygesen Gjesse, 2011/01/03 08:56:07]

These checks seems pretty fragile, and depends on the debugger client formatting
of the JSON. There is now a separate JSON parser in C++ (see Runtime_ParseJson
where it is used), and maybe that can be used instead, but that might require
more refactoring as we don't want to parse the request twice.

We can probably have it like this for now.

[marklam, 2011/01/04 20:12:13]

I see what you mean.  But yes, at least as a first pass, I would rather just
leave this as is for now.  This can be re-implemented later if needed.

+      const char *breakRequestStr =
+        "\"type\":\"request\",\"command\":\"break\"}";
+      const char *result = strstr(msg, breakRequestStr);
+      if (result != NULL) {
+        isBreakRequest = true;
+      } else {
+        // Check if we're getting a disconnect request:
+        const char *disconnectRequestStr =
+          "\"type\":\"request\",\"command\":\"disconnect\"}";
+        result = strstr(msg, disconnectRequestStr);
+        if (result != NULL) {
+          isClosingSession = true;
+        }
+      }
     }
 
+    if (isBreakRequest && !Debug::InDebugger()) {

[Søren Thygesen Gjesse, 2011/01/03 08:56:07]

Is this required? Sending the command using v8::Debug::SendCommand should
trigger a break of the DEBUGCOMMAND - is that not sufficient?

[marklam, 2011/01/04 20:12:13]

Hmmm.  This used to be needed, but I just re-tested without it and things seem
to be working fine now.  Perhaps the v8 source has mutated to make this
unnecessary since the time I added it in the early days.  I've removed this if
statement along with all references to isBreakRequest above.

+      v8::Debug::DebugBreak();
+    }
+
     // Convert UTF-8 to UTF-16.
-    unibrow::Utf8InputBuffer<> buf(*message,
-                                   StrLength(*message));
+    unibrow::Utf8InputBuffer<> buf(msg, StrLength(msg));
     int len = 0;
     while (buf.has_more()) {
       buf.GetNext();
       len++;
     }
     ScopedVector<int16_t> temp(len + 1);
-    buf.Reset(*message, StrLength(*message));
+    buf.Reset(msg, StrLength(msg));
     for (int i = 0; i < len; i++) {
       temp[i] = buf.GetNext();
     }
@@ -190,6 +216,12 @@
     // Send the request received to the debugger.
     v8::Debug::SendCommand(reinterpret_cast<const uint16_t *>(temp.start()),
                            len);
+
+    if (isClosingSession) {
+      // Session is closed.
+      agent_->OnSessionClosed(this);
+      return;
+    }
   }
 }