Revert of Add WeakMap to v8.h (patchset #3 id:40001 of https://codereview.chromium.org/886473005/)

Reason for revert:
Broke compilation on component build http://build.chromium.org/p/client.v8/builders/V8%20Win32%20-%20nosnap%20-%20shared/builds/5007/steps/compile/logs/stdio

Original issue's description:
> Add WeakMap 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/37d4c57630636f21e3add8d3d1c7c978ff5fc8e0
> Cr-Commit-Position: refs/heads/master@{#26401}

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

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

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

include/v8.h

@@ -1536,31 +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 WeakMap {
- public:
-  static WeakMap* New(Isolate* isolate);
-  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:
-  WeakMap(Isolate* isolate, Handle<Object> weak_map)
-      : isolate_(isolate), map_(isolate, weak_map) {}
-  Isolate* isolate_;
-  UniquePersistent<Object> map_;
-
-  // Disallow copying and assigning.
-  WeakMap(WeakMap&);
-  void operator=(WeakMap&);
-};
-
-
 // --- Value ---
 
 

src/api.cc

@@ -2249,112 +2249,6 @@ bool StackFrame::IsConstructor() const {
 }
 
 
-// --- W e a k M a p ---
-
-WeakMap* WeakMap::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 WeakMap(v8_isolate, v8_obj);
-}
-
-
-void WeakMap::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> WeakMap::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 WeakMap::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 WeakMap::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

@@ -2236,15 +2236,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<JSFunction> Factory::CreateApiFunction(
     Handle<FunctionTemplateInfo> obj,
     Handle<Object> prototype,

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

@@ -879,13 +879,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(WeakMap) {
-  v8::Isolate* isolate = CcTest::isolate();
-  i::SmartPointer<v8::WeakMap> weak_map(v8::WeakMap::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;