If you need some way of checking whether the VM is running you should add a command for that and not rely on a command like backtrace returning an empty response. When connecting to the debugger agent the VM should never be in a break state as all debugger events are ignored when no debugger is connected.
http://codereview.chromium.org/242034/diff/2009/2010 File src/debug-delay.js (right): http://codereview.chromium.org/242034/diff/2009/2010#newcode798 Line 798: ExecutionState.prototype.debugCommandProcessor = function(is_running_initial) { How about changing the name of the argument to auto_continue as it indicates whether an inplicit continue command is in effect. Also consider making it optional (just prefixing it with opt_). http://codereview.chromium.org/242034/diff/2009/2010#newcode1086 Line 1086: this.running_ = is_running_initial; Consider making this optional and default to false. this.running_ = opt_auto_continue || false; http://codereview.chromium.org/242034/diff/2009/2010#newcode1319 Line 1319: response.new_running_value = true; I still cannot see why just assigning response.running here will not work. http://codereview.chromium.org/242034/diff/2009/2010#newcode1538 Line 1538: if (this.isRunning()) { This should be removed. I see no reason for not allowing backtrace requests at all times. Even with this check the command sequence suspend, backtrace, continue will provide the same information. http://codereview.chromium.org/242034/diff/2009/2010#newcode1539 Line 1539: // No stacktrace when running Even if we want to disallow some commands in some situations the response should have success=false and a message explaining the cause of the failure. http://codereview.chromium.org/242034/diff/2009/2010#newcode1918 Line 1918: // TODO(peter.rybin): probably we need some body field here. I think you should just remove this comment. A response to the suspend command containing success=true and running=false should be all the information required for the debugger client. http://codereview.chromium.org/242034/diff/2009/2010#newcode1919 Line 1919: response.new_running_value = false; Also here, I cannot see why just assigning response.running will not work. http://codereview.chromium.org/242034/diff/2009/2012 File test/mjsunit/debug-backtrace.js (right): http://codereview.chromium.org/242034/diff/2009/2012#newcode101 Line 101: dcp = exec_state.debugCommandProcessor(false); Consider argument default to false. http://codereview.chromium.org/242034/diff/2009/2013 File test/mjsunit/debug-changebreakpoint.js (right): http://codereview.chromium.org/242034/diff/2009/2013#newcode62 Line 62: var dcp = exec_state.debugCommandProcessor("unspecified_running_state"); Passing other values than true or false does not make any sense. Please use true or false in all the tests. http://codereview.chromium.org/242034/diff/2009/2017 File test/mjsunit/debug-evaluate-recursive.js (right): http://codereview.chromium.org/242034/diff/2009/2017#newcode151 Line 151: assertTrue(listenerComplete, "msg5"); msg5? http://codereview.chromium.org/242034/diff/2009/2017#newcode166 Line 166: assertTrue(listenerComplete, "msg1"); msg1? http://codereview.chromium.org/242034/diff/2009/2025 File test/mjsunit/debug-suspend.js (right): http://codereview.chromium.org/242034/diff/2009/2025#newcode58 Line 58: var response = safeEval(dcp.processDebugJSONRequest(request)); This does not really test that the suspend command changes the running state, as the running state is false before as this code is running in a debug event listener and not a message handler with auto break enabled. To test suspend you could issue a continue command before the suspend command to see running go from false to true to false. http://codereview.chromium.org/242034/diff/2009/2025#newcode61 Line 61: } else { Empty else part. http://codereview.chromium.org/242034/diff/2009/2025#newcode67 Line 67: if (event == Debug.DebugEvent.Break) { Please indent the try code. http://codereview.chromium.org/242034 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
