https://codereview.chromium.org/753553002/diff/80001/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/753553002/diff/80001/include/v8.h#newcode422
include/v8.h:422: class CallbackData {
On 2014/12/03 11:56:35, rmcilroy wrote:
nit - BaseCallbackData or maybe even BaseWeakCallbackData?

Would it make sense to put this in a non-public namespace to make it
obvious
this isn't intended to be used as part of the external API.

Done.

https://codereview.chromium.org/753553002/diff/80001/include/v8.h#newcode435
include/v8.h:435:
On 2014/12/03 11:56:35, rmcilroy wrote:
nit - two newlines between classes in V8

Done.

https://codereview.chromium.org/753553002/diff/80001/include/v8.h#newcode463
include/v8.h:463: : CallbackData<T>(isolate, NULL),
On 2014/12/03 11:56:35, rmcilroy wrote:
Do we want to expose NULL parameters to PhantomCallbacks - it feels
like that
could get error prone if the caller has to know whether to check if
GetParameter() returns null (maybe this is already the case though)?
Would it
make sense to have different callbacks for either the internal field
version and
the parameter version?

Fixed to pass a type that has only valid fields to the callback.

https://codereview.chromium.org/753553002/diff/80001/include/v8.h#newcode510
include/v8.h:510: V8_INLINE void Empty() { val_ = NULL; }
On 2014/12/02 10:40:48, Erik Corry wrote:
On 2014/12/02 10:25:59, jochen (slow) wrote:
> what do you need Empty() for?

It seems I don't need it any more.  Removed

Actually I need this for the corresponding Blink change.  In the
callback for the scriptwrappable we empty the handle, which has been
zapped on the V8 side. This means we don't try to touch it from the
destructor later.

https://codereview.chromium.org/753553002/diff/80001/src/global-handles.cc
File src/global-handles.cc (right):

https://codereview.chromium.org/753553002/diff/80001/src/global-handles.cc#newcode662
src/global-handles.cc:662: // We stil need to visit the pointer in case
the object moved,
On 2014/12/03 11:56:35, rmcilroy wrote:
/s/stil/still

Done.

https://codereview.chromium.org/753553002/

--
--
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.

Reply via email to