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

Review URL: https://codereview.chromium.org/254433002

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@21150 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc
index b158715..4bb0f4d 100644 (file)
--- a/src/bootstrapper.cc
+++ b/src/bootstrapper.cc
@@ -769,16 +769,17 @@ Handle<JSGlobalProxy> Genesis::CreateNewGlobals(
   // Set global_proxy.__proto__ to js_global after ConfigureGlobalObjects
   // Return the global proxy.
 
+  Handle<JSGlobalProxy> global_proxy;
   if (global_object.location() != NULL) {
     ASSERT(global_object->IsJSGlobalProxy());
-    Handle<JSGlobalProxy> global_proxy =
-        Handle<JSGlobalProxy>::cast(global_object);
+    global_proxy = Handle<JSGlobalProxy>::cast(global_object);
     factory()->ReinitializeJSGlobalProxy(global_proxy, global_proxy_function);
-    return global_proxy;
   } else {
-    return Handle<JSGlobalProxy>::cast(
+    global_proxy = Handle<JSGlobalProxy>::cast(
         factory()->NewJSObject(global_proxy_function, TENURED));
+    global_proxy->set_hash(heap()->undefined_value());
   }
+  return global_proxy;
 }
 
 

diff --git a/src/factory.cc b/src/factory.cc
index 864cf67..38243e7 100644 (file)
--- a/src/factory.cc
+++ b/src/factory.cc
@@ -1776,6 +1776,9 @@ void Factory::ReinitializeJSGlobalProxy(Handle<JSGlobalProxy> object,
   ASSERT(constructor->has_initial_map());
   Handle<Map> map(constructor->initial_map(), isolate());
 
+  // The proxy's hash should be retained across reinitialization.
+  Handle<Object> hash(object->hash(), isolate());
+
   // Check that the already allocated object has the same size and type as
   // objects allocated using the constructor.
   ASSERT(map->instance_size() == object->map()->instance_size());
@@ -1795,6 +1798,9 @@ void Factory::ReinitializeJSGlobalProxy(Handle<JSGlobalProxy> object,
   Heap* heap = isolate()->heap();
   // Reinitialize the object from the constructor map.
   heap->InitializeJSObjectFromMap(*object, *properties, *map);
+
+  // Restore the saved hash.
+  object->set_hash(*hash);
 }
 
 

diff --git a/src/objects-inl.h b/src/objects-inl.h
index e4ccd75..029e80d 100644 (file)
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -4921,6 +4921,7 @@ ACCESSORS(GlobalObject, global_context, Context, kGlobalContextOffset)
 ACCESSORS(GlobalObject, global_receiver, JSObject, kGlobalReceiverOffset)
 
 ACCESSORS(JSGlobalProxy, native_context, Object, kNativeContextOffset)
+ACCESSORS(JSGlobalProxy, hash, Object, kHashOffset)
 
 ACCESSORS(AccessorInfo, name, Object, kNameOffset)
 ACCESSORS_TO_SMI(AccessorInfo, flag, kFlagOffset)

diff --git a/src/objects.cc b/src/objects.cc
index 178b5fd..87853f3 100644 (file)
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -5073,9 +5073,7 @@ Handle<SeededNumberDictionary> JSObject::NormalizeElements(
 }
 
 
-Smi* JSReceiver::GenerateIdentityHash() {
-  Isolate* isolate = GetIsolate();
-
+static Smi* GenerateIdentityHash(Isolate* isolate) {
   int hash_value;
   int attempts = 0;
   do {
@@ -5091,14 +5089,31 @@ Smi* JSReceiver::GenerateIdentityHash() {
 
 
 void JSObject::SetIdentityHash(Handle<JSObject> object, Handle<Smi> hash) {
+  ASSERT(!object->IsJSGlobalProxy());
   Isolate* isolate = object->GetIsolate();
   SetHiddenProperty(object, isolate->factory()->identity_hash_string(), hash);
 }
 
 
+template<typename ProxyType>
+static Handle<Object> GetOrCreateIdentityHashHelper(Handle<ProxyType> proxy) {
+  Isolate* isolate = proxy->GetIsolate();
+
+  Handle<Object> hash(proxy->hash(), isolate);
+  if (hash->IsSmi()) return hash;
+
+  hash = handle(GenerateIdentityHash(isolate), isolate);
+  proxy->set_hash(*hash);
+  return hash;
+}
+
+
 Object* JSObject::GetIdentityHash() {
   DisallowHeapAllocation no_gc;
   Isolate* isolate = GetIsolate();
+  if (IsJSGlobalProxy()) {
+    return JSGlobalProxy::cast(this)->hash();
+  }
   Object* stored_value =
       GetHiddenProperty(isolate->factory()->identity_hash_string());
   return stored_value->IsSmi()
@@ -5108,21 +5123,17 @@ Object* JSObject::GetIdentityHash() {
 
 
 Handle<Object> JSObject::GetOrCreateIdentityHash(Handle<JSObject> object) {
-  Handle<Object> hash(object->GetIdentityHash(), object->GetIsolate());
-  if (hash->IsSmi())
-    return hash;
+  if (object->IsJSGlobalProxy()) {
+    return GetOrCreateIdentityHashHelper(Handle<JSGlobalProxy>::cast(object));
+  }
 
   Isolate* isolate = object->GetIsolate();
 
-  hash = handle(object->GenerateIdentityHash(), isolate);
-  Handle<Object> result = SetHiddenProperty(object,
-      isolate->factory()->identity_hash_string(), hash);
-
-  if (result->IsUndefined()) {
-    // Trying to get hash of detached proxy.
-    return handle(Smi::FromInt(0), isolate);
-  }
+  Handle<Object> hash(object->GetIdentityHash(), isolate);
+  if (hash->IsSmi()) return hash;
 
+  hash = handle(GenerateIdentityHash(isolate), isolate);
+  SetHiddenProperty(object, isolate->factory()->identity_hash_string(), hash);
   return hash;
 }
 
@@ -5133,15 +5144,7 @@ Object* JSProxy::GetIdentityHash() {
 
 
 Handle<Object> JSProxy::GetOrCreateIdentityHash(Handle<JSProxy> proxy) {
-  Isolate* isolate = proxy->GetIsolate();
-
-  Handle<Object> hash(proxy->GetIdentityHash(), isolate);
-  if (hash->IsSmi())
-    return hash;
-
-  hash = handle(proxy->GenerateIdentityHash(), isolate);
-  proxy->set_hash(*hash);
-  return hash;
+  return GetOrCreateIdentityHashHelper(proxy);
 }
 
 
@@ -5149,6 +5152,8 @@ Object* JSObject::GetHiddenProperty(Handle<Name> key) {
   DisallowHeapAllocation no_gc;
   ASSERT(key->IsUniqueName());
   if (IsJSGlobalProxy()) {
+    // JSGlobalProxies store their hash internally.
+    ASSERT(*key != GetHeap()->identity_hash_string());
     // For a proxy, use the prototype as target object.
     Object* proxy_parent = GetPrototype();
     // If the proxy is detached, return undefined.
@@ -5183,6 +5188,8 @@ Handle<Object> JSObject::SetHiddenProperty(Handle<JSObject> object,
 
   ASSERT(key->IsUniqueName());
   if (object->IsJSGlobalProxy()) {
+    // JSGlobalProxies store their hash internally.
+    ASSERT(*key != *isolate->factory()->identity_hash_string());
     // For a proxy, use the prototype as target object.
     Handle<Object> proxy_parent(object->GetPrototype(), isolate);
     // If the proxy is detached, return undefined.

diff --git a/src/objects.h b/src/objects.h
index d607b04..0fd3dae 100644 (file)
--- a/src/objects.h
+++ b/src/objects.h
@@ -1989,8 +1989,6 @@ class JSReceiver: public HeapObject {
       KeyCollectionType type);
 
  protected:
-  Smi* GenerateIdentityHash();
-
   MUST_USE_RESULT static MaybeHandle<Object> SetPropertyWithDefinedSetter(
       Handle<JSReceiver> object,
       Handle<JSReceiver> setter,
@@ -7716,6 +7714,9 @@ class JSGlobalProxy : public JSObject {
   // It is null value if this object is not used by any context.
   DECL_ACCESSORS(native_context, Object)
 
+  // [hash]: The hash code property (undefined if not initialized yet).
+  DECL_ACCESSORS(hash, Object)
+
   // Casting.
   static inline JSGlobalProxy* cast(Object* obj);
 
@@ -7727,7 +7728,8 @@ class JSGlobalProxy : public JSObject {
 
   // Layout description.
   static const int kNativeContextOffset = JSObject::kHeaderSize;
-  static const int kSize = kNativeContextOffset + kPointerSize;
+  static const int kHashOffset = kNativeContextOffset + kPointerSize;
+  static const int kSize = kHashOffset + kPointerSize;
 
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(JSGlobalProxy);

diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index 4dad1ee..df70d5c 100644 (file)
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -2748,6 +2748,25 @@ THREADED_TEST(IdentityHash) {
 }
 
 
+THREADED_TEST(GlobalProxyIdentityHash) {
+  LocalContext env;
+  v8::Isolate* isolate = env->GetIsolate();
+  v8::HandleScope scope(isolate);
+  Handle<Object> global_proxy = env->Global();
+  int hash1 = global_proxy->GetIdentityHash();
+  // Hash should be retained after being detached.
+  env->DetachGlobal();
+  int hash2 = global_proxy->GetIdentityHash();
+  CHECK_EQ(hash1, hash2);
+  {
+    // Re-attach global proxy to a new context, hash should stay the same.
+    LocalContext env2(NULL, Handle<ObjectTemplate>(), global_proxy);
+    int hash3 = global_proxy->GetIdentityHash();
+    CHECK_EQ(hash1, hash3);
+  }
+}
+
+
 THREADED_TEST(SymbolProperties) {
   i::FLAG_harmony_symbols = true;