Reland v8::Private and related APIs

Also deprecate hidden values

BUG=none
LOG=y
R=rossberg@chromium.org,yangguo@chromium.org

Review URL: https://codereview.chromium.org/1428793002

Cr-Commit-Position: refs/heads/master@{#31658}

include/v8.h

@@ -103,6 +103,7 @@ class String;
 class StringObject;
 class Symbol;
 class SymbolObject;
+class Private;
 class Uint32;
 class Utils;
 class Value;
@@ -311,6 +312,7 @@ class Local {
   friend class String;
   friend class Object;
   friend class Context;
+  friend class Private;
   template<class F> friend class internal::CustomArguments;
   friend Local<Primitive> Undefined(Isolate* isolate);
   friend Local<Primitive> Null(Isolate* isolate);
@@ -2482,6 +2484,34 @@ class V8_EXPORT Symbol : public Name {
 };
 
 
+/**
+ * A private symbol
+ *
+ * This is an experimental feature. Use at your own risk.
+ */
+class V8_EXPORT Private : public Data {
+ public:
+  // Returns the print name string of the private symbol, or undefined if none.
+  Local<Value> Name() const;
+
+  // Create a private symbol. If name is not empty, it will be the description.
+  static Local<Private> New(Isolate* isolate,
+                            Local<String> name = Local<String>());
+
+  // Retrieve a global private symbol. If a symbol with this name has not
+  // been retrieved in the same isolate before, it is created.
+  // Note that private symbols created this way are never collected, so
+  // they should only be used for statically fixed properties.
+  // Also, there is only one global name space for the names used as keys.
+  // To minimize the potential for clashes, use qualified names as keys,
+  // e.g., "Class#property".
+  static Local<Private> ForApi(Isolate* isolate, Local<String> name);
+
+ private:
+  Private();
+};
+
+
 /**
  * A JavaScript number value (ECMA-262, 4.3.20)
  */
@@ -2709,6 +2739,18 @@ class V8_EXPORT Object : public Value {
                            PropertyAttribute attribute = None,
                            AccessControl settings = DEFAULT);
 
+  /**
+   * Functionality for private properties.
+   * This is an experimental feature, use at your own risk.
+   * Note: Private properties are not inherited. Do not rely on this, since it
+   * may change.
+   */
+  Maybe<bool> HasPrivate(Local<Context> context, Local<Private> key);
+  Maybe<bool> SetPrivate(Local<Context> context, Local<Private> key,
+                         Local<Value> value);
+  Maybe<bool> DeletePrivate(Local<Context> context, Local<Private> key);
+  MaybeLocal<Value> GetPrivate(Local<Context> context, Local<Private> key);
+
   /**
    * Returns an array containing the names of the enumerable properties
    * of this object, including properties from prototype objects.  The
@@ -2877,16 +2919,12 @@ class V8_EXPORT Object : public Value {
    */
   int GetIdentityHash();
 
-  /**
-   * Access hidden properties on JavaScript objects. These properties are
-   * hidden from the executing JavaScript and only accessible through the V8
-   * C++ API. Hidden properties introduced by V8 internally (for example the
-   * identity hash) are prefixed with "v8::".
-   */
-  // TODO(dcarney): convert these to take a isolate and optionally bailout?
-  bool SetHiddenValue(Local<String> key, Local<Value> value);
-  Local<Value> GetHiddenValue(Local<String> key);
-  bool DeleteHiddenValue(Local<String> key);
+  V8_DEPRECATE_SOON("Use v8::Object::SetPrivate instead.",
+                    bool SetHiddenValue(Local<String> key, Local<Value> value));
+  V8_DEPRECATE_SOON("Use v8::Object::GetHidden instead.",
+                    Local<Value> GetHiddenValue(Local<String> key));
+  V8_DEPRECATE_SOON("Use v8::Object::DeletePrivate instead.",
+                    bool DeleteHiddenValue(Local<String> key));
 
   /**
    * Clone this object with a fast but shallow copy.  Values will point

src/api.cc

@@ -3603,6 +3603,13 @@ bool v8::Object::ForceSet(v8::Local<Value> key, v8::Local<Value> value,
 }
 
 
+Maybe<bool> v8::Object::SetPrivate(Local<Context> context, Local<Private> key,
+                                   Local<Value> value) {
+  return DefineOwnProperty(context, Local<Name>(reinterpret_cast<Name*>(*key)),
+                           value, DontEnum);
+}
+
+
 MaybeLocal<Value> v8::Object::Get(Local<v8::Context> context,
                                   Local<Value> key) {
   PREPARE_FOR_EXECUTION(context, "v8::Object::Get()", Value);
@@ -3639,6 +3646,12 @@ Local<Value> v8::Object::Get(uint32_t index) {
 }
 
 
+MaybeLocal<Value> v8::Object::GetPrivate(Local<Context> context,
+                                         Local<Private> key) {
+  return Get(context, Local<Value>(reinterpret_cast<Value*>(*key)));
+}
+
+
 Maybe<PropertyAttribute> v8::Object::GetPropertyAttributes(
     Local<Context> context, Local<Value> key) {
   PREPARE_FOR_EXECUTION_PRIMITIVE(
@@ -3875,6 +3888,12 @@ bool v8::Object::Delete(v8::Local<Value> key) {
 }
 
 
+Maybe<bool> v8::Object::DeletePrivate(Local<Context> context,
+                                      Local<Private> key) {
+  return Delete(context, Local<Value>(reinterpret_cast<Value*>(*key)));
+}
+
+
 Maybe<bool> v8::Object::Has(Local<Context> context, Local<Value> key) {
   PREPARE_FOR_EXECUTION_PRIMITIVE(context, "v8::Object::Get()", bool);
   auto self = Utils::OpenHandle(this);
@@ -3903,6 +3922,11 @@ bool v8::Object::Has(v8::Local<Value> key) {
 }
 
 
+Maybe<bool> v8::Object::HasPrivate(Local<Context> context, Local<Private> key) {
+  return HasOwnProperty(context, Local<Name>(reinterpret_cast<Name*>(*key)));
+}
+
+
 Maybe<bool> v8::Object::Delete(Local<Context> context, uint32_t index) {
   PREPARE_FOR_EXECUTION_PRIMITIVE(context, "v8::Object::DeleteProperty()",
                                   bool);
@@ -4235,13 +4259,16 @@ int v8::Object::GetIdentityHash() {
 bool v8::Object::SetHiddenValue(v8::Local<v8::String> key,
                                 v8::Local<v8::Value> value) {
   i::Isolate* isolate = Utils::OpenHandle(this)->GetIsolate();
-  if (value.IsEmpty()) return DeleteHiddenValue(key);
   ENTER_V8(isolate);
   i::HandleScope scope(isolate);
   i::Handle<i::JSObject> self = Utils::OpenHandle(this);
   i::Handle<i::String> key_obj = Utils::OpenHandle(*key);
   i::Handle<i::String> key_string =
       isolate->factory()->InternalizeString(key_obj);
+  if (value.IsEmpty()) {
+    i::JSObject::DeleteHiddenProperty(self, key_string);
+    return true;
+  }
   i::Handle<i::Object> value_obj = Utils::OpenHandle(*value);
   i::Handle<i::Object> result =
       i::JSObject::SetHiddenProperty(self, key_string, value_obj);
@@ -5246,6 +5273,11 @@ Local<Value> Symbol::Name() const {
 }
 
 
+Local<Value> Private::Name() const {
+  return reinterpret_cast<const Symbol*>(this)->Name();
+}
+
+
 double Number::Value() const {
   i::Handle<i::Object> obj = Utils::OpenHandle(this);
   return obj->Number();
@@ -6818,7 +6850,8 @@ Local<Symbol> v8::Symbol::New(Isolate* isolate, Local<String> name) {
 
 static i::Handle<i::Symbol> SymbolFor(i::Isolate* isolate,
                                       i::Handle<i::String> name,
-                                      i::Handle<i::String> part) {
+                                      i::Handle<i::String> part,
+                                      bool private_symbol) {
   i::Handle<i::JSObject> registry = isolate->GetSymbolRegistry();
   i::Handle<i::JSObject> symbols =
       i::Handle<i::JSObject>::cast(
@@ -6827,7 +6860,10 @@ static i::Handle<i::Symbol> SymbolFor(i::Isolate* isolate,
       i::Object::GetPropertyOrElement(symbols, name).ToHandleChecked();
   if (!symbol->IsSymbol()) {
     DCHECK(symbol->IsUndefined());
-    symbol = isolate->factory()->NewSymbol();
+    if (private_symbol)
+      symbol = isolate->factory()->NewPrivateSymbol();
+    else
+      symbol = isolate->factory()->NewSymbol();
     i::Handle<i::Symbol>::cast(symbol)->set_name(*name);
     i::JSObject::SetProperty(symbols, name, symbol, i::STRICT).Assert();
   }
@@ -6839,7 +6875,7 @@ Local<Symbol> v8::Symbol::For(Isolate* isolate, Local<String> name) {
   i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
   i::Handle<i::String> i_name = Utils::OpenHandle(*name);
   i::Handle<i::String> part = i_isolate->factory()->for_string();
-  return Utils::ToLocal(SymbolFor(i_isolate, i_name, part));
+  return Utils::ToLocal(SymbolFor(i_isolate, i_name, part, false));
 }
 
 
@@ -6847,7 +6883,7 @@ Local<Symbol> v8::Symbol::ForApi(Isolate* isolate, Local<String> name) {
   i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
   i::Handle<i::String> i_name = Utils::OpenHandle(*name);
   i::Handle<i::String> part = i_isolate->factory()->for_api_string();
-  return Utils::ToLocal(SymbolFor(i_isolate, i_name, part));
+  return Utils::ToLocal(SymbolFor(i_isolate, i_name, part, false));
 }
 
 
@@ -6875,6 +6911,27 @@ Local<Symbol> v8::Symbol::GetIsConcatSpreadable(Isolate* isolate) {
 }
 
 
+Local<Private> v8::Private::New(Isolate* isolate, Local<String> name) {
+  i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
+  LOG_API(i_isolate, "Private::New()");
+  ENTER_V8(i_isolate);
+  i::Handle<i::Symbol> symbol = i_isolate->factory()->NewPrivateSymbol();
+  if (!name.IsEmpty()) symbol->set_name(*Utils::OpenHandle(*name));
+  Local<Symbol> result = Utils::ToLocal(symbol);
+  return v8::Local<Private>(reinterpret_cast<Private*>(*result));
+}
+
+
+Local<Private> v8::Private::ForApi(Isolate* isolate, Local<String> name) {
+  i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
+  i::Handle<i::String> i_name = Utils::OpenHandle(*name);
+  i::Handle<i::String> part = i_isolate->factory()->private_api_string();
+  Local<Symbol> result =
+      Utils::ToLocal(SymbolFor(i_isolate, i_name, part, true));
+  return v8::Local<Private>(reinterpret_cast<Private*>(*result));
+}
+
+
 Local<Number> v8::Number::New(Isolate* isolate, double value) {
   i::Isolate* internal_isolate = reinterpret_cast<i::Isolate*>(isolate);
   if (std::isnan(value)) {

src/factory.cc

@@ -708,14 +708,9 @@ Handle<Symbol> Factory::NewSymbol() {
 }
 
 
-Handle<Symbol> Factory::NewPrivateSymbol(Handle<Object> name) {
+Handle<Symbol> Factory::NewPrivateSymbol() {
   Handle<Symbol> symbol = NewSymbol();
   symbol->set_is_private(true);
-  if (name->IsString()) {
-    symbol->set_name(*name);
-  } else {
-    DCHECK(name->IsUndefined());
-  }
   return symbol;
 }
 

src/factory.h

@@ -217,7 +217,7 @@ class Factory final {
 
   // Create a symbol.
   Handle<Symbol> NewSymbol();
-  Handle<Symbol> NewPrivateSymbol(Handle<Object> name);
+  Handle<Symbol> NewPrivateSymbol();
 
   // Create a global (but otherwise uninitialized) context.
   Handle<Context> NewNativeContext();

src/heap/heap.cc

@@ -2659,11 +2659,12 @@ void Heap::CreateInitialObjects() {
 
   {
     HandleScope scope(isolate());
-#define SYMBOL_INIT(name)                                                   \
-  {                                                                         \
-    Handle<String> name##d = factory->NewStringFromStaticChars(#name);      \
-    Handle<Object> symbol(isolate()->factory()->NewPrivateSymbol(name##d)); \
-    roots_[k##name##RootIndex] = *symbol;                                   \
+#define SYMBOL_INIT(name)                                              \
+  {                                                                    \
+    Handle<String> name##d = factory->NewStringFromStaticChars(#name); \
+    Handle<Symbol> symbol(isolate()->factory()->NewPrivateSymbol());   \
+    symbol->set_name(*name##d);                                        \
+    roots_[k##name##RootIndex] = *symbol;                              \
   }
     PRIVATE_SYMBOL_LIST(SYMBOL_INIT)
 #undef SYMBOL_INIT

src/heap/heap.h

@@ -277,6 +277,7 @@ namespace internal {
   V(Number_string, "Number")                               \
   V(object_string, "object")                               \
   V(Object_string, "Object")                               \
+  V(private_api_string, "private_api")                     \
   V(proto_string, "__proto__")                             \
   V(prototype_string, "prototype")                         \
   V(query_colon_string, "(?:)")                            \

src/isolate.cc

@@ -2585,6 +2585,7 @@ Handle<JSObject> Isolate::GetSymbolRegistry() {
     SetUpSubregistry(registry, map, "for");
     SetUpSubregistry(registry, map, "for_api");
     SetUpSubregistry(registry, map, "keyFor");
+    SetUpSubregistry(registry, map, "private_api");
   }
   return Handle<JSObject>::cast(factory()->symbol_registry());
 }

src/profiler/heap-snapshot-generator.cc

@@ -836,7 +836,10 @@ HeapEntry* V8HeapExplorer::AddEntry(HeapObject* object) {
                     HeapEntry::kString,
                     names_->GetName(String::cast(object)));
   } else if (object->IsSymbol()) {
-    return AddEntry(object, HeapEntry::kSymbol, "symbol");
+    if (Symbol::cast(object)->is_private())
+      return AddEntry(object, HeapEntry::kHidden, "private symbol");
+    else
+      return AddEntry(object, HeapEntry::kSymbol, "symbol");
   } else if (object->IsCode()) {
     return AddEntry(object, HeapEntry::kCode, "");
   } else if (object->IsSharedFunctionInfo()) {

src/runtime/runtime-symbol.cc

@@ -28,7 +28,9 @@ RUNTIME_FUNCTION(Runtime_CreatePrivateSymbol) {
   DCHECK(args.length() == 1);
   CONVERT_ARG_HANDLE_CHECKED(Object, name, 0);
   RUNTIME_ASSERT(name->IsString() || name->IsUndefined());
-  return *isolate->factory()->NewPrivateSymbol(name);
+  Handle<Symbol> symbol = isolate->factory()->NewPrivateSymbol();
+  if (name->IsString()) symbol->set_name(*name);
+  return *symbol;
 }
 
 

test/cctest/test-api-interceptors.cc

@@ -109,7 +109,11 @@ void StringInterceptorGetter(
     if (name_str[i] != prefix[i]) return;
   }
   Handle<Object> self = Handle<Object>::Cast(info.This());
-  info.GetReturnValue().Set(self->GetHiddenValue(v8_str(name_str + i)));
+  info.GetReturnValue().Set(
+      self->GetPrivate(
+              info.GetIsolate()->GetCurrentContext(),
+              v8::Private::ForApi(info.GetIsolate(), v8_str(name_str + i)))
+          .ToLocalChecked());
 }
 
 
@@ -128,7 +132,9 @@ void StringInterceptorSetter(Local<String> name, Local<Value> value,
 
   if (value->IsInt32() && value->Int32Value() < 10000) {
     Handle<Object> self = Handle<Object>::Cast(info.This());
-    self->SetHiddenValue(name, value);
+    Handle<Context> context = info.GetIsolate()->GetCurrentContext();
+    Handle<v8::Private> symbol = v8::Private::ForApi(info.GetIsolate(), name);
+    self->SetPrivate(context, symbol, value).FromJust();
     info.GetReturnValue().Set(value);
   }
 }
@@ -1314,7 +1320,8 @@ THREADED_TEST(HiddenPropertiesWithInterceptors) {
 
   interceptor_for_hidden_properties_called = false;
 
-  v8::Local<v8::String> key = v8_str("api-test::hidden-key");
+  v8::Local<v8::Private> key =
+      v8::Private::New(isolate, v8_str("api-test::hidden-key"));
 
   // Associate an interceptor with an object and start setting hidden values.
   Local<v8::FunctionTemplate> fun_templ = v8::FunctionTemplate::New(isolate);
@@ -1323,8 +1330,11 @@ THREADED_TEST(HiddenPropertiesWithInterceptors) {
       v8::NamedPropertyHandlerConfiguration(InterceptorForHiddenProperties));
   Local<v8::Function> function = fun_templ->GetFunction();
   Local<v8::Object> obj = function->NewInstance();
-  CHECK(obj->SetHiddenValue(key, v8::Integer::New(isolate, 2302)));
-  CHECK_EQ(2302, obj->GetHiddenValue(key)->Int32Value());
+  CHECK(obj->SetPrivate(context.local(), key, v8::Integer::New(isolate, 2302))
+            .FromJust());
+  CHECK_EQ(
+      2302,
+      obj->GetPrivate(context.local(), key).ToLocalChecked()->Int32Value());
   CHECK(!interceptor_for_hidden_properties_called);
 }
 

test/cctest/test-debug.cc

@@ -4652,9 +4652,11 @@ TEST(NoHiddenProperties) {
   v8::Local<v8::Object> obj = v8::Local<v8::Object>::Cast(
       env->Global()->Get(v8::String::NewFromUtf8(isolate, "obj")));
   // Set a hidden property on the object.
-  obj->SetHiddenValue(
-      v8::String::NewFromUtf8(isolate, "v8::test-debug::a"),
-      v8::Int32::New(isolate, 11));
+  obj->SetPrivate(env.context(),
+                  v8::Private::New(isolate, v8::String::NewFromUtf8(
+                                                isolate, "v8::test-debug::a")),
+                  v8::Int32::New(isolate, 11))
+      .FromJust();
 
   // Get mirror for the object with property getter.
   CompileRun("var obj_mirror = debug.MakeMirror(obj);");
@@ -4681,18 +4683,23 @@ TEST(NoHiddenProperties) {
   // Create proto objects, add hidden properties to them and set them on
   // the global object.
   v8::Handle<v8::Object> protoObj = t0->GetFunction()->NewInstance();
-  protoObj->SetHiddenValue(
-      v8::String::NewFromUtf8(isolate, "v8::test-debug::b"),
-      v8::Int32::New(isolate, 12));
+  protoObj->SetPrivate(
+              env.context(),
+              v8::Private::New(isolate, v8::String::NewFromUtf8(
+                                            isolate, "v8::test-debug::b")),
+              v8::Int32::New(isolate, 12))
+      .FromJust();
   env->Global()->Set(v8::String::NewFromUtf8(isolate, "protoObj"),
                      protoObj);
   v8::Handle<v8::Object> grandProtoObj = t1->GetFunction()->NewInstance();
-  grandProtoObj->SetHiddenValue(
-      v8::String::NewFromUtf8(isolate, "v8::test-debug::c"),
-      v8::Int32::New(isolate, 13));
-  env->Global()->Set(
-      v8::String::NewFromUtf8(isolate, "grandProtoObj"),
-      grandProtoObj);
+  grandProtoObj->SetPrivate(
+                   env.context(),
+                   v8::Private::New(isolate, v8::String::NewFromUtf8(
+                                                 isolate, "v8::test-debug::c")),
+                   v8::Int32::New(isolate, 13))
+      .FromJust();
+  env->Global()->Set(v8::String::NewFromUtf8(isolate, "grandProtoObj"),
+                     grandProtoObj);
 
   // Setting prototypes: obj->protoObj->grandProtoObj
   protoObj->Set(v8::String::NewFromUtf8(isolate, "__proto__"),

test/cctest/test-dictionary.cc

@@ -171,12 +171,6 @@ static void TestHashSetCausesGC(Handle<HashSet> table) {
   Factory* factory = isolate->factory();
 
   Handle<JSObject> key = factory->NewJSArray(0);
-  v8::Handle<v8::Object> key_obj = v8::Utils::ToLocal(key);
-
-  // Force allocation of hash table backing store for hidden properties.
-  key_obj->SetHiddenValue(v8_str("key 1"), v8_str("val 1"));
-  key_obj->SetHiddenValue(v8_str("key 2"), v8_str("val 2"));
-  key_obj->SetHiddenValue(v8_str("key 3"), v8_str("val 3"));
 
   // Simulate a full heap so that generating an identity hash code
   // in subsequent calls will request GC.
@@ -208,12 +202,6 @@ static void TestHashMapCausesGC(Handle<HashMap> table) {
   Factory* factory = isolate->factory();
 
   Handle<JSObject> key = factory->NewJSArray(0);
-  v8::Handle<v8::Object> key_obj = v8::Utils::ToLocal(key);
-
-  // Force allocation of hash table backing store for hidden properties.
-  key_obj->SetHiddenValue(v8_str("key 1"), v8_str("val 1"));
-  key_obj->SetHiddenValue(v8_str("key 2"), v8_str("val 2"));
-  key_obj->SetHiddenValue(v8_str("key 3"), v8_str("val 3"));
 
   // Simulate a full heap so that generating an identity hash code
   // in subsequent calls will request GC.

test/cctest/test-heap-profiler.cc

@@ -1983,21 +1983,24 @@ TEST(HiddenPropertiesFastCase) {
       GetProperty(global, v8::HeapGraphEdge::kProperty, "c");
   CHECK(c);
   const v8::HeapGraphNode* hidden_props =
-      GetProperty(c, v8::HeapGraphEdge::kInternal, "hidden_properties");
+      GetProperty(c, v8::HeapGraphEdge::kProperty, "<symbol>");
   CHECK(!hidden_props);
 
   v8::Handle<v8::Value> cHandle =
       env->Global()->Get(v8::String::NewFromUtf8(env->GetIsolate(), "c"));
   CHECK(!cHandle.IsEmpty() && cHandle->IsObject());
-  cHandle->ToObject(isolate)->SetHiddenValue(v8_str("key"), v8_str("val"));
+  cHandle->ToObject(isolate)
+      ->SetPrivate(env.local(),
+                   v8::Private::ForApi(env->GetIsolate(), v8_str("key")),
+                   v8_str("val"))
+      .FromJust();
 
   snapshot = heap_profiler->TakeHeapSnapshot();
   CHECK(ValidateSnapshot(snapshot));
   global = GetGlobalObject(snapshot);
   c = GetProperty(global, v8::HeapGraphEdge::kProperty, "c");
   CHECK(c);
-  hidden_props = GetProperty(c, v8::HeapGraphEdge::kInternal,
-      "hidden_properties");
+  hidden_props = GetProperty(c, v8::HeapGraphEdge::kProperty, "<symbol>");
   CHECK(hidden_props);
 }
 

test/cctest/test-object-observe.cc

@@ -687,7 +687,10 @@ TEST(HiddenPropertiesLeakage) {
           ->Get(v8::Isolate::GetCurrent()->GetCurrentContext(), v8_str("obj"))
           .ToLocalChecked();
   Local<Object>::Cast(obj)
-      ->SetHiddenValue(v8_str("foo"), Null(CcTest::isolate()));
+      ->SetPrivate(v8::Isolate::GetCurrent()->GetCurrentContext(),
+                   v8::Private::New(CcTest::isolate(), v8_str("foo")),
+                   Null(CcTest::isolate()))
+      .FromJust();
   CompileRun("");  // trigger delivery
   CHECK(CompileRun("records")->IsNull());
 }