LGTM (with a few nits and one real comment in test-heap.cc).


https://chromiumcodereview.appspot.com/10836189/diff/1/test/cctest/test-api.cc
File test/cctest/test-api.cc (right):

https://chromiumcodereview.appspot.com/10836189/diff/1/test/cctest/test-api.cc#newcode1
test/cctest/test-api.cc:1:
Don't remove the copyright line, looks like a typo.

https://chromiumcodereview.appspot.com/10836189/diff/1/test/cctest/test-api.cc#newcode14642
test/cctest/test-api.cc:14642: v8::V8::ContextDisposedNotification();
Can we move that out of the loop to where the context is actually
disposed? Also if the line is right after the disposal, the comment is
no longer needed I think.

https://chromiumcodereview.appspot.com/10836189/diff/1/test/cctest/test-heap.cc
File test/cctest/test-heap.cc (left):

https://chromiumcodereview.appspot.com/10836189/diff/1/test/cctest/test-heap.cc#oldcode2101
test/cctest/test-heap.cc:2101: v8::V8::ContextDisposedNotification();
Yeah, that was an oversight when I added the test-case originally.

https://chromiumcodereview.appspot.com/10836189/diff/1/test/mjsunit/debug-script.js
File test/mjsunit/debug-script.js (right):

https://chromiumcodereview.appspot.com/10836189/diff/1/test/mjsunit/debug-script.js#newcode29
test/mjsunit/debug-script.js:29: // Flags: --send-idle-notification
Do multiple "// Flags" lines work? Awesome! Add an empty newline after
the second "// Flags" line.

https://chromiumcodereview.appspot.com/10836189/diff/1/test/mjsunit/debug-script.js#newcode31
test/mjsunit/debug-script.js:31:
Drop this empty newline.

https://chromiumcodereview.appspot.com/10836189/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to