Migrate V8PerContextData to the Oilpan heap.

Bug: 1013149
Change-Id: I776f91f8fec4623af9735b63060545ab473956e9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2617354
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842225}

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

@@ -17,7 +17,7 @@ ScriptState::ScriptState(v8::Local<v8::Context> context,
     : isolate_(context->GetIsolate()),
       context_(isolate_, context),
       world_(std::move(world)),
-      per_context_data_(std::make_unique<V8PerContextData>(context)),
+      per_context_data_(MakeGarbageCollected<V8PerContextData>(context)),
       reference_from_v8_context_(PERSISTENT_FROM_HERE, this) {
   DCHECK(world_);
   context_.SetWeak(this, &OnV8ContextCollectedCallback);
@@ -34,12 +34,17 @@ ScriptState::~ScriptState() {
   RendererResourceCoordinator::Get()->OnScriptStateDestroyed(this);
 }
 
+void ScriptState::Trace(Visitor* visitor) const {
+  visitor->Trace(per_context_data_);
+}
+
 void ScriptState::DetachGlobalObject() {
   DCHECK(!context_.IsEmpty());
   GetContext()->DetachGlobal();
 }
 
 void ScriptState::DisposePerContextData() {
+  per_context_data_->Dispose();
   per_context_data_ = nullptr;
   InstanceCounters::IncrementCounter(
       InstanceCounters::kDetachedScriptStateCounter);

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

@@ -130,7 +130,7 @@ class PLATFORM_EXPORT ScriptState final : public GarbageCollected<ScriptState> {
               ExecutionContext* execution_context);
   ~ScriptState();
 
-  void Trace(Visitor*) const {}
+  void Trace(Visitor*) const;
 
   static ScriptState* Current(v8::Isolate* isolate) {  // DEPRECATED
     return From(isolate->GetCurrentContext());
@@ -190,7 +190,7 @@ class PLATFORM_EXPORT ScriptState final : public GarbageCollected<ScriptState> {
   }
   void DetachGlobalObject();
 
-  V8PerContextData* PerContextData() const { return per_context_data_.get(); }
+  V8PerContextData* PerContextData() const { return per_context_data_.Get(); }
   void DisposePerContextData();
 
   // This method is expected to be called only from
@@ -217,7 +217,7 @@ class PLATFORM_EXPORT ScriptState final : public GarbageCollected<ScriptState> {
   // So you must explicitly clear the std::unique_ptr by calling
   // disposePerContextData() once you no longer need V8PerContextData.
   // Otherwise, the v8::Context will leak.
-  std::unique_ptr<V8PerContextData> per_context_data_;
+  Member<V8PerContextData> per_context_data_;
 
   // v8::Context has an internal field to this ScriptState* as a raw pointer,
   // which is out of scope of Blink GC, but it must be a strong reference.  We

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

@@ -46,21 +46,15 @@ namespace blink {
 
 namespace {
 
-constexpr char kWrapperBoilerplatesLabel[] =
-    "V8PerContextData::wrapper_boilerplates_";
-constexpr char kConstructorMapLabel[] = "V8PerContextData::constructor_map_";
 constexpr char kContextLabel[] = "V8PerContextData::context_";
 
 }  // namespace
 
 V8PerContextData::V8PerContextData(v8::Local<v8::Context> context)
     : isolate_(context->GetIsolate()),
-      wrapper_boilerplates_(isolate_, kWrapperBoilerplatesLabel),
-      constructor_map_(isolate_, kConstructorMapLabel),
       context_holder_(std::make_unique<gin::ContextHolder>(isolate_)),
       context_(isolate_, context),
-      activity_logger_(nullptr),
-      data_map_(MakeGarbageCollected<DataMap>()) {
+      activity_logger_(nullptr) {
   context_holder_->SetContext(context);
   context_.Get().AnnotateStrongRetainer(kContextLabel);
 
@@ -77,24 +71,47 @@ V8PerContextData::~V8PerContextData() {
   }
 }
 
+void V8PerContextData::Dispose() {
+  // These fields are not traced by the garbage collector and could contain
+  // strong GC roots that prevent `this` from otherwise being collected, so
+  // explicitly break any potential cycles in the ownership graph now.
+  context_holder_ = nullptr;
+  if (!context_.IsEmpty())
+    context_.SetPhantom();
+  if (!private_custom_element_definition_id_.IsEmpty())
+    private_custom_element_definition_id_.SetPhantom();
+}
+
+void V8PerContextData::Trace(Visitor* visitor) const {
+  visitor->Trace(wrapper_boilerplates_);
+  visitor->Trace(constructor_map_);
+  visitor->Trace(data_map_);
+}
+
 V8PerContextData* V8PerContextData::From(v8::Local<v8::Context> context) {
   return ScriptState::From(context)->PerContextData();
 }
 
 v8::Local<v8::Object> V8PerContextData::CreateWrapperFromCacheSlowCase(
     const WrapperTypeInfo* type) {
+  DCHECK(!wrapper_boilerplates_.Contains(type));
   v8::Context::Scope scope(GetContext());
   v8::Local<v8::Function> interface_object = ConstructorForType(type);
   CHECK(!interface_object.IsEmpty());
   v8::Local<v8::Object> instance_template =
       V8ObjectConstructor::NewInstance(isolate_, interface_object)
           .ToLocalChecked();
-  wrapper_boilerplates_.Set(type, instance_template);
+
+  TraceWrapperV8Reference<v8::Object> traced_wrapper;
+  traced_wrapper.Set(isolate_, instance_template);
+  wrapper_boilerplates_.insert(type, traced_wrapper);
+
   return instance_template->Clone();
 }
 
 v8::Local<v8::Function> V8PerContextData::ConstructorForTypeSlowCase(
     const WrapperTypeInfo* type) {
+  DCHECK(!constructor_map_.Contains(type));
   v8::Local<v8::Context> context = GetContext();
   v8::Context::Scope scope(context);
 
@@ -109,7 +126,10 @@ v8::Local<v8::Function> V8PerContextData::ConstructorForTypeSlowCase(
           type, context, world, isolate_, parent_interface_object,
           V8ObjectConstructor::CreationMode::kInstallConditionalFeatures);
 
-  constructor_map_.Set(type, interface_object);
+  TraceWrapperV8Reference<v8::Function> traced_wrapper;
+  traced_wrapper.Set(isolate_, interface_object);
+  constructor_map_.insert(type, traced_wrapper);
+
   return interface_object;
 }
 
@@ -130,26 +150,28 @@ bool V8PerContextData::GetExistingConstructorAndPrototypeForType(
     const WrapperTypeInfo* type,
     v8::Local<v8::Object>* prototype_object,
     v8::Local<v8::Function>* interface_object) {
-  *interface_object = constructor_map_.Get(type);
-  if (interface_object->IsEmpty()) {
-    *prototype_object = v8::Local<v8::Object>();
+  auto it = constructor_map_.find(type);
+  if (it == constructor_map_.end()) {
+    interface_object->Clear();
+    prototype_object->Clear();
     return false;
   }
+  *interface_object = it->value.NewLocal(isolate_);
   *prototype_object = PrototypeForType(type);
   DCHECK(!prototype_object->IsEmpty());
   return true;
 }
 
 void V8PerContextData::AddData(const char* key, Data* data) {
-  data_map_->Set(key, data);
+  data_map_.Set(key, data);
 }
 
 void V8PerContextData::ClearData(const char* key) {
-  data_map_->erase(key);
+  data_map_.erase(key);
 }
 
 V8PerContextData::Data* V8PerContextData::GetData(const char* key) {
-  return data_map_->at(key);
+  return data_map_.at(key);
 }
 
 }  // namespace blink

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

@@ -36,11 +36,11 @@
 #include "gin/public/context_holder.h"
 #include "gin/public/gin_embedders.h"
 #include "third_party/blink/renderer/platform/bindings/scoped_persistent.h"
-#include "third_party/blink/renderer/platform/bindings/v8_global_value_map.h"
+#include "third_party/blink/renderer/platform/bindings/trace_wrapper_v8_reference.h"
 #include "third_party/blink/renderer/platform/heap/handle.h"
+#include "third_party/blink/renderer/platform/heap/heap.h"
 #include "third_party/blink/renderer/platform/heap/persistent.h"
 #include "third_party/blink/renderer/platform/platform_export.h"
-#include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
 #include "third_party/blink/renderer/platform/wtf/hash_map.h"
 #include "third_party/blink/renderer/platform/wtf/text/atomic_string.h"
 #include "third_party/blink/renderer/platform/wtf/text/atomic_string_hash.h"
@@ -55,9 +55,8 @@ struct WrapperTypeInfo;
 
 // Used to hold data that is associated with a single v8::Context object, and
 // has a 1:1 relationship with v8::Context.
-class PLATFORM_EXPORT V8PerContextData final {
-  USING_FAST_MALLOC(V8PerContextData);
-
+class PLATFORM_EXPORT V8PerContextData final
+    : public GarbageCollected<V8PerContextData> {
  public:
   explicit V8PerContextData(v8::Local<v8::Context>);
 
@@ -65,23 +64,27 @@ class PLATFORM_EXPORT V8PerContextData final {
 
   ~V8PerContextData();
 
+  void Trace(Visitor* visitor) const;
+  void Dispose();
+
   v8::Local<v8::Context> GetContext() { return context_.NewLocal(isolate_); }
 
   // To create JS Wrapper objects, we create a cache of a 'boiler plate'
   // object, and then simply Clone that object each time we need a new one.
   // This is faster than going through the full object creation process.
   v8::Local<v8::Object> CreateWrapperFromCache(const WrapperTypeInfo* type) {
-    v8::Local<v8::Object> boilerplate = wrapper_boilerplates_.Get(type);
-    return !boilerplate.IsEmpty() ? boilerplate->Clone()
-                                  : CreateWrapperFromCacheSlowCase(type);
+    auto it = wrapper_boilerplates_.find(type);
+    return it != wrapper_boilerplates_.end()
+               ? it->value.Get()->Clone()
+               : CreateWrapperFromCacheSlowCase(type);
   }
 
   // Returns the interface object that is appropriately initialized (e.g.
   // context-dependent properties are installed).
   v8::Local<v8::Function> ConstructorForType(const WrapperTypeInfo* type) {
-    v8::Local<v8::Function> interface_object = constructor_map_.Get(type);
-    return (!interface_object.IsEmpty()) ? interface_object
-                                         : ConstructorForTypeSlowCase(type);
+    auto it = constructor_map_.find(type);
+    return it != constructor_map_.end() ? it->value.NewLocal(isolate_)
+                                        : ConstructorForTypeSlowCase(type);
   }
 
   v8::Local<v8::Object> PrototypeForType(const WrapperTypeInfo*);
@@ -127,13 +130,15 @@ class PLATFORM_EXPORT V8PerContextData final {
   v8::Local<v8::Object> CreateWrapperFromCacheSlowCase(const WrapperTypeInfo*);
   v8::Local<v8::Function> ConstructorForTypeSlowCase(const WrapperTypeInfo*);
 
-  v8::Isolate* isolate_;
+  v8::Isolate* const isolate_;
 
   // For each possible type of wrapper, we keep a boilerplate object.
   // The boilerplate is used to create additional wrappers of the same type.
-  V8GlobalValueMap<const WrapperTypeInfo*, v8::Object> wrapper_boilerplates_;
+  HeapHashMap<const WrapperTypeInfo*, TraceWrapperV8Reference<v8::Object>>
+      wrapper_boilerplates_;
 
-  V8GlobalValueMap<const WrapperTypeInfo*, v8::Function> constructor_map_;
+  HeapHashMap<const WrapperTypeInfo*, TraceWrapperV8Reference<v8::Function>>
+      constructor_map_;
 
   std::unique_ptr<gin::ContextHolder> context_holder_;
 
@@ -145,7 +150,7 @@ class PLATFORM_EXPORT V8PerContextData final {
   V8DOMActivityLogger* activity_logger_;
 
   using DataMap = HeapHashMap<const char*, Member<Data>>;
-  Persistent<DataMap> data_map_;
+  DataMap data_map_;
 
   DISALLOW_COPY_AND_ASSIGN(V8PerContextData);
 };