[Bindings] Simplify V8PrivateProperty::Symbol methods

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

Index: third_party/WebKit/Source/bindings/core/v8/V8PrivateProperty.h
diff --git a/third_party/WebKit/Source/bindings/core/v8/V8PrivateProperty.h b/third_party/WebKit/Source/bindings/core/v8/V8PrivateProperty.h
index 3284bfae7e63c4c310b2ebba2cac8f959d5f690d..f1cf265784a6cc245dbd377a1c2bb289f0393b65 100644
--- a/third_party/WebKit/Source/bindings/core/v8/V8PrivateProperty.h
+++ b/third_party/WebKit/Source/bindings/core/v8/V8PrivateProperty.h
@@ -17,7 +17,6 @@
 
 namespace blink {
 
-class ScriptState;
 class ScriptWrappable;
 
 // Apply |X| for each pair of (InterfaceName, PrivateKeyName).
@@ -88,35 +87,31 @@ class CORE_EXPORT V8PrivateProperty {
     STACK_ALLOCATED();
 
    public:
-    bool hasValue(v8::Local<v8::Context> context,
-                  v8::Local<v8::Object> object) const {
-      return v8CallBoolean(object->HasPrivate(context, m_privateSymbol));
+    bool hasValue(v8::Local<v8::Object> object) const {
+      return v8CallBoolean(object->HasPrivate(context(), m_privateSymbol));
     }
 
     // Returns the value of the private property if set, or undefined.
-    v8::Local<v8::Value> getOrUndefined(v8::Local<v8::Context> context,
-                                        v8::Local<v8::Object> object) const {
-      return object->GetPrivate(context, m_privateSymbol).ToLocalChecked();
+    v8::Local<v8::Value> getOrUndefined(v8::Local<v8::Object> object) const {
+      return object->GetPrivate(context(), m_privateSymbol).ToLocalChecked();
     }
 
     // Returns the value of the private property if set, or an empty handle.
-    v8::Local<v8::Value> get(v8::Local<v8::Context> context,
-                             v8::Local<v8::Object> object) const {
-      if (!v8CallBoolean(object->HasPrivate(context, m_privateSymbol)))
+    v8::Local<v8::Value> getOrEmpty(v8::Local<v8::Object> object) const {
+      if (!hasValue(object))
         return v8::Local<v8::Value>();
       v8::Local<v8::Value> value;
-      if (v8Call(object->GetPrivate(context, m_privateSymbol), value))
+      if (v8Call(object->GetPrivate(context(), m_privateSymbol), value))

[Yuki, 2017/03/30 07:25:42]

v8Call is deprecated, so it's better to explicitly call Maybe::ToLocalChecked().

[peria, 2017/03/30 07:41:37]

Done.

         return value;
       return v8::Local<v8::Value>();
     }
 
-    bool set(v8::Local<v8::Context> context,
-             v8::Local<v8::Object> object,
-             v8::Local<v8::Value> value) const {
-      return v8CallBoolean(object->SetPrivate(context, m_privateSymbol, value));
+    bool set(v8::Local<v8::Object> object, v8::Local<v8::Value> value) const {
+      return v8CallBoolean(
+          object->SetPrivate(context(), m_privateSymbol, value));
     }
 
-    v8::Local<v8::Private> getPrivate() { return m_privateSymbol; }
+    v8::Local<v8::Private> getPrivate() const { return m_privateSymbol; }
 
    private:
     friend class V8PrivateProperty;
@@ -125,42 +120,40 @@ class CORE_EXPORT V8PrivateProperty {
     friend class V8CustomEvent;
     friend class V8ServiceWorkerMessageEventInternal;
 
-    explicit Symbol(v8::Local<v8::Private> privateSymbol)
-        : m_privateSymbol(privateSymbol) {}
+    Symbol(const v8::Eternal<v8::Private>& eternalSymbol, v8::Isolate* isolate)

[Yuki, 2017/03/30 07:25:41]

This looks a little tricky.  Taking v8::Local<v8::Private> seems straightforward
way to me, especially you're going to use ScopedPersistent for window.document.

[Yuki, 2017/03/30 07:25:42]

nit: Our convention is to take v8::Context / v8::Isolate as the first argument
following V8's style.

[peria, 2017/03/30 07:41:36]

Done.

+        : m_privateSymbol(eternalSymbol.Get(isolate)), m_isolate(isolate) {}
+
+    v8::Local<v8::Context> context() const {
+      return m_isolate->GetCurrentContext();
+    }
 
     // Only friend classes are allowed to use this API.
-    v8::Local<v8::Value> getFromMainWorld(ScriptState*, ScriptWrappable*);
+    v8::Local<v8::Value> getFromMainWorld(ScriptWrappable*);
 
     v8::Local<v8::Private> m_privateSymbol;
+    v8::Isolate* m_isolate;
   };
 
   static std::unique_ptr<V8PrivateProperty> create() {
     return WTF::wrapUnique(new V8PrivateProperty());
   }
 
-#define V8_PRIVATE_PROPERTY_DEFINE_GETTER(InterfaceName, KeyName)              \
-  static Symbol V8_PRIVATE_PROPERTY_GETTER_NAME(InterfaceName, KeyName)(       \
-      v8::Isolate * isolate) /* NOLINT(readability/naming/underscores) */      \
-  {                                                                            \
-    V8PrivateProperty* privateProp =                                           \
-        V8PerIsolateData::from(isolate)->privateProperty();                    \
-    if (UNLIKELY(privateProp                                                   \
-                     ->V8_PRIVATE_PROPERTY_MEMBER_NAME(InterfaceName, KeyName) \
-                     .IsEmpty())) {                                            \
-      privateProp->V8_PRIVATE_PROPERTY_MEMBER_NAME(InterfaceName, KeyName)     \
-          .Set(                                                                \
-              isolate,                                                         \
-              createV8Private(                                                 \
-                  isolate,                                                     \
-                  V8_PRIVATE_PROPERTY_SYMBOL_STRING(InterfaceName, KeyName),   \
-                  sizeof V8_PRIVATE_PROPERTY_SYMBOL_STRING(                    \
-                      InterfaceName,                                           \
-                      KeyName))); /* NOLINT(readability/naming/underscores) */ \
-    }                                                                          \
-    return Symbol(                                                             \
-        privateProp->V8_PRIVATE_PROPERTY_MEMBER_NAME(InterfaceName, KeyName)   \
-            .Get(isolate));                                                    \
-  }
+#define V8_PRIVATE_PROPERTY_DEFINE_GETTER(InterfaceName, KeyName)             \
+  static Symbol V8_PRIVATE_PROPERTY_GETTER_NAME(                              \

[peria, 2017/03/30 07:08:26]

A presubmit error warns for "V8_PRIVATE_PROPERTY_GETTER_NAME",
but we can't use a NOLINT comment in C-style comment.

[Yuki, 2017/03/30 07:25:41]

/* NOLINT(readability/naming/underscores) */
did work before.  Does this no longer work?

[peria, 2017/03/30 07:41:36]

Correct.  Presubmit checker works only for "// NOLINT" style.

+      InterfaceName, KeyName)(v8::Isolate * isolate) {                        \
+    V8PrivateProperty* privateProp =                                          \
+        V8PerIsolateData::from(isolate)->privateProperty();                   \
+    v8::Eternal<v8::Private>& eternalProperty =                               \

[Yuki, 2017/03/30 07:25:42]

nit: Hungarian notation is not recommended in general and in Google C++ coding
style, IIRC.  You don't need to include the type info in the name.  The name
should be given based on what it is or for what purpose it is.

[peria, 2017/03/30 07:41:37]

Done.

+        privateProp->V8_PRIVATE_PROPERTY_MEMBER_NAME(InterfaceName, KeyName); \
+    if (UNLIKELY(eternalProperty.IsEmpty())) {                                \
+      static constexpr char kSymbolName[] =                                   \
+          V8_PRIVATE_PROPERTY_SYMBOL_STRING(InterfaceName, KeyName);          \
+      eternalProperty.Set(isolate, createV8Private(isolate, kSymbolName,      \
+                                                   sizeof(kSymbolName)));     \
+    }                                                                         \
+    return Symbol(eternalProperty, isolate);                                  \
+  }  // NOLINT
+
   V8_PRIVATE_PROPERTY_FOR_EACH(V8_PRIVATE_PROPERTY_DEFINE_GETTER)
 #undef V8_PRIVATE_PROPERTY_DEFINE_GETTER