Revert of Add WeakKeyMap to v8.h (patchset #2 id:20001 of https://codereview.chromium.org/891473005/)

Reason for revert:
Revert this patch due to shared win build compilation failure

http://build.chromium.org/p/client.v8/builders/V8%20Win32%20-%20nosnap%20-%20shared/builds/5030/steps/compile/logs/stdio

Original issue's description:
> 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}

TBR=jochen@chromium.org,mstarzinger@chromium.org,rossberg@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:437416

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

Cr-Commit-Position: refs/heads/master@{#26430}

include/v8.h

@@ -1536,32 +1536,6 @@ class V8_EXPORT JSON {
 };
 
 
-/**
- * A map whose keys are referenced weakly. It is similar to JavaScript WeakMap
- * but can be created without entering a v8::Context and hence shouldn't
- * escape to JavaScript.
- */
-class V8_EXPORT NativeWeakMap {
- public:
-  static NativeWeakMap* New(Isolate* isolate);
-  ~NativeWeakMap();
-  void Set(Handle<Value> key, Handle<Value> value);
-  Local<Value> Get(Handle<Value> key);
-  bool Has(Handle<Value> key);
-  bool Delete(Handle<Value> key);
-
- private:
-  NativeWeakMap(Isolate* isolate, Handle<Object> weak_map);
-
-  Isolate* isolate_;
-  UniquePersistent<Object> map_;
-
-  // Disallow copying and assigning.
-  NativeWeakMap(NativeWeakMap&);
-  void operator=(NativeWeakMap&);
-};
-
-
 // --- Value ---
 
 

src/api.cc

@@ -2250,119 +2250,6 @@ bool StackFrame::IsConstructor() const {
 }
 
 
-// --- N a t i v e W e a k M a p ---
-
-NativeWeakMap* NativeWeakMap::New(Isolate* v8_isolate) {
-  i::Isolate* isolate = reinterpret_cast<i::Isolate*>(v8_isolate);
-  ENTER_V8(isolate);
-  i::HandleScope scope(isolate);
-  i::Handle<i::JSWeakMap> weakmap = isolate->factory()->NewJSWeakMap();
-  i::Runtime::WeakCollectionInitialize(isolate, weakmap);
-  Local<Object> v8_obj = Utils::ToLocal(i::Handle<i::JSObject>::cast(weakmap));
-  return new NativeWeakMap(v8_isolate, v8_obj);
-}
-
-
-NativeWeakMap::NativeWeakMap(Isolate* isolate, Handle<Object> weak_map)
-    : isolate_(isolate), map_(isolate, weak_map) {}
-
-
-NativeWeakMap::~NativeWeakMap() {}
-
-
-void NativeWeakMap::Set(Handle<Value> v8_key, Handle<Value> v8_value) {
-  v8::HandleScope handleScope(isolate_);
-  Local<Object> map_handle = Local<Object>::New(isolate_, map_);
-  i::Isolate* isolate = reinterpret_cast<i::Isolate*>(isolate_);
-  ENTER_V8(isolate);
-  i::Handle<i::Object> key = Utils::OpenHandle(*v8_key);
-  i::Handle<i::Object> value = Utils::OpenHandle(*v8_value);
-  i::Handle<i::JSWeakMap> weak_collection =
-      i::Handle<i::JSWeakMap>::cast(Utils::OpenHandle(*map_handle));
-  if (!key->IsJSReceiver() && !key->IsSymbol()) {
-    DCHECK(false);
-    return;
-  }
-  i::Handle<i::ObjectHashTable> table(
-      i::ObjectHashTable::cast(weak_collection->table()));
-  if (!table->IsKey(*key)) {
-    DCHECK(false);
-    return;
-  }
-  i::Runtime::WeakCollectionSet(weak_collection, key, value);
-}
-
-
-Local<Value> NativeWeakMap::Get(Handle<Value> v8_key) {
-  v8::EscapableHandleScope handleScope(isolate_);
-  Local<Object> map_handle = Local<Object>::New(isolate_, map_);
-  i::Isolate* isolate = reinterpret_cast<i::Isolate*>(isolate_);
-  ENTER_V8(isolate);
-  i::Handle<i::Object> key = Utils::OpenHandle(*v8_key);
-  i::Handle<i::JSWeakMap> weak_collection =
-      i::Handle<i::JSWeakMap>::cast(Utils::OpenHandle(*map_handle));
-  if (!key->IsJSReceiver() && !key->IsSymbol()) {
-    DCHECK(false);
-    return Undefined(isolate_);
-  }
-  i::Handle<i::ObjectHashTable> table(
-      i::ObjectHashTable::cast(weak_collection->table()));
-  if (!table->IsKey(*key)) {
-    DCHECK(false);
-    return Undefined(isolate_);
-  }
-  i::Handle<i::Object> lookup(table->Lookup(key), isolate);
-  if (lookup->IsTheHole()) return Undefined(isolate_);
-  Local<Value> result = Utils::ToLocal(lookup);
-  return handleScope.Escape(result);
-}
-
-
-bool NativeWeakMap::Has(Handle<Value> v8_key) {
-  v8::HandleScope handleScope(isolate_);
-  Local<Object> map_handle = Local<Object>::New(isolate_, map_);
-  i::Isolate* isolate = reinterpret_cast<i::Isolate*>(isolate_);
-  ENTER_V8(isolate);
-  i::Handle<i::Object> key = Utils::OpenHandle(*v8_key);
-  i::Handle<i::JSWeakMap> weak_collection =
-      i::Handle<i::JSWeakMap>::cast(Utils::OpenHandle(*map_handle));
-  if (!key->IsJSReceiver() && !key->IsSymbol()) {
-    DCHECK(false);
-    return false;
-  }
-  i::Handle<i::ObjectHashTable> table(
-      i::ObjectHashTable::cast(weak_collection->table()));
-  if (!table->IsKey(*key)) {
-    DCHECK(false);
-    return false;
-  }
-  i::Handle<i::Object> lookup(table->Lookup(key), isolate);
-  return !lookup->IsTheHole();
-}
-
-
-bool NativeWeakMap::Delete(Handle<Value> v8_key) {
-  v8::HandleScope handleScope(isolate_);
-  Local<Object> map_handle = Local<Object>::New(isolate_, map_);
-  i::Isolate* isolate = reinterpret_cast<i::Isolate*>(isolate_);
-  ENTER_V8(isolate);
-  i::Handle<i::Object> key = Utils::OpenHandle(*v8_key);
-  i::Handle<i::JSWeakMap> weak_collection =
-      i::Handle<i::JSWeakMap>::cast(Utils::OpenHandle(*map_handle));
-  if (!key->IsJSReceiver() && !key->IsSymbol()) {
-    DCHECK(false);
-    return false;
-  }
-  i::Handle<i::ObjectHashTable> table(
-      i::ObjectHashTable::cast(weak_collection->table()));
-  if (!table->IsKey(*key)) {
-    DCHECK(false);
-    return false;
-  }
-  return i::Runtime::WeakCollectionDelete(weak_collection, key);
-}
-
-
 // --- J S O N ---
 
 Local<Value> JSON::Parse(Local<String> json_string) {

src/factory.cc

@@ -2237,15 +2237,6 @@ Handle<JSObject> Factory::NewArgumentsObject(Handle<JSFunction> callee,
 }
 
 
-Handle<JSWeakMap> Factory::NewJSWeakMap() {
-  // TODO(adamk): Currently the map is only created three times per
-  // isolate. If it's created more often, the map should be moved into the
-  // strong root list.
-  Handle<Map> map = NewMap(JS_WEAK_MAP_TYPE, JSWeakMap::kSize);
-  return Handle<JSWeakMap>::cast(NewJSObjectFromMap(map));
-}
-
-
 Handle<Map> Factory::ObjectLiteralMapFromCache(Handle<Context> context,
                                                int number_of_properties,
                                                bool* is_result_from_cache) {

src/factory.h

@@ -359,8 +359,6 @@ class Factory FINAL {
     return NewJSObjectFromMap(neander_map());
   }
 
-  Handle<JSWeakMap> NewJSWeakMap();
-
   Handle<JSObject> NewArgumentsObject(Handle<JSFunction> callee, int length);
 
   // JS objects are pretenured when allocated by the bootstrapper and

src/runtime/runtime-collections.cc

@@ -296,11 +296,12 @@ RUNTIME_FUNCTION(Runtime_MapIteratorNext) {
 }
 
 
-void Runtime::WeakCollectionInitialize(
+static Handle<JSWeakCollection> WeakCollectionInitialize(
     Isolate* isolate, Handle<JSWeakCollection> weak_collection) {
   DCHECK(weak_collection->map()->inobject_properties() == 0);
   Handle<ObjectHashTable> table = ObjectHashTable::New(isolate, 0);
   weak_collection->set_table(*table);
+  return weak_collection;
 }
 
 
@@ -308,8 +309,7 @@ RUNTIME_FUNCTION(Runtime_WeakCollectionInitialize) {
   HandleScope scope(isolate);
   DCHECK(args.length() == 1);
   CONVERT_ARG_HANDLE_CHECKED(JSWeakCollection, weak_collection, 0);
-  Runtime::WeakCollectionInitialize(isolate, weak_collection);
-  return *weak_collection;
+  return *WeakCollectionInitialize(isolate, weak_collection);
 }
 
 
@@ -341,24 +341,6 @@ RUNTIME_FUNCTION(Runtime_WeakCollectionHas) {
 }
 
 
-bool Runtime::WeakCollectionDelete(Handle<JSWeakCollection> weak_collection,
-                                   Handle<Object> key) {
-  DCHECK(key->IsJSReceiver() || key->IsSymbol());
-  Handle<ObjectHashTable> table(
-      ObjectHashTable::cast(weak_collection->table()));
-  DCHECK(table->IsKey(*key));
-  bool was_present = false;
-  Handle<ObjectHashTable> new_table =
-      ObjectHashTable::Remove(table, key, &was_present);
-  weak_collection->set_table(*new_table);
-  if (*table != *new_table) {
-    // Zap the old table since we didn't record slots for its elements.
-    table->FillWithHoles(0, table->length());
-  }
-  return was_present;
-}
-
-
 RUNTIME_FUNCTION(Runtime_WeakCollectionDelete) {
   HandleScope scope(isolate);
   DCHECK(args.length() == 2);
@@ -368,23 +350,15 @@ RUNTIME_FUNCTION(Runtime_WeakCollectionDelete) {
   Handle<ObjectHashTable> table(
       ObjectHashTable::cast(weak_collection->table()));
   RUNTIME_ASSERT(table->IsKey(*key));
-  bool was_present = Runtime::WeakCollectionDelete(weak_collection, key);
-  return isolate->heap()->ToBoolean(was_present);
-}
-
-
-void Runtime::WeakCollectionSet(Handle<JSWeakCollection> weak_collection,
-                                Handle<Object> key, Handle<Object> value) {
-  DCHECK(key->IsJSReceiver() || key->IsSymbol());
-  Handle<ObjectHashTable> table(
-      ObjectHashTable::cast(weak_collection->table()));
-  DCHECK(table->IsKey(*key));
-  Handle<ObjectHashTable> new_table = ObjectHashTable::Put(table, key, value);
+  bool was_present = false;
+  Handle<ObjectHashTable> new_table =
+      ObjectHashTable::Remove(table, key, &was_present);
   weak_collection->set_table(*new_table);
   if (*table != *new_table) {
     // Zap the old table since we didn't record slots for its elements.
     table->FillWithHoles(0, table->length());
   }
+  return isolate->heap()->ToBoolean(was_present);
 }
 
 
@@ -398,7 +372,12 @@ RUNTIME_FUNCTION(Runtime_WeakCollectionSet) {
   Handle<ObjectHashTable> table(
       ObjectHashTable::cast(weak_collection->table()));
   RUNTIME_ASSERT(table->IsKey(*key));
-  Runtime::WeakCollectionSet(weak_collection, key, value);
+  Handle<ObjectHashTable> new_table = ObjectHashTable::Put(table, key, value);
+  weak_collection->set_table(*new_table);
+  if (*table != *new_table) {
+    // Zap the old table since we didn't record slots for its elements.
+    table->FillWithHoles(0, table->length());
+  }
   return *weak_collection;
 }
 
@@ -435,9 +414,14 @@ RUNTIME_FUNCTION(Runtime_GetWeakSetValues) {
 RUNTIME_FUNCTION(Runtime_ObservationWeakMapCreate) {
   HandleScope scope(isolate);
   DCHECK(args.length() == 0);
-  Handle<JSWeakMap> weakmap = isolate->factory()->NewJSWeakMap();
-  Runtime::WeakCollectionInitialize(isolate, weakmap);
-  return *weakmap;
+  // TODO(adamk): Currently this runtime function is only called three times per
+  // isolate. If it's called more often, the map should be moved into the
+  // strong root list.
+  Handle<Map> map =
+      isolate->factory()->NewMap(JS_WEAK_MAP_TYPE, JSWeakMap::kSize);
+  Handle<JSWeakMap> weakmap =
+      Handle<JSWeakMap>::cast(isolate->factory()->NewJSObjectFromMap(map));
+  return *WeakCollectionInitialize(isolate, weakmap);
 }
 }
 }  // namespace v8::internal

src/runtime/runtime.h

@@ -874,13 +874,6 @@ class Runtime : public AllStatic {
   MUST_USE_RESULT static MaybeHandle<Object> CreateArrayLiteralBoilerplate(
       Isolate* isolate, Handle<FixedArray> literals,
       Handle<FixedArray> elements);
-
-  static void WeakCollectionInitialize(
-      Isolate* isolate, Handle<JSWeakCollection> weak_collection);
-  static void WeakCollectionSet(Handle<JSWeakCollection> weak_collection,
-                                Handle<Object> key, Handle<Object> value);
-  static bool WeakCollectionDelete(Handle<JSWeakCollection> weak_collection,
-                                   Handle<Object> key);
 };
 
 

test/cctest/test-api.cc

@@ -46,7 +46,6 @@
 #include "src/isolate.h"
 #include "src/objects.h"
 #include "src/parser.h"
-#include "src/smart-pointers.h"
 #include "src/snapshot.h"
 #include "src/unicode-inl.h"
 #include "src/utils.h"
@@ -4738,78 +4737,6 @@ TEST(MessageHandler5) {
 }
 
 
-TEST(NativeWeakMap) {
-  v8::Isolate* isolate = CcTest::isolate();
-  i::SmartPointer<v8::NativeWeakMap> weak_map(v8::NativeWeakMap::New(isolate));
-  CHECK(!weak_map.is_empty());
-
-  LocalContext env;
-  HandleScope scope(isolate);
-
-  Local<Object> value = v8::Object::New(isolate);
-
-  Local<Object> local1 = v8::Object::New(isolate);
-  CHECK(!weak_map->Has(local1));
-  CHECK(weak_map->Get(local1)->IsUndefined());
-  weak_map->Set(local1, value);
-  CHECK(weak_map->Has(local1));
-  CHECK(value->Equals(weak_map->Get(local1)));
-
-  WeakCallCounter counter(1234);
-  WeakCallCounterAndPersistent<Value> o1(&counter);
-  WeakCallCounterAndPersistent<Value> o2(&counter);
-  WeakCallCounterAndPersistent<Value> s1(&counter);
-  {
-    HandleScope scope(isolate);
-    Local<v8::Object> obj1 = v8::Object::New(isolate);
-    Local<v8::Object> obj2 = v8::Object::New(isolate);
-    Local<v8::Symbol> sym1 = v8::Symbol::New(isolate);
-
-    weak_map->Set(obj1, value);
-    weak_map->Set(obj2, value);
-    weak_map->Set(sym1, value);
-
-    o1.handle.Reset(isolate, obj1);
-    o2.handle.Reset(isolate, obj2);
-    s1.handle.Reset(isolate, sym1);
-
-    CHECK(weak_map->Has(local1));
-    CHECK(weak_map->Has(obj1));
-    CHECK(weak_map->Has(obj2));
-    CHECK(weak_map->Has(sym1));
-
-    CHECK(value->Equals(weak_map->Get(local1)));
-    CHECK(value->Equals(weak_map->Get(obj1)));
-    CHECK(value->Equals(weak_map->Get(obj2)));
-    CHECK(value->Equals(weak_map->Get(sym1)));
-  }
-  CcTest::heap()->CollectAllGarbage(TestHeap::Heap::kNoGCFlags);
-  {
-    HandleScope scope(isolate);
-    CHECK(value->Equals(weak_map->Get(local1)));
-    CHECK(value->Equals(weak_map->Get(Local<Value>::New(isolate, o1.handle))));
-    CHECK(value->Equals(weak_map->Get(Local<Value>::New(isolate, o2.handle))));
-    CHECK(value->Equals(weak_map->Get(Local<Value>::New(isolate, s1.handle))));
-  }
-
-  o1.handle.SetWeak(&o1, &WeakPointerCallback);
-  o2.handle.SetWeak(&o2, &WeakPointerCallback);
-  s1.handle.SetWeak(&s1, &WeakPointerCallback);
-
-  CcTest::heap()->CollectAllGarbage(TestHeap::Heap::kNoGCFlags);
-  CHECK_EQ(3, counter.NumberOfWeakCalls());
-
-  CHECK(o1.handle.IsEmpty());
-  CHECK(o2.handle.IsEmpty());
-  CHECK(s1.handle.IsEmpty());
-
-  CHECK(value->Equals(weak_map->Get(local1)));
-  CHECK(weak_map->Delete(local1));
-  CHECK(!weak_map->Has(local1));
-  CHECK(weak_map->Get(local1)->IsUndefined());
-}
-
-
 THREADED_TEST(GetSetProperty) {
   LocalContext context;
   v8::Isolate* isolate = context->GetIsolate();

test/cctest/test-weakmaps.cc

@@ -42,7 +42,10 @@ static Isolate* GetIsolateFrom(LocalContext* context) {
 
 
 static Handle<JSWeakMap> AllocateJSWeakMap(Isolate* isolate) {
-  Handle<JSWeakMap> weakmap = isolate->factory()->NewJSWeakMap();
+  Factory* factory = isolate->factory();
+  Handle<Map> map = factory->NewMap(JS_WEAK_MAP_TYPE, JSWeakMap::kSize);
+  Handle<JSObject> weakmap_obj = factory->NewJSObjectFromMap(map);
+  Handle<JSWeakMap> weakmap(JSWeakMap::cast(*weakmap_obj));
   // Do not leak handles for the hash table, it would make entries strong.
   {
     HandleScope scope(isolate);
@@ -55,7 +58,11 @@ static Handle<JSWeakMap> AllocateJSWeakMap(Isolate* isolate) {
 static void PutIntoWeakMap(Handle<JSWeakMap> weakmap,
                            Handle<JSObject> key,
                            Handle<Object> value) {
-  Runtime::WeakCollectionSet(weakmap, key, value);
+  Handle<ObjectHashTable> table = ObjectHashTable::Put(
+      Handle<ObjectHashTable>(ObjectHashTable::cast(weakmap->table())),
+      Handle<JSObject>(JSObject::cast(*key)),
+      value);
+  weakmap->set_table(*table);
 }
 
 static int NumberOfWeakCalls = 0;

test/cctest/test-weaksets.cc

@@ -58,7 +58,11 @@ static Handle<JSWeakSet> AllocateJSWeakSet(Isolate* isolate) {
 static void PutIntoWeakSet(Handle<JSWeakSet> weakset,
                            Handle<JSObject> key,
                            Handle<Object> value) {
-  Runtime::WeakCollectionSet(weakset, key, value);
+  Handle<ObjectHashTable> table = ObjectHashTable::Put(
+      Handle<ObjectHashTable>(ObjectHashTable::cast(weakset->table())),
+      Handle<JSObject>(JSObject::cast(*key)),
+      value);
+  weakset->set_table(*table);
 }
 
 static int NumberOfWeakCalls = 0;