new persistent semantics

new persistent semantics

adds copying and autodispose as traits

R=marja@chromium.org, mstarzinger@chromium.org, svenpanne@chromium.org

BUG=

Committed: https://code.google.com/p/v8/source/detail?r=16588

[dcarney, 2013-08-23 12:48:40]

please have a look

[marja, 2013-08-23 13:02:40]

https://chromiumcodereview.appspot.com/23401003/diff/1/include/v8.h
File include/v8.h (left):

https://chromiumcodereview.appspot.com/23401003/diff/1/include/v8.h#oldcode314
include/v8.h:314: #ifndef V8_USE_UNSAFE_HANDLES
Would it be feasible to just do one round of V8_USE_UNSAFE_HANDLES removal in a
separate CL?

(That'll break all V8 users except blink & chromium though, but so will this
CL.)

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

https://chromiumcodereview.appspot.com/23401003/diff/1/include/v8.h#newcode484
include/v8.h:484: class NonCopyablePersistentTraits {
Neat :)

https://chromiumcodereview.appspot.com/23401003/diff/1/include/v8.h#newcode4708
include/v8.h:4708: static internal::Object** CopyPersistent(internal::Object**
handle);
This creates another legit Persistent for the same object, right? (And creates a
strong reference.) Can this func be named to reflect it better?
CreateNewGlobalReference or sth?

[dcarney, 2013-08-23 13:06:10]

https://chromiumcodereview.appspot.com/23401003/diff/1/include/v8.h
File include/v8.h (left):

https://chromiumcodereview.appspot.com/23401003/diff/1/include/v8.h#oldcode314
include/v8.h:314: #ifndef V8_USE_UNSAFE_HANDLES
On 2013/08/23 13:02:40, marja wrote:
> Would it be feasible to just do one round of V8_USE_UNSAFE_HANDLES removal in
a
> separate CL?
> 
> (That'll break all V8 users except blink & chromium though, but so will this
> CL.)


I announced in the mailing list that I'd do this.  Plus they've all had lots of
warning.

[marja, 2013-08-26 07:08:14]

https://chromiumcodereview.appspot.com/23401003/diff/1/include/v8.h
File include/v8.h (left):

https://chromiumcodereview.appspot.com/23401003/diff/1/include/v8.h#oldcode314
include/v8.h:314: #ifndef V8_USE_UNSAFE_HANDLES
On 2013/08/23 13:06:10, dcarney wrote:
> On 2013/08/23 13:02:40, marja wrote:
> > Would it be feasible to just do one round of V8_USE_UNSAFE_HANDLES removal
in
> a
> > separate CL?
> > 
> > (That'll break all V8 users except blink & chromium though, but so will this
> > CL.)
> 
> 
> I announced in the mailing list that I'd do this.  Plus they've all had lots
of
> warning.

Fair enough. In that case, would it be feasible to do one CL for just removing
the if(n)defs?

[Sven Panne, 2013-09-05 08:24:08]

https://codereview.chromium.org/23401003/diff/1/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/23401003/diff/1/include/v8.h#newcode456
include/v8.h:456: V8_INLINE(Isolate* GetIsolate()) { return isolate_; }
Constify all the getters! ;-)

https://codereview.chromium.org/23401003/diff/1/src/global-handles.h
File src/global-handles.h (right):

https://codereview.chromium.org/23401003/diff/1/src/global-handles.h#newcode148
src/global-handles.h:148: WeakCallback weak_callback,
I'm a bit lost here: Why do we need a 2nd kind of callback (again)?

[dcarney, 2013-09-05 08:32:04]

https://codereview.chromium.org/23401003/diff/1/src/global-handles.h
File src/global-handles.h (right):

https://codereview.chromium.org/23401003/diff/1/src/global-handles.h#newcode148
src/global-handles.h:148: WeakCallback weak_callback,
On 2013/09/05 08:24:09, Sven Panne wrote:
> I'm a bit lost here: Why do we need a 2nd kind of callback (again)?

this handle removes direct access to the underlying persistent, forcing the user
to dispose the handle directly themselves, which has always been an issue

[Sven Panne, 2013-09-05 08:43:01]

LGTM, perhaps wait for a "LGTM" or "don't really care" from Michael.

https://codereview.chromium.org/23401003/diff/1/src/global-handles.h
File src/global-handles.h (right):

https://codereview.chromium.org/23401003/diff/1/src/global-handles.h#newcode148
src/global-handles.h:148: WeakCallback weak_callback,
On 2013/09/05 08:32:05, dcarney wrote:
> On 2013/09/05 08:24:09, Sven Panne wrote:
> > I'm a bit lost here: Why do we need a 2nd kind of callback (again)?
> 
> this handle removes direct access to the underlying persistent, forcing the
user
> to dispose the handle directly themselves, which has always been an issue

Ah, OK. So the revivable stuff is only there for a transitional phase, right?

[dcarney, 2013-09-05 08:43:40]

> Ah, OK. So the revivable stuff is only there for a transitional phase, right?

that's correct

[Michael Starzinger, 2013-09-05 12:57:03]

ACK-LGTM. :)

[marja, 2013-09-09 08:01:37]

Oops, I'm in the reviewer list too. lgtm, though I'm not a V8 committer even :)

[dcarney, 2013-09-09 09:25:36]

Committed patchset #2 manually as r16588 (presubmit successful).