Revert of Add WeakMap to v8.h (patchset #3 id:40001 of https://codereview.chromium...
authoryurys <yurys@chromium.org>
Tue, 3 Feb 2015 14:42:34 +0000 (06:42 -0800)
committerCommit bot <commit-bot@chromium.org>
Tue, 3 Feb 2015 14:42:49 +0000 (14:42 +0000)
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
src/api.cc
src/factory.cc
src/factory.h
src/runtime/runtime-collections.cc
src/runtime/runtime.h
test/cctest/test-api.cc
test/cctest/test-weakmaps.cc
test/cctest/test-weaksets.cc

index d1c476a..e8ca3df 100644 (file)
@@ -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 ---
 
 
index 0030e09..6caa82a 100644 (file)
@@ -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) {
index c1c818c..8226a0e 100644 (file)
@@ -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,
index 5be5c12..141fc5e 100644 (file)
@@ -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
index ffffbdd..abdd056 100644 (file)
@@ -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
index 3385215..dca28f1 100644 (file)
@@ -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);
 };
 
 
index 0d2c71e..0009253 100644 (file)
@@ -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();
index c3b0c2e..6c19cb8 100644 (file)
@@ -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;
index 60f2d45..299cc92 100644 (file)
@@ -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;