PaymentRequest: Use TraceWrapperV8Reference instead of ScriptValue

To avoid cyclic references between V8 and Blink, we should use
TraceWrapperV8Reference instead of ScriptValue.

This CL introduces a new static method-ToWorldSafeScriptValue()- and use
it instead of V8ValueFor() to create a clone across worlds as needed.

Related discussion:
  https://chromium-review.googlesource.com/c/chromium/src/+/1262968#message-e08afac4b75f8a1893fbaf7b536607e146a9bed6

Bug: none
Change-Id: Ib95bd8f47de55c7c42740373ebf80587af2b39f8
Reviewed-on: https://chromium-review.googlesource.com/c/1267155
Commit-Queue: Jinho Bang <jinho.bang@samsung.com>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604541}

third_party/blink/renderer/bindings/core/v8/script_value.cc

@@ -35,6 +35,28 @@
 #include "third_party/blink/renderer/platform/bindings/script_state.h"
 
 namespace blink {
+namespace {
+
+v8::Local<v8::Value> ToWorldSafeValue(ScriptState* target_script_state,
+                                      v8::Local<v8::Value> value) {
+  if (value.IsEmpty() || !value->IsObject())
+    return value;
+
+  v8::Local<v8::Context> creation_context =
+      value.As<v8::Object>()->CreationContext();
+  v8::Isolate* isolate = target_script_state->GetIsolate();
+  if (&ScriptState::From(creation_context)->World() ==
+      &target_script_state->World()) {
+    return value;
+  }
+
+  v8::Context::Scope target_context_scope(target_script_state->GetContext());
+  scoped_refptr<SerializedScriptValue> serialized =
+      SerializedScriptValue::SerializeAndSwallowExceptions(isolate, value);
+  return serialized->Deserialize(isolate);
+}
+
+}  // namespace
 
 v8::Local<v8::Value> ScriptValue::V8Value() const {
   if (IsEmpty())
@@ -56,15 +78,9 @@ v8::Local<v8::Value> ScriptValue::V8ValueFor(
     ScriptState* target_script_state) const {
   if (IsEmpty())
     return v8::Local<v8::Value>();
-  v8::Isolate* isolate = target_script_state->GetIsolate();
-  if (&script_state_->World() == &target_script_state->World())
-    return value_->NewLocal(isolate);
 
-  DCHECK(isolate->InContext());
-  v8::Local<v8::Value> value = value_->NewLocal(isolate);
-  scoped_refptr<SerializedScriptValue> serialized =
-      SerializedScriptValue::SerializeAndSwallowExceptions(isolate, value);
-  return serialized->Deserialize(isolate);
+  return ToWorldSafeValue(target_script_state,
+                          value_->NewLocal(target_script_state->GetIsolate()));
 }
 
 bool ScriptValue::ToString(String& result) const {
@@ -83,4 +99,14 @@ ScriptValue ScriptValue::CreateNull(ScriptState* script_state) {
   return ScriptValue(script_state, v8::Null(script_state->GetIsolate()));
 }
 
+// static
+ScriptValue ScriptValue::ToWorldSafeScriptValue(
+    ScriptState* target_script_state,
+    const TraceWrapperV8Reference<v8::Value>& value) {
+  return ScriptValue(
+      target_script_state,
+      ToWorldSafeValue(target_script_state,
+                       value.NewLocal(target_script_state->GetIsolate())));
+}
+
 }  // namespace blink

third_party/blink/renderer/bindings/core/v8/script_value.h

@@ -169,6 +169,10 @@ class CORE_EXPORT ScriptValue final {
 
   static ScriptValue CreateNull(ScriptState*);
 
+  static ScriptValue ToWorldSafeScriptValue(
+      ScriptState* target_script_state,
+      const TraceWrapperV8Reference<v8::Value>& value);
+
  private:
   // TODO(peria): Move ScriptValue to Oilpan heap.
   GC_PLUGIN_IGNORE("813731")

third_party/blink/renderer/modules/payments/payment_method_change_event.cc

@@ -6,6 +6,7 @@
 
 #include "third_party/blink/renderer/core/execution_context/execution_context.h"
 #include "third_party/blink/renderer/platform/bindings/script_state.h"
+#include "third_party/blink/renderer/platform/heap/visitor.h"
 
 namespace blink {
 
@@ -25,7 +26,14 @@ const String& PaymentMethodChangeEvent::methodName() const {
 
 const ScriptValue PaymentMethodChangeEvent::methodDetails(
     ScriptState* script_state) const {
-  return ScriptValue(script_state, method_details_.V8ValueFor(script_state));
+  if (method_details_.IsEmpty())
+    return ScriptValue::CreateNull(script_state);
+  return ScriptValue::ToWorldSafeScriptValue(script_state, method_details_);
+}
+
+void PaymentMethodChangeEvent::Trace(Visitor* visitor) {
+  visitor->Trace(method_details_);
+  PaymentRequestUpdateEvent::Trace(visitor);
 }
 
 PaymentMethodChangeEvent::PaymentMethodChangeEvent(
@@ -35,9 +43,11 @@ PaymentMethodChangeEvent::PaymentMethodChangeEvent(
     : PaymentRequestUpdateEvent(ExecutionContext::From(script_state),
                                 type,
                                 init),
-      method_name_(init->methodName()),
-      method_details_(init->hasMethodDetails()
-                          ? init->methodDetails()
-                          : ScriptValue::CreateNull(script_state)) {}
+      method_name_(init->methodName()) {
+  if (init->hasMethodDetails()) {
+    method_details_.Set(init->methodDetails().GetIsolate(),
+                        init->methodDetails().V8Value());
+  }
+}
 
 }  // namespace blink

third_party/blink/renderer/modules/payments/payment_method_change_event.h

@@ -10,6 +10,7 @@
 #include "third_party/blink/renderer/modules/payments/payment_method_change_event_init.h"
 #include "third_party/blink/renderer/modules/payments/payment_request_update_event.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/heap/handle.h"
 #include "third_party/blink/renderer/platform/wtf/text/atomic_string.h"
 #include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
@@ -34,13 +35,15 @@ class MODULES_EXPORT PaymentMethodChangeEvent final
   const String& methodName() const;
   const ScriptValue methodDetails(ScriptState*) const;
 
+  void Trace(Visitor* visitor) override;
+
  private:
   PaymentMethodChangeEvent(ScriptState*,
                            const AtomicString& type,
                            const PaymentMethodChangeEventInit*);
 
   String method_name_;
-  ScriptValue method_details_;
+  TraceWrapperV8Reference<v8::Value> method_details_;
 };
 
 }  // namespace blink

third_party/blink/renderer/modules/payments/payment_response.cc

@@ -63,7 +63,8 @@ void PaymentResponse::UpdateDetailsFromJSON(ScriptState* script_state,
                                             const String& json) {
   ScriptState::Scope scope(script_state);
   if (json.IsEmpty()) {
-    details_ = V8ObjectBuilder(script_state).GetScriptValue();
+    details_.Set(script_state->GetIsolate(),
+                 V8ObjectBuilder(script_state).V8Value());
     return;
   }
 
@@ -75,10 +76,11 @@ void PaymentResponse::UpdateDetailsFromJSON(ScriptState* script_state,
                      json, exception_state);
   if (exception_state.HadException()) {
     exception_state.ClearException();
-    details_ = V8ObjectBuilder(script_state).GetScriptValue();
+    details_.Set(script_state->GetIsolate(),
+                 V8ObjectBuilder(script_state).V8Value());
     return;
   }
-  details_ = ScriptValue(script_state, parsed_value);
+  details_.Set(script_state->GetIsolate(), parsed_value);
 }
 
 ScriptValue PaymentResponse::toJSONForBinding(ScriptState* script_state) const {
@@ -102,7 +104,7 @@ ScriptValue PaymentResponse::toJSONForBinding(ScriptState* script_state) const {
 }
 
 ScriptValue PaymentResponse::details(ScriptState* script_state) const {
-  return ScriptValue(script_state, details_.V8ValueFor(script_state));
+  return ScriptValue::ToWorldSafeScriptValue(script_state, details_);
 }
 
 ScriptPromise PaymentResponse::complete(ScriptState* script_state,
@@ -135,6 +137,7 @@ ExecutionContext* PaymentResponse::GetExecutionContext() const {
 }
 
 void PaymentResponse::Trace(blink::Visitor* visitor) {
+  visitor->Trace(details_);
   visitor->Trace(shipping_address_);
   visitor->Trace(payment_state_resolver_);
   EventTargetWithInlineData::Trace(visitor);

third_party/blink/renderer/modules/payments/payment_response.h

@@ -14,6 +14,7 @@
 #include "third_party/blink/renderer/modules/modules_export.h"
 #include "third_party/blink/renderer/modules/payments/payment_currency_amount.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/heap/handle.h"
 #include "third_party/blink/renderer/platform/wtf/noncopyable.h"
 #include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
@@ -73,7 +74,7 @@ class MODULES_EXPORT PaymentResponse final
  private:
   String request_id_;
   String method_name_;
-  ScriptValue details_;
+  TraceWrapperV8Reference<v8::Value> details_;
   Member<PaymentAddress> shipping_address_;
   String shipping_option_;
   String payer_name_;