Misc debugger enhancements and bug fixes....

src/debug-agent.cc

 // Copyright 2009 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
 //       with the distribution.
 //     * Neither the name of Google Inc. nor the names of its
 //       contributors may be used to endorse or promote products derived
 //       from this software without specific prior written permission.
 //
 // THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
 // "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
 // LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
 // A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
 // OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
 // SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
 // LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
 // DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
 // THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 
 #include "v8.h"
+#include "debug.h"
 #include "debug-agent.h"
 
 #ifdef ENABLE_DEBUGGER_SUPPORT
+
 namespace v8 {
 namespace internal {
 
 // Public V8 debugger API message handler function. This function just delegates
 // to the debugger agent through it's data parameter.
 void DebuggerAgentMessageHandler(const v8::Debug::Message& message) {
   DebuggerAgent::instance_->DebuggerMessage(message);
 }
 
 // static
@@ skipping 117 matching lines @@
 
 
 void DebuggerAgentSession::Run() {
   // Send the hello message.
   bool ok = DebuggerAgentUtil::SendConnectMessage(client_, *agent_->name_);
   if (!ok) return;
 
   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();
     }
 
     // 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;
+    }
   }
 }
 
 
 void DebuggerAgentSession::DebuggerMessage(Vector<uint16_t> message) {
   DebuggerAgentUtil::SendMessage(client_, message);
 }
 
 
 void DebuggerAgentSession::Shutdown() {
@@ skipping 215 matching lines @@
       return total_received;
     }
     total_received += received;
   }
   return total_received;
 }
 
 } }  // namespace v8::internal
 
 #endif  // ENABLE_DEBUGGER_SUPPORT