https://codereview.chromium.org/11369154/diff/1/src/object-observe.js
File src/object-observe.js (right):

https://codereview.chromium.org/11369154/diff/1/src/object-observe.js#newcode65
src/object-observe.js:65: changeObservers: new InternalArray,
Nit: can we perhaps rename that property to just "observers"? Sounds
less like a method. :)

https://codereview.chromium.org/11369154/diff/1/src/object-observe.js#newcode147
src/object-observe.js:147: return;
This is calling the notify method on something that's not a notifier.
Shouldn't this better be an error?

https://codereview.chromium.org/11369154/diff/1/test/mjsunit/harmony/object-observe.js
File test/mjsunit/harmony/object-observe.js (right):

https://codereview.chromium.org/11369154/diff/1/test/mjsunit/harmony/object-observe.js#newcode121
test/mjsunit/harmony/object-observe.js:121: }, TypeError);
Also:  notifier.notify.call({}, { type: 'a' })
Does nothing, but perhaps should throw? (see other comment)

https://codereview.chromium.org/11369154/diff/1/test/mjsunit/harmony/object-observe.js#newcode124
test/mjsunit/harmony/object-observe.js:124: assertFalse(recordCreated);
// not observed yet
Nit: 2 spaces before //

https://codereview.chromium.org/11369154/diff/1/test/mjsunit/harmony/object-observe.js#newcode138
test/mjsunit/harmony/object-observe.js:138: object: notifier, // object
property is ignored
dito

https://codereview.chromium.org/11369154/

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

Reply via email to