Trace CustomEvent.detail instead of keeping ScriptValue

Before this CL we used V8PrivateProperty to avoid
cyclic references between V8 and Blink. Now we have
trace wrapper and DOM object can have a reference to
V8 objects. Use trace wrapper for CustomEvent

BUG=501866,700680

Change-Id: I9089d9cf0828bb979467cc5bd859e460847b6c28
Reviewed-on: https://chromium-review.googlesource.com/484162
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#467934}

third_party/WebKit/Source/bindings/bindings.gni

@@ -10,7 +10,6 @@ import("//third_party/WebKit/Source/bindings/modules/v8/v8.gni")
 bindings_core_v8_files =
     get_path_info([
                     "core/v8/custom/V8CSSStyleDeclarationCustom.cpp",
-                    "core/v8/custom/V8CustomEventCustom.cpp",
                     "core/v8/custom/V8CustomXPathNSResolver.cpp",
                     "core/v8/custom/V8CustomXPathNSResolver.h",
                     "core/v8/custom/V8DevToolsHostCustom.cpp",

third_party/WebKit/Source/bindings/core/v8/custom/V8CustomEventCustom.cpp

-/*
- * Copyright (C) 2012 Google Inc. All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are
- * met:
- *
- *     * Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- *     * Redistributions in binary form must reproduce the above
- * copyright notice, this list of conditions and the following disclaimer
- * in the documentation and/or other materials provided with the
- * distribution.
- *     * Neither the name of Google Inc. nor the names of its
- * contributors may be used to endorse or promote products derived from
- * this software without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#include "bindings/core/v8/V8CustomEvent.h"
-
-#include "bindings/core/v8/Dictionary.h"
-#include "bindings/core/v8/ScriptState.h"
-#include "bindings/core/v8/SerializedScriptValue.h"
-#include "bindings/core/v8/SerializedScriptValueFactory.h"
-#include "bindings/core/v8/V8BindingForCore.h"
-#include "bindings/core/v8/V8CustomEventInit.h"
-#include "bindings/core/v8/V8DOMWrapper.h"
-#include "bindings/core/v8/V8Event.h"
-#include "bindings/core/v8/V8PrivateProperty.h"
-#include "core/dom/ContextFeatures.h"
-#include "platform/RuntimeEnabledFeatures.h"
-
-namespace blink {
-
-static void StoreDetail(ScriptState* script_state,
-                        CustomEvent* impl,
-                        v8::Local<v8::Object> wrapper,
-                        v8::Local<v8::Value> detail) {
-  auto private_detail =
-      V8PrivateProperty::GetCustomEventDetail(script_state->GetIsolate());
-  private_detail.Set(wrapper, detail);
-
-  // When a custom event is created in an isolated world, serialize
-  // |detail| and store it in |impl| so that we can clone |detail|
-  // when the getter of |detail| is called in the main world later.
-  if (script_state->World().IsIsolatedWorld())
-    impl->SetSerializedDetail(
-        SerializedScriptValue::SerializeAndSwallowExceptions(
-            script_state->GetIsolate(), detail));
-}
-
-void V8CustomEvent::constructorCustom(
-    const v8::FunctionCallbackInfo<v8::Value>& info) {
-  ExceptionState exception_state(
-      info.GetIsolate(), ExceptionState::kConstructionContext, "CustomEvent");
-  if (UNLIKELY(info.Length() < 1)) {
-    exception_state.ThrowTypeError(
-        ExceptionMessages::NotEnoughArguments(1, info.Length()));
-    return;
-  }
-
-  V8StringResource<> type = info[0];
-  if (!type.Prepare())
-    return;
-
-  CustomEventInit event_init_dict;
-  if (!IsUndefinedOrNull(info[1])) {
-    if (!info[1]->IsObject()) {
-      exception_state.ThrowTypeError(
-          "parameter 2 ('eventInitDict') is not an object.");
-      return;
-    }
-    V8CustomEventInit::toImpl(info.GetIsolate(), info[1], event_init_dict,
-                              exception_state);
-    if (exception_state.HadException())
-      return;
-  }
-
-  CustomEvent* impl = CustomEvent::Create(type, event_init_dict);
-  v8::Local<v8::Object> wrapper = info.Holder();
-  wrapper = impl->AssociateWithWrapper(
-      info.GetIsolate(), &V8CustomEvent::wrapperTypeInfo, wrapper);
-
-  // TODO(bashi): Workaround for http://crbug.com/529941. We need to store
-  // |detail| as a private property to avoid cycle references.
-  if (event_init_dict.hasDetail()) {
-    v8::Local<v8::Value> v8_detail = event_init_dict.detail().V8Value();
-    StoreDetail(ScriptState::Current(info.GetIsolate()), impl, wrapper,
-                v8_detail);
-  }
-  V8SetReturnValue(info, wrapper);
-}
-
-void V8CustomEvent::initCustomEventMethodEpilogueCustom(
-    const v8::FunctionCallbackInfo<v8::Value>& info,
-    CustomEvent* impl) {
-  if (impl->IsBeingDispatched())
-    return;
-  if (info.Length() >= 4) {
-    v8::Local<v8::Value> detail = info[3];
-    if (!detail.IsEmpty()) {
-      StoreDetail(ScriptState::Current(info.GetIsolate()), impl, info.Holder(),
-                  detail);
-    }
-  }
-}
-
-void V8CustomEvent::detailAttributeGetterCustom(
-    const v8::FunctionCallbackInfo<v8::Value>& info) {
-  v8::Isolate* isolate = info.GetIsolate();
-  CustomEvent* event = V8CustomEvent::toImpl(info.Holder());
-  ScriptState* script_state = ScriptState::Current(isolate);
-
-  auto private_detail = V8PrivateProperty::GetCustomEventDetail(isolate);
-  v8::Local<v8::Value> detail = private_detail.GetOrEmpty(info.Holder());
-  if (!detail.IsEmpty()) {
-    V8SetReturnValue(info, detail);
-    return;
-  }
-
-  // Be careful not to return a V8 value which is created in different world.
-  if (SerializedScriptValue* serialized_value = event->SerializedDetail()) {
-    detail = serialized_value->Deserialize(isolate);
-  } else if (script_state->World().IsIsolatedWorld()) {
-    v8::Local<v8::Value> main_world_detail =
-        private_detail.GetFromMainWorld(event);
-    if (!main_world_detail.IsEmpty()) {
-      event->SetSerializedDetail(
-          SerializedScriptValue::SerializeAndSwallowExceptions(
-              isolate, main_world_detail));
-      detail = event->SerializedDetail()->Deserialize(isolate);
-    }
-  }
-
-  // |detail| should be null when it is an empty handle because its default
-  // value is null.
-  if (detail.IsEmpty())
-    detail = v8::Null(isolate);
-  private_detail.Set(info.Holder(), detail);
-  V8SetReturnValue(info, detail);
-}
-
-}  // namespace blink

third_party/WebKit/Source/core/events/CustomEvent.cpp

@@ -30,32 +30,44 @@
 
 namespace blink {
 
-CustomEvent::CustomEvent() {}
+CustomEvent::CustomEvent() : detail_(this) {}
 
-CustomEvent::CustomEvent(const AtomicString& type,
+CustomEvent::CustomEvent(ScriptState* script_state,
+                         const AtomicString& type,
                          const CustomEventInit& initializer)
-    : Event(type, initializer) {}
+    : Event(type, initializer), detail_(this) {
+  world_ = RefPtr<DOMWrapperWorld>(script_state->World());
+  if (initializer.hasDetail()) {
+    detail_.Set(initializer.detail().GetIsolate(),
+                initializer.detail().V8Value());
+  }
+}
 
 CustomEvent::~CustomEvent() {}
 
-void CustomEvent::initCustomEvent(const AtomicString& type,
+void CustomEvent::initCustomEvent(ScriptState* script_state,
+                                  const AtomicString& type,
                                   bool can_bubble,
                                   bool cancelable,
-                                  const ScriptValue&) {
+                                  const ScriptValue& script_value) {
   initEvent(type, can_bubble, cancelable);
+  world_ = RefPtr<DOMWrapperWorld>(script_state->World());
+  if (!IsBeingDispatched() && !script_value.IsEmpty())
+    detail_.Set(script_value.GetIsolate(), script_value.V8Value());
 }
 
-void CustomEvent::initCustomEvent(
-    const AtomicString& type,
-    bool can_bubble,
-    bool cancelable,
-    PassRefPtr<SerializedScriptValue> serialized_detail) {
-  if (IsBeingDispatched())
-    return;
-
-  initEvent(type, can_bubble, cancelable);
-
-  serialized_detail_ = std::move(serialized_detail);
+ScriptValue CustomEvent::detail(ScriptState* script_state) const {
+  v8::Isolate* isolate = script_state->GetIsolate();
+  if (detail_.IsEmpty())
+    return ScriptValue(script_state, v8::Null(isolate));
+  // Returns a clone of |detail_| if the world is different.
+  if (!world_ || world_->GetWorldId() != script_state->World().GetWorldId()) {
+    v8::Local<v8::Value> value = detail_.NewLocal(isolate);
+    RefPtr<SerializedScriptValue> serialized =
+        SerializedScriptValue::SerializeAndSwallowExceptions(isolate, value);
+    return ScriptValue(script_state, serialized->Deserialize(isolate));
+  }
+  return ScriptValue(script_state, detail_.NewLocal(isolate));
 }
 
 const AtomicString& CustomEvent::InterfaceName() const {
@@ -66,4 +78,9 @@ DEFINE_TRACE(CustomEvent) {
   Event::Trace(visitor);
 }
 
+DEFINE_TRACE_WRAPPERS(CustomEvent) {
+  visitor->TraceWrappers(detail_);
+  Event::TraceWrappers(visitor);
+}
+
 }  // namespace blink

third_party/WebKit/Source/core/events/CustomEvent.h

@@ -26,14 +26,14 @@
 #ifndef CustomEvent_h
 #define CustomEvent_h
 
+#include "bindings/core/v8/DOMWrapperWorld.h"
+#include "bindings/core/v8/TraceWrapperV8Reference.h"
 #include "core/CoreExport.h"
 #include "core/events/CustomEventInit.h"
 #include "core/events/Event.h"
 
 namespace blink {
 
-class SerializedScriptValue;
-
 class CORE_EXPORT CustomEvent final : public Event {
   DEFINE_WRAPPERTYPEINFO();
 
@@ -42,35 +42,34 @@ class CORE_EXPORT CustomEvent final : public Event {
 
   static CustomEvent* Create() { return new CustomEvent; }
 
-  static CustomEvent* Create(const AtomicString& type,
+  static CustomEvent* Create(ScriptState* script_state,
+                             const AtomicString& type,
                              const CustomEventInit& initializer) {
-    return new CustomEvent(type, initializer);
+    return new CustomEvent(script_state, type, initializer);
   }
 
-  void initCustomEvent(const AtomicString& type,
+  void initCustomEvent(ScriptState*,
+                       const AtomicString& type,
                        bool can_bubble,
                        bool cancelable,
                        const ScriptValue& detail);
-  void initCustomEvent(const AtomicString& type,
-                       bool can_bubble,
-                       bool cancelable,
-                       PassRefPtr<SerializedScriptValue>);
 
   const AtomicString& InterfaceName() const override;
 
-  SerializedScriptValue* SerializedDetail() { return serialized_detail_.Get(); }
-  void SetSerializedDetail(
-      PassRefPtr<SerializedScriptValue> serialized_detail) {
-    serialized_detail_ = std::move(serialized_detail);
-  }
+  ScriptValue detail(ScriptState*) const;
 
   DECLARE_VIRTUAL_TRACE();
 
+  DECLARE_VIRTUAL_TRACE_WRAPPERS();
+
  private:
   CustomEvent();
-  CustomEvent(const AtomicString& type, const CustomEventInit& initializer);
+  CustomEvent(ScriptState*,
+              const AtomicString& type,
+              const CustomEventInit& initializer);
 
-  RefPtr<SerializedScriptValue> serialized_detail_;
+  RefPtr<DOMWrapperWorld> world_;
+  TraceWrapperV8Reference<v8::Value> detail_;
 };
 
 }  // namespace blink

third_party/WebKit/Source/core/events/CustomEvent.idl

@@ -26,12 +26,12 @@
 // https://dom.spec.whatwg.org/#interface-customevent
 
 [
-    // TODO(bashi): Don't use CustomConstructor. Constructor should be:
-    // Constructor(DOMString type, optional CustomEventInit eventInitDict),
-    CustomConstructor(DOMString type, optional CustomEventInit eventInitDict),
+    Constructor(DOMString type, optional CustomEventInit eventInitDict),
+    ConstructorCallWith=ScriptState,
+    DependentLifetime,
     Exposed=(Window,Worker),
 ] interface CustomEvent : Event {
-    [Custom=Getter] readonly attribute any detail;
+    [CallWith=ScriptState] readonly attribute any detail;
 
-    [Measure,Custom=(CallEpilogue)] void initCustomEvent(DOMString type, optional boolean bubbles = false, optional boolean cancelable = false, optional any detail = null);
+    [Measure, CallWith=ScriptState] void initCustomEvent(DOMString type, optional boolean bubbles = false, optional boolean cancelable = false, optional any detail = null);
 };

third_party/WebKit/Source/platform/bindings/V8PrivateProperty.h

@@ -38,7 +38,6 @@ class ScriptWrappable;
   X(CustomElementLifecycle, AttributeChangedCallback) \
   X(CustomElementLifecycle, CreatedCallback)          \
   X(CustomElementLifecycle, DetachedCallback)         \
-  X(CustomEvent, Detail)                              \
   X(DOMException, Error)                              \
   X(ErrorEvent, Error)                                \
   X(FetchEvent, Request)                              \