Store JSGlobalProxy's identity hash directly on the proxy itself

Store JSGlobalProxy's identity hash directly on the proxy itself

Previously, the hash was stored on the underlying global object, since
it was stored in the hidden property table. This patch moves to an
implementation modeled on JSProxy, adding a new 'hash' field to JSGlobalProxy.

This allows storing the global proxy in a Map, Set, WeakMap, or WeakSet and
accessing it even after the proxy has been attached to a new global, which
is Firefox's current behavior and was the consensus of a recent thread on public-script-coord:
http://lists.w3.org/Archives/Public/public-script-coord/2014AprJun/0012.html

R=verwaest@chromium.org

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

[Toon Verwaest, 2014-04-28 14:32:49]

https://codereview.chromium.org/254433002/diff/20001/src/object-observe.js
File src/object-observe.js (right):

https://codereview.chromium.org/254433002/diff/20001/src/object-observe.js#ne...
src/object-observe.js:83: key = %UnwrapGlobalProxy(key);
I'd strongly prefer merging UnwrapGlobalProxy with WeakCollectionGet/Set in
"WeakCollectionGetForObserve", so we don't have a free-floating runtime function
that unwraps global proxies, but keep that behavior very local to
Object.observe.

https://codereview.chromium.org/254433002/diff/20001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/254433002/diff/20001/src/objects.cc#newcode4995
src/objects.cc:4995: if (hash->IsSmi())
Either put on single line or use { }

https://codereview.chromium.org/254433002/diff/20001/src/objects.cc#newcode5021
src/objects.cc:5021: if (hash->IsSmi())
Same as above

[adamk, 2014-04-28 18:44:50]

https://codereview.chromium.org/254433002/diff/20001/src/object-observe.js
File src/object-observe.js (right):

https://codereview.chromium.org/254433002/diff/20001/src/object-observe.js#ne...
src/object-observe.js:83: key = %UnwrapGlobalProxy(key);
On 2014/04/28 14:32:50, Toon Verwaest wrote:
> I'd strongly prefer merging UnwrapGlobalProxy with WeakCollectionGet/Set in
> "WeakCollectionGetForObserve", so we don't have a free-floating runtime
function
> that unwraps global proxies, but keep that behavior very local to
> Object.observe.

Done, factoring out the logic into WeakCollectionGet and WeakCollectionSet
static functions.

https://codereview.chromium.org/254433002/diff/20001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/254433002/diff/20001/src/objects.cc#newcode4995
src/objects.cc:4995: if (hash->IsSmi())
On 2014/04/28 14:32:50, Toon Verwaest wrote:
> Either put on single line or use { }

Done. This was pre-existing code, fwiw.

https://codereview.chromium.org/254433002/diff/20001/src/objects.cc#newcode5021
src/objects.cc:5021: if (hash->IsSmi())
On 2014/04/28 14:32:50, Toon Verwaest wrote:
> Same as above

Done.

[Toon Verwaest, 2014-04-29 09:45:29]

lgtm, thanks!

[adamk, 2014-05-02 18:16:02]

Now that Object.observe never tries to hash the global proxy, this change goes
back to being simple (basically just reverting the WeakCollectionGetForObserve
stuff).

Landing, since the rest of the patch is unchanged.

[adamk, 2014-05-02 18:16:28]

On 2014/05/02 18:16:02, adamk wrote:
> Now that Object.observe never tries to hash the global proxy, this change goes
> back to being simple (basically just reverting the WeakCollectionGetForObserve
> stuff).
> 
> Landing, since the rest of the patch is unchanged.

Actually, not landing, since we might want to look at security...

[Toon Verwaest, 2014-05-03 05:44:42]

Still LGTM. I don't see a security issue with this patch.

[adamk, 2014-05-05 18:16:08]

On 2014/05/03 05:44:42, Toon Verwaest wrote:
> Still LGTM. I don't see a security issue with this patch.

Good. I didn't either, but thought another pair of eyes was good given last
week...

[adamk, 2014-05-05 18:28:04]

Committed patchset #4 manually as r21150 (presubmit successful).