[Bindings] Simplify V8PrivateProperty::Symbol methods

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

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef V8PrivateProperty_h
 #define V8PrivateProperty_h
 
 #include <memory>
 
 #include "bindings/core/v8/ScriptPromiseProperties.h"
 #include "bindings/core/v8/V8BindingMacros.h"
 #include "bindings/core/v8/V8PerIsolateData.h"
 #include "core/CoreExport.h"
 #include "v8/include/v8.h"
 #include "wtf/Allocator.h"
 #include "wtf/PtrUtil.h"
 
 namespace blink {
 
-class ScriptState;
 class ScriptWrappable;
 
 // Apply |X| for each pair of (InterfaceName, PrivateKeyName).
 #define V8_PRIVATE_PROPERTY_FOR_EACH(X)               \
   X(CustomEvent, Detail)                              \
   X(CustomElement, AdoptedCallback)                   \
   X(CustomElement, AttributeChangedCallback)          \
   X(CustomElement, ConnectedCallback)                 \
   X(CustomElement, DisconnectedCallback)              \
   X(CustomElement, Document)                          \
@@ skipping 50 matching lines @@
  public:
   // Provides fast access to V8's private properties.
   //
   // Retrieving/creating a global private symbol from a string is very
   // expensive compared to get or set a private property.  This class
   // provides a way to cache a private symbol and re-use it.
   class CORE_EXPORT Symbol {
     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;
     // The following classes are exceptionally allowed to call to
     // getFromMainWorld.
     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
 
  private:
   V8PrivateProperty() {}
 
   static v8::Local<v8::Private> createV8Private(v8::Isolate*,
                                                 const char* symbol,
                                                 size_t length);
 
 #define V8_PRIVATE_PROPERTY_DECLARE_MEMBER(InterfaceName, KeyName) \
   v8::Eternal<v8::Private> V8_PRIVATE_PROPERTY_MEMBER_NAME(        \
       InterfaceName, KeyName);  // NOLINT(readability/naming/underscores)
   V8_PRIVATE_PROPERTY_FOR_EACH(V8_PRIVATE_PROPERTY_DECLARE_MEMBER)
 #undef V8_PRIVATE_PROPERTY_DECLARE_MEMBER
 };
 
 }  // namespace blink
 
 #endif  // V8PrivateProperty_h