New issue
Advanced search Search tips

Issue 803435 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Missing write barrier in DOMWrapperMap

Project Member Reported by u...@chromium.org, Jan 18 2018

Issue description

The setter of DOMWrapperMap is missing a write barrier for incremental wrapper tracing:

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/bindings/DOMWrapperMap.h?rcl=64e2da98606c06b9a13deef29c5667a5d6c52052&l=72

  WARN_UNUSED_RESULT bool Set(KeyType* key,
                              const WrapperTypeInfo* wrapper_type_info,
                              v8::Local<v8::Object>& wrapper) {
    if (UNLIKELY(ContainsKey(key))) {
      wrapper = NewLocal(isolate_, key);
      return false;
    }
    v8::Global<v8::Object> global(isolate_, wrapper);
    wrapper_type_info->ConfigureWrapper(&global);
    map_.Set(key, std::move(global));
    return true;
  }


 

Comment 1 by u...@chromium.org, Jan 18 2018

DOMWrapperMap breaks the ScriptWrappableVisitor abstraction.

It should call ScriptWrappableVisitor::TraceWrappers(map, key) and ScriptWrappableVisitor::WriteBarrier(map, key, value).

Instead of that, DOMWrapperMap currently goes directly to V8 via map_.RegisterExternallyReferencedObject(object); (https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/bindings/DOMWrapperMap.h?rcl=b28bdd7d0fc2d0afa71a387717ccc0d7264bcbec&l=98)

So the derived visitors have no way of visiting DOMWrapperMap values.

Comment 2 by u...@chromium.org, Jan 18 2018

Actually, TraceWrappers and WriteBarrier should be called one level higher in DOMDataStore because it knows that the key is ScriptWrappable.

Comment 3 by u...@chromium.org, Apr 23 2018

Status: Fixed (was: Assigned)
Fixed in https://chromium-review.googlesource.com/c/chromium/src/+/873919

Sign in to add a comment