Remove DOMDataStore::WrappedReference and hold TraceWrapperV8Reference directly in ephemeron map.

Change-Id: I6ef8b0098a795dc550037306e7b972ea907b0fc1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1884285Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710616}

third_party/blink/renderer/platform/bindings/dom_data_store.cc

@@ -10,20 +10,16 @@ DOMDataStore::DOMDataStore(v8::Isolate* isolate, bool is_main_world)
     : is_main_world_(is_main_world) {}
 
 void DOMDataStore::Dispose() {
-  for (const auto& it : wrapper_map_) {
+  for (auto& it : wrapper_map_) {
     // Explicitly reset references so that a following V8 GC will not find them
     // and treat them as roots. There's optimizations (see
     // EmbedderHeapTracer::IsRootForNonTracingGC) that would not treat them as
     // roots and then Blink would not be able to find and remove them from a DOM
     // world. Explicitly resetting on disposal avoids that problem
-    it.value->ref.Clear();
+    it.value.Clear();
   }
 }
 
-void DOMDataStore::WrappedReference::Trace(Visitor* visitor) {
-  visitor->Trace(ref);
-}
-
 void DOMDataStore::Trace(Visitor* visitor) {
   visitor->Trace(wrapper_map_);
 }

third_party/blink/renderer/platform/bindings/dom_data_store.h

@@ -35,6 +35,7 @@
 #include "base/optional.h"
 #include "third_party/blink/renderer/platform/bindings/dom_wrapper_world.h"
 #include "third_party/blink/renderer/platform/bindings/script_wrappable.h"
+#include "third_party/blink/renderer/platform/bindings/trace_wrapper_v8_reference.h"
 #include "third_party/blink/renderer/platform/bindings/wrapper_type_info.h"
 #include "third_party/blink/renderer/platform/heap/unified_heap_marking_visitor.h"
 #include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
@@ -118,7 +119,7 @@ class DOMDataStore final : public GarbageCollected<DOMDataStore> {
       return object->MainWorldWrapper(isolate);
     auto it = wrapper_map_.find(object);
     if (it != wrapper_map_.end())
-      return it->value->ref.NewLocal(isolate);
+      return it->value.NewLocal(isolate);
     return v8::Local<v8::Object>();
   }
 
@@ -132,13 +133,12 @@ class DOMDataStore final : public GarbageCollected<DOMDataStore> {
       return object->SetWrapper(isolate, wrapper_type_info, wrapper);
 
     auto result = wrapper_map_.insert(
-        object, MakeGarbageCollected<WrappedReference>(isolate, wrapper));
+        object, TraceWrapperV8Reference<v8::Object>(isolate, wrapper));
     if (LIKELY(result.is_new_entry)) {
-      wrapper_type_info->ConfigureWrapper(
-          &result.stored_value->value->ref.Get());
+      wrapper_type_info->ConfigureWrapper(&result.stored_value->value.Get());
     } else {
-      DCHECK(!result.stored_value->value->ref.IsEmpty());
-      wrapper = result.stored_value->value->ref.NewLocal(isolate);
+      DCHECK(!result.stored_value->value.IsEmpty());
+      wrapper = result.stored_value->value.NewLocal(isolate);
     }
     return result.is_new_entry;
   }
@@ -149,8 +149,8 @@ class DOMDataStore final : public GarbageCollected<DOMDataStore> {
     DCHECK(!is_main_world_);
     const auto& it = wrapper_map_.find(object);
     if (it != wrapper_map_.end()) {
-      if (it->value->ref.Get() == handle) {
-        it->value->ref.Clear();
+      if (it->value.Get() == handle) {
+        it->value.Clear();
         wrapper_map_.erase(it);
         return true;
       }
@@ -164,7 +164,7 @@ class DOMDataStore final : public GarbageCollected<DOMDataStore> {
       return object->SetReturnValue(return_value);
     auto it = wrapper_map_.find(object);
     if (it != wrapper_map_.end()) {
-      return_value.Set(it->value->ref.Get());
+      return_value.Set(it->value.Get());
       return true;
     }
     return false;
@@ -198,24 +198,10 @@ class DOMDataStore final : public GarbageCollected<DOMDataStore> {
     return wrappable->IsEqualTo(holder);
   }
 
-  // Wrapper around TraceWrapperV8Reference to allow use in ephemeron hash map
-  // below.
-  class PLATFORM_EXPORT WrappedReference final
-      : public GarbageCollected<WrappedReference> {
-   public:
-    WrappedReference() = default;
-    WrappedReference(v8::Isolate* isolate, v8::Local<v8::Object> handle)
-        : ref(isolate, handle) {}
-
-    virtual void Trace(Visitor*);
-
-    TraceWrapperV8Reference<v8::Object> ref;
-  };
-
   bool is_main_world_;
-  // Ephemeron map: WrappedReference will be kept alive as long as
-  // ScriptWrappable is alive.
-  HeapHashMap<WeakMember<const ScriptWrappable>, Member<WrappedReference>>
+  // Ephemeron map: V8 wrapper will be kept alive as long as ScriptWrappable is.
+  HeapHashMap<WeakMember<const ScriptWrappable>,
+              TraceWrapperV8Reference<v8::Object>>
       wrapper_map_;
 
   DISALLOW_COPY_AND_ASSIGN(DOMDataStore);