Add WeakKeyMap to v8.h

Add WeakKeyMap to v8.h

A new map wich references its keys weakly is added to v8.h. Internally it uses the same storage as JSWeakMap but doesn't depend on the JavaScript part of WeakMap implementation in weak-collection.js, hence it can be instantiated without entering any context.

BUG=chromium:437416
LOG=Y

Committed: https://crrev.com/ee7ed39ac8327124e74dd7ad5f1de0dede988cb7
Cr-Commit-Position: refs/heads/master@{#26425}

[yurys, 2015-02-04 08:22:30]

yurys@chromium.org changed reviewers:
+ jochen@chromium.org, mstarzinger@chromium.org, rossberg@chromium.org

[yurys, 2015-02-04 08:22:30]

This patch is slightly modified version of
https://crrev.com/37d4c57630636f21e3add8d3d1c7c978ff5fc8e0 which was reverted
due to component build failure.

The differences are:
1) WeakMap has been renamed to WeakKeyMap to avoid possible confusion with ES6
WeakMap that may appear in the public API later.
2) WeakKeyMap constructor and destructor implementations were moved to api.cc to
avoid requirement to dll-export v8::UniquePersistent<Object> class.

[Michael Starzinger, 2015-02-04 09:08:15]

LGTM on (2), please wait for Andreas about (1).

[rossberg, 2015-02-04 11:58:42]

Michael and I discussed this, and the best we could come up with was
NativeWeakMap, which makes clear that it is not a JS object.

[Michael Starzinger, 2015-02-04 11:59:53]

On 2015/02/04 11:58:42, rossberg wrote:
> Michael and I discussed this, and the best we could come up with was
> NativeWeakMap, which makes clear that it is not a JS object.

I would be fine with using NativeWeakMap.

[yurys, 2015-02-04 12:06:45]

New patchsets have been uploaded after l-g-t-m from mstarzinger@chromium.org

[yurys, 2015-02-04 12:07:18]

On 2015/02/04 11:59:53, Michael Starzinger wrote:
> On 2015/02/04 11:58:42, rossberg wrote:
> > Michael and I discussed this, and the best we could come up with was
> > NativeWeakMap, which makes clear that it is not a JS object.
> 
> I would be fine with using NativeWeakMap.

Done. Renamed it to NativeWeakMap.

[rossberg, 2015-02-04 12:16:46]

lgtm

[yurys, 2015-02-04 12:22:41]

The CQ bit was checked by yurys@chromium.org

[commit-bot: I haz the power, 2015-02-04 12:23:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/891473005/20001

[commit-bot: I haz the power, 2015-02-04 12:52:59]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-02-04 12:53:17]

Patchset 2 (id:??) landed as
https://crrev.com/ee7ed39ac8327124e74dd7ad5f1de0dede988cb7
Cr-Commit-Position: refs/heads/master@{#26425}

[yurys, 2015-02-04 15:12:01]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/901663002/ by yurys@chromium.org.

The reason for reverting is: Revert this patch due to shared win build
compilation failure

http://build.chromium.org/p/client.v8/builders/V8%20Win32%20-%20nosnap%20-%20....

[yurys, 2015-02-04 15:16:54]

The error looks weird to me. C4251 warning disabled in several build
configurations [1], should we do the same for v8? Otherwise I would have to
somehow instantiate v8::UniquePersistent<v8::Object> in v8.h and mark that class
as exported.


[1]
https://code.google.com/p/chromium/codesearch#search/&q=C4251&sq=package:chro...

[dcarney, 2015-02-04 19:15:42]

On 2015/02/04 15:16:54, yurys wrote:
> The error looks weird to me. C4251 warning disabled in several build
> configurations [1], should we do the same for v8? Otherwise I would have to
> somehow instantiate v8::UniquePersistent<v8::Object> in v8.h and mark that
class
> as exported.
> 
> 
> [1]
>
https://code.google.com/p/chromium/codesearch#search/&q=C4251&sq=package:chro...

it's generally a bad idea to embed handles into objects.  It's completely
contrary to of the way the rest of the api is written. in this case, you could
make the class extend Data, since you want to pretend it's not in the js class
hierachy and you wouldn't run into the windows warning.

[yurys, 2015-02-05 08:39:59]

On 2015/02/04 19:15:42, dcarney wrote:
> On 2015/02/04 15:16:54, yurys wrote:
> > The error looks weird to me. C4251 warning disabled in several build
> > configurations [1], should we do the same for v8? Otherwise I would have to
> > somehow instantiate v8::UniquePersistent<v8::Object> in v8.h and mark that
> class
> > as exported.
> > 
> > 
> > [1]
> >
>
https://code.google.com/p/chromium/codesearch#search/&q=C4251&sq=package:chro...
> 
> it's generally a bad idea to embed handles into objects.  It's completely
> contrary to of the way the rest of the api is written. in this case, you could
> make the class extend Data, since you want to pretend it's not in the js class
> hierachy and you wouldn't run into the windows warning.

Thanks for your comments, you are absolutely right. Fixed version is up at
https://codereview.chromium.org/900123003/