Move ScriptPromiseProperties from V8HiddenValue to V8PrivateProperty

This change makes V8HiddenValue class stateless, and we can remove
m_hiddenValue in V8PerIsolate.

BUG=611864

Review-Url: https://codereview.chromium.org/2774233007
Cr-Commit-Position: refs/heads/master@{#462837}

third_party/WebKit/Source/bindings/core/v8/ScriptPromiseProperties.h

@@ -6,13 +6,13 @@
 #define ScriptPromiseProperties_h
 
 // See ScriptPromiseProperty.h
-#define SCRIPT_PROMISE_PROPERTIES(P, ...) \
-  P(Ready##__VA_ARGS__)                   \
-  P(Closed##__VA_ARGS__)                  \
-  P(Finished##__VA_ARGS__)                \
-  P(Loaded##__VA_ARGS__)                  \
-  P(Released##__VA_ARGS__)                \
-  P(UserChoice##__VA_ARGS__)              \
-  P(PreloadResponse##__VA_ARGS__)
+#define SCRIPT_PROMISE_PROPERTIES(P, ...)   \
+  P(ScriptPromise, Ready##__VA_ARGS__)      \
+  P(ScriptPromise, Closed##__VA_ARGS__)     \
+  P(ScriptPromise, Finished##__VA_ARGS__)   \
+  P(ScriptPromise, Loaded##__VA_ARGS__)     \
+  P(ScriptPromise, Released##__VA_ARGS__)   \
+  P(ScriptPromise, UserChoice##__VA_ARGS__) \
+  P(ScriptPromise, PreloadResponse##__VA_ARGS__)
 
 #endif  // ScriptPromiseProperties_h

third_party/WebKit/Source/bindings/core/v8/ScriptPromisePropertyBase.cpp

@@ -40,9 +40,8 @@ ScriptPromise ScriptPromisePropertyBase::promise(DOMWrapperWorld& world) {
   v8::Local<v8::Object> wrapper = ensureHolderWrapper(scriptState);
   DCHECK(wrapper->CreationContext() == context);
 
-  v8::Local<v8::Value> cachedPromise =
-      V8HiddenValue::getHiddenValue(scriptState, wrapper, promiseName());
-  if (!cachedPromise.IsEmpty() && cachedPromise->IsPromise())
+  v8::Local<v8::Value> cachedPromise = promiseSymbol().getOrUndefined(wrapper);
+  if (!cachedPromise->IsUndefined() && cachedPromise->IsPromise())
     return ScriptPromise(scriptState, cachedPromise);
 
   // Create and cache the Promise
@@ -50,13 +49,12 @@ ScriptPromise ScriptPromisePropertyBase::promise(DOMWrapperWorld& world) {
   if (!v8::Promise::Resolver::New(context).ToLocal(&resolver))
     return ScriptPromise();
   v8::Local<v8::Promise> promise = resolver->GetPromise();
-  V8HiddenValue::setHiddenValue(scriptState, wrapper, promiseName(), promise);
+  promiseSymbol().set(wrapper, promise);
 
   switch (m_state) {
     case Pending:
       // Cache the resolver too
-      V8HiddenValue::setHiddenValue(scriptState, wrapper, resolverName(),
-                                    resolver);
+      resolverSymbol().set(wrapper, resolver);
       break;
     case Resolved:
     case Rejected:
@@ -90,12 +88,12 @@ void ScriptPromisePropertyBase::resolveOrReject(State targetState) {
     ScriptState* scriptState = ScriptState::from(wrapper->CreationContext());
     ScriptState::Scope scope(scriptState);
 
+    V8PrivateProperty::Symbol symbol = resolverSymbol();
+    DCHECK(symbol.hasValue(wrapper));
     v8::Local<v8::Promise::Resolver> resolver =
-        V8HiddenValue::getHiddenValue(scriptState, wrapper, resolverName())
-            .As<v8::Promise::Resolver>();
-    DCHECK(!resolver.IsEmpty());
+        symbol.getOrUndefined(wrapper).As<v8::Promise::Resolver>();
 
-    V8HiddenValue::deleteHiddenValue(scriptState, wrapper, resolverName());
+    symbol.deleteProperty(wrapper);
     resolveOrRejectInternal(resolver);
     ++i;
   }
@@ -163,9 +161,9 @@ void ScriptPromisePropertyBase::clearWrappers() {
        i != m_wrappers.end(); ++i) {
     v8::Local<v8::Object> wrapper = (*i)->newLocal(m_isolate);
     if (!wrapper.IsEmpty()) {
-      ScriptState* scriptState = ScriptState::from(wrapper->CreationContext());
-      V8HiddenValue::deleteHiddenValue(scriptState, wrapper, resolverName());
-      V8HiddenValue::deleteHiddenValue(scriptState, wrapper, promiseName());
+      resolverSymbol().deleteProperty(wrapper);
+      // TODO(peria): Use deleteProperty() if http://crbug.com/v8/6227 is fixed.
+      promiseSymbol().set(wrapper, v8::Undefined(m_isolate));
     }
   }
   m_wrappers.clear();
@@ -182,32 +180,34 @@ void ScriptPromisePropertyBase::checkWrappers() {
   }
 }
 
-v8::Local<v8::String> ScriptPromisePropertyBase::promiseName() {
+V8PrivateProperty::Symbol ScriptPromisePropertyBase::promiseSymbol() {
   switch (m_name) {
-#define P(Name) \
-  case Name:    \
-    return V8HiddenValue::Name##Promise(m_isolate);
+#define P(Interface, Name)                                     \
+  case Name:                                                   \
+    return V8PrivateProperty::V8_PRIVATE_PROPERTY_GETTER_NAME( \
+        Interface, Name##Promise)(m_isolate);
 
     SCRIPT_PROMISE_PROPERTIES(P)
 
 #undef P
   }
-  ASSERT_NOT_REACHED();
-  return v8::Local<v8::String>();
+  NOTREACHED();
+  return V8PrivateProperty::getSymbol(m_isolate, "noPromise");
 }
 
-v8::Local<v8::String> ScriptPromisePropertyBase::resolverName() {
+V8PrivateProperty::Symbol ScriptPromisePropertyBase::resolverSymbol() {
   switch (m_name) {
-#define P(Name) \
-  case Name:    \
-    return V8HiddenValue::Name##Resolver(m_isolate);
+#define P(Interface, Name)                                     \
+  case Name:                                                   \
+    return V8PrivateProperty::V8_PRIVATE_PROPERTY_GETTER_NAME( \
+        Interface, Name##Resolver)(m_isolate);
 
     SCRIPT_PROMISE_PROPERTIES(P)
 
 #undef P
   }
-  ASSERT_NOT_REACHED();
-  return v8::Local<v8::String>();
+  NOTREACHED();
+  return V8PrivateProperty::getSymbol(m_isolate, "noResolver");
 }
 
 DEFINE_TRACE(ScriptPromisePropertyBase) {

third_party/WebKit/Source/bindings/core/v8/ScriptPromisePropertyBase.h

@@ -10,6 +10,7 @@
 #include "bindings/core/v8/ScopedPersistent.h"
 #include "bindings/core/v8/ScriptPromise.h"
 #include "bindings/core/v8/ScriptPromiseProperties.h"
+#include "bindings/core/v8/V8PrivateProperty.h"
 #include "core/CoreExport.h"
 #include "core/dom/ContextLifecycleObserver.h"
 #include "platform/wtf/Compiler.h"
@@ -33,7 +34,7 @@ class CORE_EXPORT ScriptPromisePropertyBase
   virtual ~ScriptPromisePropertyBase();
 
   enum Name {
-#define P(Name) Name,
+#define P(Unused, Name) Name,
     SCRIPT_PROMISE_PROPERTIES(P)
 #undef P
   };
@@ -83,8 +84,8 @@ class CORE_EXPORT ScriptPromisePropertyBase
   NEVER_INLINE void checkThis();
   NEVER_INLINE void checkWrappers();
 
-  v8::Local<v8::String> promiseName();
-  v8::Local<v8::String> resolverName();
+  V8PrivateProperty::Symbol promiseSymbol();
+  V8PrivateProperty::Symbol resolverSymbol();
 
   v8::Isolate* m_isolate;
   Name m_name;

third_party/WebKit/Source/bindings/core/v8/ScriptPromisePropertyTest.cpp

@@ -431,6 +431,8 @@ TEST_F(ScriptPromisePropertyGarbageCollectedTest, Resolve_DeadContext) {
 }
 
 TEST_F(ScriptPromisePropertyGarbageCollectedTest, Reset) {
+  ScriptState::Scope scope(mainScriptState());
+
   ScriptPromise oldPromise, newPromise;
   ScriptValue oldActual, newActual;
   GarbageCollectedScriptWrappable* oldValue =

third_party/WebKit/Source/bindings/core/v8/V8EventListenerHelper.h

@@ -33,6 +33,7 @@
 
 #include "bindings/core/v8/V8Binding.h"
 #include "bindings/core/v8/V8EventListener.h"
+#include "bindings/core/v8/V8HiddenValue.h"
 #include "core/CoreExport.h"
 #include "platform/wtf/Allocator.h"
 #include "v8/include/v8.h"

third_party/WebKit/Source/bindings/core/v8/V8HiddenValue.cpp

@@ -10,18 +10,6 @@
 
 namespace blink {
 
-#define V8_DEFINE_METHOD(name)                                            \
-  v8::Local<v8::String> V8HiddenValue::name(v8::Isolate* isolate) {       \
-    V8HiddenValue* hiddenValue =                                          \
-        V8PerIsolateData::from(isolate)->hiddenValue();                   \
-    if (hiddenValue->m_##name.isEmpty()) {                                \
-      hiddenValue->m_##name.set(isolate, v8AtomicString(isolate, #name)); \
-    }                                                                     \
-    return hiddenValue->m_##name.newLocal(isolate);                       \
-  }
-
-V8_HIDDEN_VALUES(V8_DEFINE_METHOD);
-
 v8::Local<v8::Value> V8HiddenValue::getHiddenValue(ScriptState* scriptState,
                                                    v8::Local<v8::Object> object,
                                                    v8::Local<v8::String> key) {
@@ -59,14 +47,4 @@ bool V8HiddenValue::deleteHiddenValue(ScriptState* scriptState,
       v8::Undefined(scriptState->isolate())));
 }
 
-v8::Local<v8::Value> V8HiddenValue::getHiddenValueFromMainWorldWrapper(
-    ScriptState* scriptState,
-    ScriptWrappable* wrappable,
-    v8::Local<v8::String> key) {
-  v8::Local<v8::Object> wrapper =
-      wrappable->mainWorldWrapper(scriptState->isolate());
-  return wrapper.IsEmpty() ? v8::Local<v8::Value>()
-                           : getHiddenValue(scriptState, wrapper, key);
-}
-
 }  // namespace blink

third_party/WebKit/Source/bindings/core/v8/V8HiddenValue.h

@@ -17,26 +17,9 @@
 namespace blink {
 
 class ScriptState;
-class ScriptWrappable;
-
-#define V8_HIDDEN_VALUES(V)                  \
-  SCRIPT_PROMISE_PROPERTIES(V, Promise)      \
-  SCRIPT_PROMISE_PROPERTIES(V, Resolver)
 
 class CORE_EXPORT V8HiddenValue {
-  USING_FAST_MALLOC(V8HiddenValue);
-  WTF_MAKE_NONCOPYABLE(V8HiddenValue);
-
  public:
-  static std::unique_ptr<V8HiddenValue> create() {
-    return WTF::wrapUnique(new V8HiddenValue());
-  }
-
-#define V8_DECLARE_METHOD(name) \
-  static v8::Local<v8::String> name(v8::Isolate* isolate);
-  V8_HIDDEN_VALUES(V8_DECLARE_METHOD);
-#undef V8_DECLARE_METHOD
-
   static v8::Local<v8::Value> getHiddenValue(ScriptState*,
                                              v8::Local<v8::Object>,
                                              v8::Local<v8::String>);
@@ -47,17 +30,6 @@ class CORE_EXPORT V8HiddenValue {
   static bool deleteHiddenValue(ScriptState*,
                                 v8::Local<v8::Object>,
                                 v8::Local<v8::String>);
-  static v8::Local<v8::Value> getHiddenValueFromMainWorldWrapper(
-      ScriptState*,
-      ScriptWrappable*,
-      v8::Local<v8::String>);
-
- private:
-  V8HiddenValue() {}
-
-#define V8_DECLARE_FIELD(name) ScopedPersistent<v8::String> m_##name;
-  V8_HIDDEN_VALUES(V8_DECLARE_FIELD);
-#undef V8_DECLARE_FIELD
 };
 
 }  // namespace blink

third_party/WebKit/Source/bindings/core/v8/V8PerIsolateData.cpp

@@ -30,7 +30,6 @@
 #include "bindings/core/v8/DOMDataStore.h"
 #include "bindings/core/v8/ScriptSourceCode.h"
 #include "bindings/core/v8/V8Binding.h"
-#include "bindings/core/v8/V8HiddenValue.h"
 #include "bindings/core/v8/V8ObjectConstructor.h"
 #include "bindings/core/v8/V8PrivateProperty.h"
 #include "bindings/core/v8/V8ScriptRunner.h"
@@ -60,7 +59,6 @@ V8PerIsolateData::V8PerIsolateData(WebTaskRunner* taskRunner)
           isMainThread() ? gin::IsolateHolder::kDisallowAtomicsWait
                          : gin::IsolateHolder::kAllowAtomicsWait),
       m_stringCache(WTF::wrapUnique(new StringCache(isolate()))),
-      m_hiddenValue(V8HiddenValue::create()),
       m_privateProperty(V8PrivateProperty::create()),
       m_constructorMode(ConstructorMode::CreateNewObject),
       m_useCounterDisabled(false),
@@ -124,7 +122,6 @@ void V8PerIsolateData::destroy(v8::Isolate* isolate) {
   if (data->m_scriptRegexpScriptState)
     data->m_scriptRegexpScriptState->disposePerContextData();
   data->m_liveRoot.clear();
-  data->m_hiddenValue.reset();
   data->m_privateProperty.reset();
   data->m_stringCache->dispose();
   data->m_stringCache.reset();

third_party/WebKit/Source/bindings/core/v8/V8PerIsolateData.h

@@ -31,7 +31,6 @@
 #include "bindings/core/v8/ScopedPersistent.h"
 #include "bindings/core/v8/ScriptState.h"
 #include "bindings/core/v8/ScriptWrappableVisitor.h"
-#include "bindings/core/v8/V8HiddenValue.h"
 #include "bindings/core/v8/WrapperTypeInfo.h"
 #include "core/CoreExport.h"
 #include "gin/public/isolate_holder.h"
@@ -132,7 +131,6 @@ class CORE_EXPORT V8PerIsolateData {
 
   bool isUseCounterDisabled() const { return m_useCounterDisabled; }
 
-  V8HiddenValue* hiddenValue() { return m_hiddenValue.get(); }
   V8PrivateProperty* privateProperty() { return m_privateProperty.get(); }
 
   // Accessors to the cache of interface templates.
@@ -251,7 +249,6 @@ class CORE_EXPORT V8PerIsolateData {
   HashMap<const void*, Vector<v8::Eternal<v8::Name>>> m_eternalNameCache;
 
   std::unique_ptr<StringCache> m_stringCache;
-  std::unique_ptr<V8HiddenValue> m_hiddenValue;
   std::unique_ptr<V8PrivateProperty> m_privateProperty;
   ScopedPersistent<v8::Value> m_liveRoot;
   RefPtr<ScriptState> m_scriptRegexpScriptState;

third_party/WebKit/Source/bindings/core/v8/V8PrivateProperty.h

@@ -60,7 +60,9 @@ class ScriptWrappable;
   X(SameObject, NotificationData)                     \
   X(SameObject, NotificationVibrate)                  \
   X(SameObject, PerformanceLongTaskTimingAttribution) \
-  X(V8NodeFilterCondition, Filter)
+  X(V8NodeFilterCondition, Filter)                    \
+  SCRIPT_PROMISE_PROPERTIES(X, Promise)               \
+  SCRIPT_PROMISE_PROPERTIES(X, Resolver)
 
 // The getter's name for a private property.
 #define V8_PRIVATE_PROPERTY_GETTER_NAME(InterfaceName, PrivateKeyName) \