LGTM, but I got some suggestions. And a test case would be very nice.
https://codereview.chromium.org/11421100/diff/2001/src/liveedit-debugger.js File src/liveedit-debugger.js (right): https://codereview.chromium.org/11421100/diff/2001/src/liveedit-debugger.js#newcode86 src/liveedit-debugger.js:86: failure.details = details; On 2012/11/28 15:51:28, Yang wrote:
I'd put this after the following if, even though this doesn't really
make a
difference.
I still think this would make sense. https://codereview.chromium.org/11421100/diff/2001/src/liveedit.cc File src/liveedit.cc (right): https://codereview.chromium.org/11421100/diff/2001/src/liveedit.cc#newcode943 src/liveedit.cc:943: isolate->clear_pending_exception(); If I'm not mistaken, ReportPendingMessages() already clears the pending exception and the pending message. https://codereview.chromium.org/11421100/diff/2001/src/liveedit.cc#newcode946 src/liveedit.cc:946: if (!try_catch.Exception().IsEmpty()) { use try_catch.HasCaught() seems cleaner. https://codereview.chromium.org/11421100/diff/2001/src/liveedit.cc#newcode951 src/liveedit.cc:951: if (exception->IsJSObject() && !try_catch.Message().IsEmpty()) { It seems that GetLineNumber() and the Get*Column() methods call into javascript to calculate those from start position and end position. I suggest attaching only message->script(), message->start_position() and message->end_position() to the error object, and use script.locationFromPosition to compute the rest when the exception is passed into javascript. https://codereview.chromium.org/11421100/diff/2001/src/liveedit.cc#newcode984 src/liveedit.cc:984: if (rethrow_exception.is_null()) { Would it be possible to use try_catch.ReThrow()? https://codereview.chromium.org/11421100/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
