I've uploaded a patch that addresses jkummerow's comments:
https://codereview.chromium.org/264793015/
https://codereview.chromium.org/263833007/diff/80001/src/object-observe.js
File src/object-observe.js (right):
https://codereview.chromium.org/263833007/diff/80001/src/object-observe.js#newcode545
src/object-observe.js:545: function
NativeObjectNotifierPerformChange(objectInfo, changeType, changeFn) {
On 2014/05/02 19:31:37, Jakob wrote:
Please avoid name clashes of JS builtins and runtime functions.
Done, I've renamed the runtime functions to make their purpose clear.
https://codereview.chromium.org/263833007/diff/80001/src/runtime.cc
File src/runtime.cc (right):
https://codereview.chromium.org/263833007/diff/80001/src/runtime.cc#newcode15003
src/runtime.cc:15003: CONVERT_ARG_HANDLE_CHECKED(Object, callback, 1);
On 2014/05/02 19:31:37, Jakob wrote:
Please be as specific as you can regarding the object type here. I
have a
feeling |callback| must be a JSFunction, so use
CONVERT_ARG_HANDLE_CHECKED(JSFunction, callback, 1);
Done.
https://codereview.chromium.org/263833007/diff/80001/src/runtime.cc#newcode15037
src/runtime.cc:15037:
RUNTIME_FUNCTION(Runtime_NativeObjectNotifierPerformChange) {
On 2014/05/02 19:31:37, Jakob wrote:
I don't see this function declared in runtime.h, is that intentional?
That is not intentional, and combined with the same-naming, it appears
that this caused us to not be doing the right thing with performChange.
I've updated the performChange leak test to actually test that
performChange changes contexts.
https://codereview.chromium.org/263833007/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
