LGTM
http://codereview.chromium.org/50007/diff/1/5 File src/debug-agent.cc (right): http://codereview.chromium.org/50007/diff/1/5#newcode52 Line 52: // If an error occoured wait a bit before retrying. More comments? http://codereview.chromium.org/50007/diff/1/5#newcode73 Line 73: terminate_now_->Wait(kOneSecondInMicros); I think unless we have a concrete situation where we know it's good to wait then we should probably not have a sleep here. http://codereview.chromium.org/50007/diff/1/5#newcode79 Line 79: void DebuggerAgent::Shutdown() { Is it possible to assert we have the lock here? http://codereview.chromium.org/50007/diff/1/2 File test/cctest/test-debug.cc (right): http://codereview.chromium.org/50007/diff/1/2#newcode3829 Line 3829: const int port = 5858; You already used port 5858 in this file. In the interest of being able to parallelize the tests reliably you should use a different number. http://codereview.chromium.org/50007 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
