Revision: 21150
Author:   ad...@chromium.org
Date:     Mon May  5 18:27:57 2014 UTC
Log:      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=verwa...@chromium.org

Review URL: https://codereview.chromium.org/254433002
http://code.google.com/p/v8/source/detail?r=21150

Modified:
 /branches/bleeding_edge/src/bootstrapper.cc
 /branches/bleeding_edge/src/factory.cc
 /branches/bleeding_edge/src/objects-inl.h
 /branches/bleeding_edge/src/objects.cc
 /branches/bleeding_edge/src/objects.h
 /branches/bleeding_edge/test/cctest/test-api.cc

=======================================
--- /branches/bleeding_edge/src/bootstrapper.cc Fri May  2 16:13:10 2014 UTC
+++ /branches/bleeding_edge/src/bootstrapper.cc Mon May  5 18:27:57 2014 UTC
@@ -769,16 +769,17 @@
   // 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;
 }


=======================================
--- /branches/bleeding_edge/src/factory.cc      Wed Apr 30 17:27:40 2014 UTC
+++ /branches/bleeding_edge/src/factory.cc      Mon May  5 18:27:57 2014 UTC
@@ -1776,6 +1776,9 @@
   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 @@
   Heap* heap = isolate()->heap();
   // Reinitialize the object from the constructor map.
   heap->InitializeJSObjectFromMap(*object, *properties, *map);
+
+  // Restore the saved hash.
+  object->set_hash(*hash);
 }


=======================================
--- /branches/bleeding_edge/src/objects-inl.h   Mon May  5 14:31:51 2014 UTC
+++ /branches/bleeding_edge/src/objects-inl.h   Mon May  5 18:27:57 2014 UTC
@@ -4921,6 +4921,7 @@
 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)
=======================================
--- /branches/bleeding_edge/src/objects.cc      Mon May  5 09:57:45 2014 UTC
+++ /branches/bleeding_edge/src/objects.cc      Mon May  5 18:27:57 2014 UTC
@@ -5073,9 +5073,7 @@
 }


-Smi* JSReceiver::GenerateIdentityHash() {
-  Isolate* isolate = GetIsolate();
-
+static Smi* GenerateIdentityHash(Isolate* isolate) {
   int hash_value;
   int attempts = 0;
   do {
@@ -5091,14 +5089,31 @@


 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 @@


 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 @@


 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 @@
   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 @@

   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.
=======================================
--- /branches/bleeding_edge/src/objects.h       Mon May  5 14:31:51 2014 UTC
+++ /branches/bleeding_edge/src/objects.h       Mon May  5 18:27:57 2014 UTC
@@ -1989,8 +1989,6 @@
       KeyCollectionType type);

  protected:
-  Smi* GenerateIdentityHash();
-
   MUST_USE_RESULT static MaybeHandle<Object> SetPropertyWithDefinedSetter(
       Handle<JSReceiver> object,
       Handle<JSReceiver> setter,
@@ -7716,6 +7714,9 @@
   // 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 @@

   // 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);
=======================================
--- /branches/bleeding_edge/test/cctest/test-api.cc Mon May 5 16:48:33 2014 UTC +++ /branches/bleeding_edge/test/cctest/test-api.cc Mon May 5 18:27:57 2014 UTC
@@ -2746,6 +2746,25 @@
     CHECK_NE(o1->GetIdentityHash(), o2->GetIdentityHash());
   }
 }
+
+
+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) {

--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to