Make it possible to add more than one piece of embedder data to isolates

Make it possible to add more than one piece of embedder data to isolates

This will allow for using gin and blink bindings in the same process

BUG=317398
R=svenpanne@chromium.org, dcarney@chromium.org
LOG=y

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

[Sven Panne, 2013-11-20 10:03:02]

LGTM with nits

https://codereview.chromium.org/77913003/diff/1/src/isolate.cc
File src/isolate.cc (right):

https://codereview.chromium.org/77913003/diff/1/src/isolate.cc#newcode1723
src/isolate.cc:1723: embedder_data_(),
Can't we simply remove this line?

https://codereview.chromium.org/77913003/diff/1/src/isolate.h
File src/isolate.h (right):

https://codereview.chromium.org/77913003/diff/1/src/isolate.h#newcode1053
src/isolate.h:1053: void SetData(uint32_t slot, void* data) {
embedder_data_[slot] = data; }
An ASSERT that slot is in range might be nice here and below.

[jochen (gone - plz use gerrit), 2013-11-20 10:59:21]

Committed patchset #1 manually as r17907 (presubmit successful).

[Paweł Hajdan Jr., 2013-12-18 12:49:54]

Drive-by.

https://codereview.chromium.org/77913003/diff/1/include/v8.h
File include/v8.h (left):

https://codereview.chromium.org/77913003/diff/1/include/v8.h#oldcode5544
include/v8.h:5544: V8_INLINE static void SetEmbedderData(v8::Isolate* isolate,
void* data) {
Shouldn't there be V8_DEPRECATED SetEmbedderData(v8::Isolate* isolate, void*
data)?

Looks like this CL removes that signature without deprecating it first.

Is it possible to have a reasonable implementation of this, or does it just have
to be removed without deprecation?

[jochen (gone - plz use gerrit), 2013-12-20 10:15:40]

On 2013/12/18 12:49:54, Paweł Hajdan Jr. wrote:
> Drive-by.
> 
> https://codereview.chromium.org/77913003/diff/1/include/v8.h
> File include/v8.h (left):
> 
> https://codereview.chromium.org/77913003/diff/1/include/v8.h#oldcode5544
> include/v8.h:5544: V8_INLINE static void SetEmbedderData(v8::Isolate* isolate,
> void* data) {
> Shouldn't there be V8_DEPRECATED SetEmbedderData(v8::Isolate* isolate, void*
> data)?
> 
> Looks like this CL removes that signature without deprecating it first.
> 
> Is it possible to have a reasonable implementation of this, or does it just
have
> to be removed without deprecation?

I'm not sure I understand your comment?

[Paweł Hajdan Jr., 2013-12-20 11:28:57]

On 2013/12/20 10:15:40, jochen wrote:
> On 2013/12/18 12:49:54, Paweł Hajdan Jr. wrote:
> > Drive-by.
> > 
> > https://codereview.chromium.org/77913003/diff/1/include/v8.h
> > File include/v8.h (left):
> > 
> > https://codereview.chromium.org/77913003/diff/1/include/v8.h#oldcode5544
> > include/v8.h:5544: V8_INLINE static void SetEmbedderData(v8::Isolate*
isolate,
> > void* data) {
> > Shouldn't there be V8_DEPRECATED SetEmbedderData(v8::Isolate* isolate, void*
> > data)?
> > 
> > Looks like this CL removes that signature without deprecating it first.
> > 
> > Is it possible to have a reasonable implementation of this, or does it just
> have
> > to be removed without deprecation?
> 
> I'm not sure I understand your comment?

I think this CL breaks API in a backward-incompatible way, and doesn't use
V8_DEPRECATED to keep the old interface for at least one release.

[Sven Panne, 2013-12-20 11:44:42]

> I think this CL breaks API in a backward-incompatible way, and doesn't use
> V8_DEPRECATED to keep the old interface for at least one release.

We intentionally violate this policy now because tons of old stuff has been
accumulated by now which really slows our development down, and we want to have
a kind of a clean slate (well, it's not *that* clean, but...). I intend to make
a PSA on v8-users@ about this. Not a perfect solution, but I think it's
bearable.

[Paweł Hajdan Jr., 2013-12-20 12:58:25]

On 2013/12/20 11:44:42, Sven Panne wrote:
> > I think this CL breaks API in a backward-incompatible way, and doesn't use
> > V8_DEPRECATED to keep the old interface for at least one release.
> 
> We intentionally violate this policy now because tons of old stuff has been
> accumulated by now which really slows our development down, and we want to
have
> a kind of a clean slate (well, it's not *that* clean, but...). I intend to
make
> a PSA on v8-users@ about this. Not a perfect solution, but I think it's
> bearable.

This is fine for me, thank you for explaining. Looking forward to the PSA, and
indeed I totally agree with doing a sweeping update once rather than have a
series of breaking changes over a longer time. Better both for V8 and users.