LGTM.

Please mention the issue number in the commit message.



http://codereview.chromium.org/6118001/diff/1/src/handles.cc
File src/handles.cc (right):

http://codereview.chromium.org/6118001/diff/1/src/handles.cc#newcode428
src/handles.cc:428: ASSERT(!(object->HasPixelElements() ||
object->HasExternalArrayElements()));
I like to split asserts so that there is only one condition per assert:

ASSERT(!object->HasPixelElements());
ASSERT(!object->HasExternalArrayElements());

http://codereview.chromium.org/6118001/diff/1/src/handles.h
File src/handles.h (right):

http://codereview.chromium.org/6118001/diff/1/src/handles.h#newcode220
src/handles.h:220: Handle<Object> IgnoreAttributesAndSetLocalProperty(
Maybe the name should rather be SetLocalPropertyAndIgnoreAttributes?

http://codereview.chromium.org/6118001/

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

Reply via email to