Add a new weak handle callback that passes an Isolate to an embedder

Add a new weak handle callback that passes an Isolate to an embedder

BUG=
TEST=test-api.cc (IndependentWeakHandle)

[haraken, 2013-01-17 10:46:16]

As discussed offline, I'm implementing another version of weak handle callback
that passes an Isolate to an embedder.

This is a first round of patches.

- I'm not sure if introducing a new V8 API (i.e. Weaken()) is a good approach.
Comments appreciated.

- I removed GlobalHandles::IterateWeakRoots() because it is unused.

- I will update all tests in test-api.cc if the V8 API change looks OK.

michael, sven: Could you take a look?

[haraken, 2013-01-17 13:50:26]

This patch depends on https://chromiumcodereview.appspot.com/11958015/ and
https://chromiumcodereview.appspot.com/11969037/ . I'll rebase the patch for
review after landing the two patches.

[Sven Panne, 2013-01-18 08:08:11]

https://chromiumcodereview.appspot.com/11959031/diff/1/include/v8.h
File include/v8.h (right):

https://chromiumcodereview.appspot.com/11959031/diff/1/include/v8.h#newcode154
include/v8.h:154: typedef void (*WeakReferenceCallback)(Persistent<Value>
object,
We should add a remark here that this kind of callback is deprecated. Or better:
Find out if our deprecation macro somehow works for typedefs.

https://chromiumcodereview.appspot.com/11959031/diff/1/include/v8.h#newcode156
include/v8.h:156: typedef void (*WeakenedReferenceCallback)(Isolate* isolate,
Hmmm, I'm not totally happy with that name. We should come up with a better one,
but I don't have a suggestion right now.

https://chromiumcodereview.appspot.com/11959031/diff/1/include/v8.h#newcode413
include/v8.h:413: V8_INLINE(void MakeWeak(void* parameters,
WeakReferenceCallback callback));
Add a TODO comment that this should be deprecated soon.

https://chromiumcodereview.appspot.com/11959031/diff/1/include/v8.h#newcode417
include/v8.h:417: V8_INLINE(void Weaken(Isolate* isolate,
As discussed offline, the 3-arg MakeWeak is brand-new, so we can just change it
to expect the new kind of callback. No need for a third API entry where 1 should
survive in the long run.

https://chromiumcodereview.appspot.com/11959031/diff/1/include/v8.h#newcode3536
include/v8.h:3536: static void WeakenGlobal(internal::Isolate* isolate,
see above

https://chromiumcodereview.appspot.com/11959031/diff/1/include/v8.h#newcode4384
include/v8.h:4384: void Persistent<T>::Weaken(Isolate* isolate, void*
parameters,
see above

https://chromiumcodereview.appspot.com/11959031/diff/1/src/api.cc
File src/api.cc (right):

https://chromiumcodereview.appspot.com/11959031/diff/1/src/api.cc#newcode657
src/api.cc:657: void V8::WeakenGlobal(i::Isolate* isolate, i::Object** object,
no need for this, see other comments

https://chromiumcodereview.appspot.com/11959031/diff/1/src/global-handles.cc
File src/global-handles.cc (right):

https://chromiumcodereview.appspot.com/11959031/diff/1/src/global-handles.cc#...
src/global-handles.cc:278: if (new_callback() == NULL) {
This is a hack and works only because we don't actually check what is stored in
the union. Add an "callback_is_set()" predicate.

[haraken, 2013-01-22 00:27:37]

Sven: Thank you very much for detailed review! Updated the patch.

https://chromiumcodereview.appspot.com/11959031/diff/1/include/v8.h
File include/v8.h (right):

https://chromiumcodereview.appspot.com/11959031/diff/1/include/v8.h#newcode154
include/v8.h:154: typedef void (*WeakReferenceCallback)(Persistent<Value>
object,
On 2013/01/18 08:08:12, Sven Panne wrote:
> We should add a remark here that this kind of callback is deprecated. Or
better:
> Find out if our deprecation macro somehow works for typedefs.

Done.

https://chromiumcodereview.appspot.com/11959031/diff/1/include/v8.h#newcode156
include/v8.h:156: typedef void (*WeakenedReferenceCallback)(Isolate* isolate,
On 2013/01/18 08:08:12, Sven Panne wrote:
> Hmmm, I'm not totally happy with that name. We should come up with a better
one,
> but I don't have a suggestion right now.

For now I changed the name to WeakHandleCallback.

https://chromiumcodereview.appspot.com/11959031/diff/1/include/v8.h#newcode413
include/v8.h:413: V8_INLINE(void MakeWeak(void* parameters,
WeakReferenceCallback callback));
On 2013/01/18 08:08:12, Sven Panne wrote:
> Add a TODO comment that this should be deprecated soon.

Done.

https://chromiumcodereview.appspot.com/11959031/diff/1/include/v8.h#newcode417
include/v8.h:417: V8_INLINE(void Weaken(Isolate* isolate,
On 2013/01/18 08:08:12, Sven Panne wrote:
> As discussed offline, the 3-arg MakeWeak is brand-new, so we can just change
it
> to expect the new kind of callback. No need for a third API entry where 1
should
> survive in the long run.

Done.

https://chromiumcodereview.appspot.com/11959031/diff/1/include/v8.h#newcode3536
include/v8.h:3536: static void WeakenGlobal(internal::Isolate* isolate,
On 2013/01/18 08:08:12, Sven Panne wrote:
> see above

Done.

https://chromiumcodereview.appspot.com/11959031/diff/1/include/v8.h#newcode4384
include/v8.h:4384: void Persistent<T>::Weaken(Isolate* isolate, void*
parameters,
On 2013/01/18 08:08:12, Sven Panne wrote:
> see above

Done.

https://chromiumcodereview.appspot.com/11959031/diff/1/src/api.cc
File src/api.cc (right):

https://chromiumcodereview.appspot.com/11959031/diff/1/src/api.cc#newcode657
src/api.cc:657: void V8::WeakenGlobal(i::Isolate* isolate, i::Object** object,
On 2013/01/18 08:08:12, Sven Panne wrote:
> no need for this, see other comments

Done.

https://chromiumcodereview.appspot.com/11959031/diff/1/src/global-handles.cc
File src/global-handles.cc (right):

https://chromiumcodereview.appspot.com/11959031/diff/1/src/global-handles.cc#...
src/global-handles.cc:278: if (new_callback() == NULL) {
On 2013/01/18 08:08:12, Sven Panne wrote:
> This is a hack and works only because we don't actually check what is stored
in
> the union. Add an "callback_is_set()" predicate.

Done.

[Sven Panne, 2013-01-22 07:15:11]

I am currently doing a comprehensive sweep over the whole Persistent class
regarding Isolates, so I'll directly incorporate your patches, probably slightly
modified.

In general, there is currently no point in further submitting partial patches
w.r.t. Isolates, I'll revisit basically all of v8.h in Q1.

[haraken, 2013-01-22 07:20:38]

On 2013/01/22 07:15:11, Sven Panne wrote:
> I am currently doing a comprehensive sweep over the whole Persistent class
> regarding Isolates, so I'll directly incorporate your patches, probably
slightly
> modified.

OK, thanks. What I was trying to do is (1) Add WeakHandleCallback, (2)inline
MakeWeak(), Clear(), IsWeak() and IsNearDeath(). If all of these will be
incorporated into your clean-up patch, please close those bugs.

[Sven Panne, 2013-01-22 15:37:19]

A variant of this has been included in
https://codereview.chromium.org/12033011/, closing...