From 81931e726be5c8a1bbf0c422a992a096b0fc4db6 Mon Sep 17 00:00:00 2001 From: peterwmwong Date: Mon, 13 Nov 2017 07:47:15 -0600 Subject: [PATCH] Remove NativeWeakMap Bug: v8:7016 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng Change-Id: I5a509fb91a337eec4a58ab4a13d7104e8ba3ff23 Reviewed-on: https://chromium-review.googlesource.com/760677 Commit-Queue: Jakob Gruber Reviewed-by: Jakob Gruber Reviewed-by: Adam Klein Cr-Commit-Position: refs/heads/master@{#49346} --- include/v8.h | 14 ------ src/api.cc | 95 ------------------------------------ src/api.h | 4 -- src/factory.cc | 9 ---- src/factory.h | 2 - test/cctest/test-api.cc | 80 ------------------------------ test/cctest/test-weakmaps.cc | 5 +- 7 files changed, 4 insertions(+), 205 deletions(-) diff --git a/include/v8.h b/include/v8.h index f0fb659a26..e5f049c3f3 100644 --- a/include/v8.h +++ b/include/v8.h @@ -2087,20 +2087,6 @@ class V8_EXPORT ValueDeserializer { PrivateData* private_; }; -/** - * 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 Data { - public: - static Local New(Isolate* isolate); - void Set(Local key, Local value); - Local Get(Local key) const; - bool Has(Local key); - bool Delete(Local key); -}; - // --- Value --- diff --git a/src/api.cc b/src/api.cc index ea60f18ed7..1213e20838 100644 --- a/src/api.cc +++ b/src/api.cc @@ -3152,101 +3152,6 @@ bool StackFrame::IsConstructor() const { bool StackFrame::IsWasm() const { return Utils::OpenHandle(this)->is_wasm(); } -// --- N a t i v e W e a k M a p --- - -Local NativeWeakMap::New(Isolate* v8_isolate) { - i::Isolate* isolate = reinterpret_cast(v8_isolate); - ENTER_V8_NO_SCRIPT_NO_EXCEPTION(isolate); - i::Handle weakmap = isolate->factory()->NewJSWeakMap(); - i::JSWeakCollection::Initialize(weakmap, isolate); - return Utils::NativeWeakMapToLocal(weakmap); -} - - -void NativeWeakMap::Set(Local v8_key, Local v8_value) { - i::Handle weak_collection = Utils::OpenHandle(this); - i::Isolate* isolate = weak_collection->GetIsolate(); - ENTER_V8_NO_SCRIPT_NO_EXCEPTION(isolate); - i::HandleScope scope(isolate); - i::Handle key = Utils::OpenHandle(*v8_key); - i::Handle value = Utils::OpenHandle(*v8_value); - if (!key->IsJSReceiver() && !key->IsSymbol()) { - DCHECK(false); - return; - } - i::Handle table( - i::ObjectHashTable::cast(weak_collection->table())); - if (!table->IsKey(isolate, *key)) { - DCHECK(false); - return; - } - int32_t hash = key->GetOrCreateHash(isolate)->value(); - i::JSWeakCollection::Set(weak_collection, key, value, hash); -} - -Local NativeWeakMap::Get(Local v8_key) const { - i::Handle weak_collection = Utils::OpenHandle(this); - i::Isolate* isolate = weak_collection->GetIsolate(); - ENTER_V8_NO_SCRIPT_NO_EXCEPTION(isolate); - i::Handle key = Utils::OpenHandle(*v8_key); - if (!key->IsJSReceiver() && !key->IsSymbol()) { - DCHECK(false); - return v8::Undefined(reinterpret_cast(isolate)); - } - i::Handle table( - i::ObjectHashTable::cast(weak_collection->table())); - if (!table->IsKey(isolate, *key)) { - DCHECK(false); - return v8::Undefined(reinterpret_cast(isolate)); - } - i::Handle lookup(table->Lookup(key), isolate); - if (lookup->IsTheHole(isolate)) - return v8::Undefined(reinterpret_cast(isolate)); - return Utils::ToLocal(lookup); -} - - -bool NativeWeakMap::Has(Local v8_key) { - i::Handle weak_collection = Utils::OpenHandle(this); - i::Isolate* isolate = weak_collection->GetIsolate(); - ENTER_V8_NO_SCRIPT_NO_EXCEPTION(isolate); - i::HandleScope scope(isolate); - i::Handle key = Utils::OpenHandle(*v8_key); - if (!key->IsJSReceiver() && !key->IsSymbol()) { - DCHECK(false); - return false; - } - i::Handle table( - i::ObjectHashTable::cast(weak_collection->table())); - if (!table->IsKey(isolate, *key)) { - DCHECK(false); - return false; - } - i::Handle lookup(table->Lookup(key), isolate); - return !lookup->IsTheHole(isolate); -} - - -bool NativeWeakMap::Delete(Local v8_key) { - i::Handle weak_collection = Utils::OpenHandle(this); - i::Isolate* isolate = weak_collection->GetIsolate(); - ENTER_V8_NO_SCRIPT_NO_EXCEPTION(isolate); - i::HandleScope scope(isolate); - i::Handle key = Utils::OpenHandle(*v8_key); - if (!key->IsJSReceiver() && !key->IsSymbol()) { - DCHECK(false); - return false; - } - i::Handle table( - i::ObjectHashTable::cast(weak_collection->table())); - if (!table->IsKey(isolate, *key)) { - DCHECK(false); - return false; - } - int32_t hash = key->GetOrCreateHash(isolate)->value(); - return i::JSWeakCollection::Delete(weak_collection, key, hash); -} - // --- J S O N --- diff --git a/src/api.h b/src/api.h index cde27a4a1e..0a70ac83e4 100644 --- a/src/api.h +++ b/src/api.h @@ -108,7 +108,6 @@ class RegisteredExtension { V(StackTrace, FixedArray) \ V(StackFrame, StackFrameInfo) \ V(Proxy, JSProxy) \ - V(NativeWeakMap, JSWeakMap) \ V(debug::GeneratorObject, JSGeneratorObject) \ V(debug::Script, Script) \ V(Promise, JSPromise) \ @@ -208,8 +207,6 @@ class Utils { v8::internal::Handle obj); static inline Local ExternalToLocal( v8::internal::Handle obj); - static inline Local NativeWeakMapToLocal( - v8::internal::Handle obj); static inline Local CallableToLocal( v8::internal::Handle obj); static inline Local ToLocalPrimitive( @@ -332,7 +329,6 @@ MAKE_TO_LOCAL(NumberToLocal, Object, Number) MAKE_TO_LOCAL(IntegerToLocal, Object, Integer) MAKE_TO_LOCAL(Uint32ToLocal, Object, Uint32) MAKE_TO_LOCAL(ExternalToLocal, JSObject, External) -MAKE_TO_LOCAL(NativeWeakMapToLocal, JSWeakMap, NativeWeakMap) MAKE_TO_LOCAL(CallableToLocal, JSReceiver, Function) MAKE_TO_LOCAL(ToLocalPrimitive, Object, Primitive) MAKE_TO_LOCAL(ToLocal, FixedArray, PrimitiveArray) diff --git a/src/factory.cc b/src/factory.cc index f0a816b9b4..5673eec323 100644 --- a/src/factory.cc +++ b/src/factory.cc @@ -2828,15 +2828,6 @@ Handle Factory::NewArgumentsObject(Handle callee, return result; } - -Handle 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 = NewMap(JS_WEAK_MAP_TYPE, JSWeakMap::kSize); - return Handle::cast(NewJSObjectFromMap(map)); -} - Handle Factory::ObjectLiteralMapFromCache(Handle native_context, int number_of_properties) { DCHECK(native_context->IsNativeContext()); diff --git a/src/factory.h b/src/factory.h index bd7e5b3cb6..1c63f5a49d 100644 --- a/src/factory.h +++ b/src/factory.h @@ -495,8 +495,6 @@ class V8_EXPORT_PRIVATE Factory final { Handle NewBigIntFromSafeInteger( double value, PretenureFlag pretenure = NOT_TENURED); - Handle NewJSWeakMap(); - Handle NewArgumentsObject(Handle callee, int length); // JS objects are pretenured when allocated by the bootstrapper and diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 6fe5d504ee..b9baa30773 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -4868,86 +4868,6 @@ TEST(MessageHandler5) { } -TEST(NativeWeakMap) { - v8::Isolate* isolate = CcTest::isolate(); - HandleScope scope(isolate); - Local weak_map(v8::NativeWeakMap::New(isolate)); - CHECK(!weak_map.IsEmpty()); - - LocalContext env; - Local value = v8::Object::New(isolate); - - Local 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(env.local(), weak_map->Get(local1)).FromJust()); - - WeakCallCounter counter(1234); - WeakCallCounterAndPersistent o1(&counter); - WeakCallCounterAndPersistent o2(&counter); - WeakCallCounterAndPersistent s1(&counter); - { - HandleScope scope(isolate); - Local obj1 = v8::Object::New(isolate); - Local obj2 = v8::Object::New(isolate); - Local 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(env.local(), weak_map->Get(local1)).FromJust()); - CHECK(value->Equals(env.local(), weak_map->Get(obj1)).FromJust()); - CHECK(value->Equals(env.local(), weak_map->Get(obj2)).FromJust()); - CHECK(value->Equals(env.local(), weak_map->Get(sym1)).FromJust()); - } - CcTest::CollectAllGarbage(); - { - HandleScope scope(isolate); - CHECK(value->Equals(env.local(), weak_map->Get(local1)).FromJust()); - CHECK(value->Equals(env.local(), - weak_map->Get(Local::New(isolate, o1.handle))) - .FromJust()); - CHECK(value->Equals(env.local(), - weak_map->Get(Local::New(isolate, o2.handle))) - .FromJust()); - CHECK(value->Equals(env.local(), - weak_map->Get(Local::New(isolate, s1.handle))) - .FromJust()); - } - - o1.handle.SetWeak(&o1, &WeakPointerCallback, - v8::WeakCallbackType::kParameter); - o2.handle.SetWeak(&o2, &WeakPointerCallback, - v8::WeakCallbackType::kParameter); - s1.handle.SetWeak(&s1, &WeakPointerCallback, - v8::WeakCallbackType::kParameter); - - CcTest::CollectAllGarbage(); - CHECK_EQ(3, counter.NumberOfWeakCalls()); - - CHECK(o1.handle.IsEmpty()); - CHECK(o2.handle.IsEmpty()); - CHECK(s1.handle.IsEmpty()); - - CHECK(value->Equals(env.local(), weak_map->Get(local1)).FromJust()); - 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(); diff --git a/test/cctest/test-weakmaps.cc b/test/cctest/test-weakmaps.cc index 616d567f00..8db1855cf5 100644 --- a/test/cctest/test-weakmaps.cc +++ b/test/cctest/test-weakmaps.cc @@ -44,7 +44,10 @@ static Isolate* GetIsolateFrom(LocalContext* context) { static Handle AllocateJSWeakMap(Isolate* isolate) { - Handle weakmap = isolate->factory()->NewJSWeakMap(); + Handle map = + isolate->factory()->NewMap(JS_WEAK_MAP_TYPE, JSWeakMap::kSize); + Handle weakmap_obj = isolate->factory()->NewJSObjectFromMap(map); + Handle weakmap(JSWeakMap::cast(*weakmap_obj)); // Do not leak handles for the hash table, it would make entries strong. { HandleScope scope(isolate);