Commit bdc5aa8e authored by Ulan Degenbaev's avatar Ulan Degenbaev Committed by Commit Bot

Revert "Remove DOMDataStore::WrappedReference and hold TraceWrapperV8Reference...

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

This reverts commit 481fb0c1.

Reason for revert: speculative revert for crbug.com/1019839

Original change's description:
> Remove DOMDataStore::WrappedReference and hold TraceWrapperV8Reference directly in ephemeron map.
> 
> Change-Id: I6ef8b0098a795dc550037306e7b972ea907b0fc1
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1884285
> Reviewed-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}

TBR=jbroman@chromium.org,haraken@chromium.org,mlippautz@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Change-Id: Iba339b519f2aa602773ceb79c95948c1cffdcbdb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1893282Reviewed-by: default avatarUlan Degenbaev <ulan@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#711289}
parent 1718b889
...@@ -10,16 +10,20 @@ DOMDataStore::DOMDataStore(v8::Isolate* isolate, bool is_main_world) ...@@ -10,16 +10,20 @@ DOMDataStore::DOMDataStore(v8::Isolate* isolate, bool is_main_world)
: is_main_world_(is_main_world) {} : is_main_world_(is_main_world) {}
void DOMDataStore::Dispose() { void DOMDataStore::Dispose() {
for (auto& it : wrapper_map_) { for (const auto& it : wrapper_map_) {
// Explicitly reset references so that a following V8 GC will not find them // Explicitly reset references so that a following V8 GC will not find them
// and treat them as roots. There's optimizations (see // and treat them as roots. There's optimizations (see
// EmbedderHeapTracer::IsRootForNonTracingGC) that would not treat them as // EmbedderHeapTracer::IsRootForNonTracingGC) that would not treat them as
// roots and then Blink would not be able to find and remove them from a DOM // roots and then Blink would not be able to find and remove them from a DOM
// world. Explicitly resetting on disposal avoids that problem // world. Explicitly resetting on disposal avoids that problem
it.value.Clear(); it.value->ref.Clear();
} }
} }
void DOMDataStore::WrappedReference::Trace(Visitor* visitor) {
visitor->Trace(ref);
}
void DOMDataStore::Trace(Visitor* visitor) { void DOMDataStore::Trace(Visitor* visitor) {
visitor->Trace(wrapper_map_); visitor->Trace(wrapper_map_);
} }
......
...@@ -35,7 +35,6 @@ ...@@ -35,7 +35,6 @@
#include "base/optional.h" #include "base/optional.h"
#include "third_party/blink/renderer/platform/bindings/dom_wrapper_world.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/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/bindings/wrapper_type_info.h"
#include "third_party/blink/renderer/platform/heap/unified_heap_marking_visitor.h" #include "third_party/blink/renderer/platform/heap/unified_heap_marking_visitor.h"
#include "third_party/blink/renderer/platform/wtf/allocator/allocator.h" #include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
...@@ -119,7 +118,7 @@ class DOMDataStore final : public GarbageCollected<DOMDataStore> { ...@@ -119,7 +118,7 @@ class DOMDataStore final : public GarbageCollected<DOMDataStore> {
return object->MainWorldWrapper(isolate); return object->MainWorldWrapper(isolate);
auto it = wrapper_map_.find(object); auto it = wrapper_map_.find(object);
if (it != wrapper_map_.end()) if (it != wrapper_map_.end())
return it->value.NewLocal(isolate); return it->value->ref.NewLocal(isolate);
return v8::Local<v8::Object>(); return v8::Local<v8::Object>();
} }
...@@ -133,12 +132,13 @@ class DOMDataStore final : public GarbageCollected<DOMDataStore> { ...@@ -133,12 +132,13 @@ class DOMDataStore final : public GarbageCollected<DOMDataStore> {
return object->SetWrapper(isolate, wrapper_type_info, wrapper); return object->SetWrapper(isolate, wrapper_type_info, wrapper);
auto result = wrapper_map_.insert( auto result = wrapper_map_.insert(
object, TraceWrapperV8Reference<v8::Object>(isolate, wrapper)); object, MakeGarbageCollected<WrappedReference>(isolate, wrapper));
if (LIKELY(result.is_new_entry)) { if (LIKELY(result.is_new_entry)) {
wrapper_type_info->ConfigureWrapper(&result.stored_value->value.Get()); wrapper_type_info->ConfigureWrapper(
&result.stored_value->value->ref.Get());
} else { } else {
DCHECK(!result.stored_value->value.IsEmpty()); DCHECK(!result.stored_value->value->ref.IsEmpty());
wrapper = result.stored_value->value.NewLocal(isolate); wrapper = result.stored_value->value->ref.NewLocal(isolate);
} }
return result.is_new_entry; return result.is_new_entry;
} }
...@@ -149,8 +149,8 @@ class DOMDataStore final : public GarbageCollected<DOMDataStore> { ...@@ -149,8 +149,8 @@ class DOMDataStore final : public GarbageCollected<DOMDataStore> {
DCHECK(!is_main_world_); DCHECK(!is_main_world_);
const auto& it = wrapper_map_.find(object); const auto& it = wrapper_map_.find(object);
if (it != wrapper_map_.end()) { if (it != wrapper_map_.end()) {
if (it->value.Get() == handle) { if (it->value->ref.Get() == handle) {
it->value.Clear(); it->value->ref.Clear();
wrapper_map_.erase(it); wrapper_map_.erase(it);
return true; return true;
} }
...@@ -164,7 +164,7 @@ class DOMDataStore final : public GarbageCollected<DOMDataStore> { ...@@ -164,7 +164,7 @@ class DOMDataStore final : public GarbageCollected<DOMDataStore> {
return object->SetReturnValue(return_value); return object->SetReturnValue(return_value);
auto it = wrapper_map_.find(object); auto it = wrapper_map_.find(object);
if (it != wrapper_map_.end()) { if (it != wrapper_map_.end()) {
return_value.Set(it->value.Get()); return_value.Set(it->value->ref.Get());
return true; return true;
} }
return false; return false;
...@@ -198,10 +198,24 @@ class DOMDataStore final : public GarbageCollected<DOMDataStore> { ...@@ -198,10 +198,24 @@ class DOMDataStore final : public GarbageCollected<DOMDataStore> {
return wrappable->IsEqualTo(holder); 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_; bool is_main_world_;
// Ephemeron map: V8 wrapper will be kept alive as long as ScriptWrappable is. // Ephemeron map: WrappedReference will be kept alive as long as
HeapHashMap<WeakMember<const ScriptWrappable>, // ScriptWrappable is alive.
TraceWrapperV8Reference<v8::Object>> HeapHashMap<WeakMember<const ScriptWrappable>, Member<WrappedReference>>
wrapper_map_; wrapper_map_;
DISALLOW_COPY_AND_ASSIGN(DOMDataStore); DISALLOW_COPY_AND_ASSIGN(DOMDataStore);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment