Use private symbols instead of hidden values

Private symbols use the same infrastructure as ES6 symbols, except that
they are private (i.e. proxies can't intercept them), and we're about to
remove hidden values from the api.

BUG=none
R=rdevlin.cronin@chromium.org

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

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

5 files changed, 124 insertions, 50 deletions

diff --git a/extensions/renderer/module_system.cc b/extensions/renderer/module_system.cc
index b828ca0..3e3da6a 100644
--- a/extensions/renderer/module_system.cc
+++ b/extensions/renderer/module_system.cc
@@ -161,10 +161,8 @@ ModuleSystem::ModuleSystem(ScriptContext* context, SourceMap* source_map)
 
   v8::Local<v8::Object> global(context->v8_context()->Global());
   v8::Isolate* isolate = context->isolate();
-  global->SetHiddenValue(ToV8StringUnsafe(isolate, kModulesField),
-                         v8::Object::New(isolate));
-  global->SetHiddenValue(ToV8StringUnsafe(isolate, kModuleSystem),
-                         v8::External::New(isolate, this));
+  SetPrivate(global, kModulesField, v8::Object::New(isolate));
+  SetPrivate(global, kModuleSystem, v8::External::New(isolate, this));
 
   gin::ModuleRegistry::From(context->v8_context())->AddObserver(this);
   if (context_->GetRenderFrame()) {
@@ -182,8 +180,8 @@ void ModuleSystem::Invalidate() {
   {
     v8::HandleScope scope(GetIsolate());
     v8::Local<v8::Object> global = context()->v8_context()->Global();
-    global->DeleteHiddenValue(ToV8StringUnsafe(GetIsolate(), kModulesField));
-    global->DeleteHiddenValue(ToV8StringUnsafe(GetIsolate(), kModuleSystem));
+    DeletePrivate(global, kModulesField);
+    DeletePrivate(global, kModuleSystem);
   }
 
   // Invalidate all active and clobbered NativeHandlers we own.
@@ -244,9 +242,9 @@ v8::Local<v8::Value> ModuleSystem::RequireForJsInner(
   // The module system might have been deleted. This can happen if a different
   // context keeps a reference to us, but our frame is destroyed (e.g.
   // background page keeps reference to chrome object in a closed popup).
-  v8::Local<v8::Value> modules_value = global->GetHiddenValue(
-      ToV8StringUnsafe(GetIsolate(), kModulesField));
-  if (modules_value.IsEmpty() || modules_value->IsUndefined()) {
+  v8::Local<v8::Value> modules_value;
+  if (!GetPrivate(global, kModulesField, &modules_value) ||
+      modules_value->IsUndefined()) {
     Warn(GetIsolate(), "Extension view no longer exists");
     return v8::Undefined(GetIsolate());
   }
@@ -383,17 +381,18 @@ void ModuleSystem::LazyFieldGetterInner(
     RequireFunction require_function) {
   CHECK(!info.Data().IsEmpty());
   CHECK(info.Data()->IsObject());
-  v8::HandleScope handle_scope(info.GetIsolate());
+  v8::Isolate* isolate = info.GetIsolate();
+  v8::HandleScope handle_scope(isolate);
   v8::Local<v8::Object> parameters = v8::Local<v8::Object>::Cast(info.Data());
   // This context should be the same as context()->v8_context().
   v8::Local<v8::Context> context = parameters->CreationContext();
   v8::Local<v8::Object> global(context->Global());
-  v8::Local<v8::Value> module_system_value = global->GetHiddenValue(
-      ToV8StringUnsafe(info.GetIsolate(), kModuleSystem));
-  if (module_system_value.IsEmpty() || !module_system_value->IsExternal()) {
+  v8::Local<v8::Value> module_system_value;
+  if (!GetPrivate(context, global, kModuleSystem, &module_system_value) ||
+      !module_system_value->IsExternal()) {
     // ModuleSystem has been deleted.
     // TODO(kalman): See comment in header file.
-    Warn(info.GetIsolate(),
+    Warn(isolate,
          "Module system has been deleted, does extension view exist?");
     return;
   }
@@ -403,7 +402,7 @@ void ModuleSystem::LazyFieldGetterInner(
 
   v8::Local<v8::Value> v8_module_name;
   if (!GetProperty(context, parameters, kModuleName, &v8_module_name)) {
-    Warn(info.GetIsolate(), "Cannot find module.");
+    Warn(isolate, "Cannot find module.");
     return;
   }
   std::string name = *v8::String::Utf8Value(v8_module_name);
@@ -413,7 +412,7 @@ void ModuleSystem::LazyFieldGetterInner(
   v8::Context::Scope context_scope(context);
   NativesEnabledScope natives_enabled_scope(module_system);
 
-  v8::TryCatch try_catch(info.GetIsolate());
+  v8::TryCatch try_catch(isolate);
   v8::Local<v8::Value> module_value;
   if (!(module_system->*require_function)(name).ToLocal(&module_value)) {
     module_system->HandleException(try_catch);
@@ -611,17 +610,15 @@ void ModuleSystem::Private(const v8::FunctionCallbackInfo<v8::Value>& args) {
     return;
   }
   v8::Local<v8::Object> obj = args[0].As<v8::Object>();
-  v8::Local<v8::String> privates_key =
-      ToV8StringUnsafe(GetIsolate(), "privates");
-  v8::Local<v8::Value> privates = obj->GetHiddenValue(privates_key);
-  if (privates.IsEmpty()) {
+  v8::Local<v8::Value> privates;
+  if (!GetPrivate(obj, "privates", &privates) || !privates->IsObject()) {
     privates = v8::Object::New(args.GetIsolate());
     if (privates.IsEmpty()) {
       GetIsolate()->ThrowException(
           ToV8StringUnsafe(GetIsolate(), "Failed to create privates"));
       return;
     }
-    obj->SetHiddenValue(privates_key, privates);
+    SetPrivate(obj, "privates", privates);
   }
   args.GetReturnValue().Set(privates);
 }
diff --git a/extensions/renderer/object_backed_native_handler.cc b/extensions/renderer/object_backed_native_handler.cc
index 45d2dd3..421b273 100644
--- a/extensions/renderer/object_backed_native_handler.cc
+++ b/extensions/renderer/object_backed_native_handler.cc
@@ -37,17 +37,17 @@ v8::Local<v8::Object> ObjectBackedNativeHandler::NewInstance() {
 // static
 void ObjectBackedNativeHandler::Router(
     const v8::FunctionCallbackInfo<v8::Value>& args) {
-  v8::HandleScope handle_scope(args.GetIsolate());
+  v8::Isolate* isolate = args.GetIsolate();
+  v8::HandleScope handle_scope(isolate);
   v8::Local<v8::Object> data = args.Data().As<v8::Object>();
+  v8::Local<v8::Context> context = isolate->GetCurrentContext();
 
-  v8::Local<v8::Value> handler_function_value =
-      data->GetHiddenValue(
-          v8::String::NewFromUtf8(args.GetIsolate(), kHandlerFunction));
+  v8::Local<v8::Value> handler_function_value;
   // See comment in header file for why we do this.
-  if (handler_function_value.IsEmpty() ||
+  if (!GetPrivate(context, data, kHandlerFunction, &handler_function_value) ||
       handler_function_value->IsUndefined()) {
-    ScriptContext* script_context = ScriptContextSet::GetContextByV8Context(
-        args.GetIsolate()->GetCurrentContext());
+    ScriptContext* script_context =
+        ScriptContextSet::GetContextByV8Context(context);
     console::Error(script_context ? script_context->GetRenderFrame() : nullptr,
                    "Extension view no longer exists");
     return;
@@ -67,9 +67,8 @@ void ObjectBackedNativeHandler::RouteFunction(
   v8::Context::Scope context_scope(context_->v8_context());
 
   v8::Local<v8::Object> data = v8::Object::New(isolate);
-  data->SetHiddenValue(
-      v8::String::NewFromUtf8(isolate, kHandlerFunction),
-      v8::External::New(isolate, new HandlerFunction(handler_function)));
+  SetPrivate(data, kHandlerFunction,
+             v8::External::New(isolate, new HandlerFunction(handler_function)));
   v8::Local<v8::FunctionTemplate> function_template =
       v8::FunctionTemplate::New(isolate, Router, data);
   v8::Local<v8::ObjectTemplate>::New(isolate, object_template_)
@@ -88,13 +87,11 @@ void ObjectBackedNativeHandler::Invalidate() {
 
   for (size_t i = 0; i < router_data_.Size(); i++) {
     v8::Local<v8::Object> data = router_data_.Get(i);
-    v8::Local<v8::Value> handler_function_value =
-        data->GetHiddenValue(
-            v8::String::NewFromUtf8(isolate, kHandlerFunction));
-    CHECK(!handler_function_value.IsEmpty());
+    v8::Local<v8::Value> handler_function_value;
+    CHECK(GetPrivate(data, kHandlerFunction, &handler_function_value));
     delete static_cast<HandlerFunction*>(
         handler_function_value.As<v8::External>()->Value());
-    data->DeleteHiddenValue(v8::String::NewFromUtf8(isolate, kHandlerFunction));
+    DeletePrivate(data, kHandlerFunction);
   }
 
   router_data_.Clear();
@@ -103,4 +100,56 @@ void ObjectBackedNativeHandler::Invalidate() {
   NativeHandler::Invalidate();
 }
 
+void ObjectBackedNativeHandler::SetPrivate(v8::Local<v8::Object> obj,
+                                           const char* key,
+                                           v8::Local<v8::Value> value) {
+  SetPrivate(context_->v8_context(), obj, key, value);
+}
+
+// static
+void ObjectBackedNativeHandler::SetPrivate(v8::Local<v8::Context> context,
+                                           v8::Local<v8::Object> obj,
+                                           const char* key,
+                                           v8::Local<v8::Value> value) {
+  obj->SetPrivate(context, v8::Private::ForApi(context->GetIsolate(),
+                                               v8::String::NewFromUtf8(
+                                                   context->GetIsolate(), key)),
+                  value)
+      .FromJust();
+}
+
+bool ObjectBackedNativeHandler::GetPrivate(v8::Local<v8::Object> obj,
+                                           const char* key,
+                                           v8::Local<v8::Value>* result) {
+  return GetPrivate(context_->v8_context(), obj, key, result);
+}
+
+// static
+bool ObjectBackedNativeHandler::GetPrivate(v8::Local<v8::Context> context,
+                                           v8::Local<v8::Object> obj,
+                                           const char* key,
+                                           v8::Local<v8::Value>* result) {
+  return obj->GetPrivate(context,
+                         v8::Private::ForApi(context->GetIsolate(),
+                                             v8::String::NewFromUtf8(
+                                                 context->GetIsolate(), key)))
+      .ToLocal(result);
+}
+
+void ObjectBackedNativeHandler::DeletePrivate(v8::Local<v8::Object> obj,
+                                              const char* key) {
+  DeletePrivate(context_->v8_context(), obj, key);
+}
+
+// static
+void ObjectBackedNativeHandler::DeletePrivate(v8::Local<v8::Context> context,
+                                              v8::Local<v8::Object> obj,
+                                              const char* key) {
+  obj->DeletePrivate(context,
+                     v8::Private::ForApi(
+                         context->GetIsolate(),
+                         v8::String::NewFromUtf8(context->GetIsolate(), key)))
+      .FromJust();
+}
+
 }  // namespace extensions
diff --git a/extensions/renderer/object_backed_native_handler.h b/extensions/renderer/object_backed_native_handler.h
index e4e13a5..a60a33a 100644
--- a/extensions/renderer/object_backed_native_handler.h
+++ b/extensions/renderer/object_backed_native_handler.h
@@ -49,6 +49,27 @@ class ObjectBackedNativeHandler : public NativeHandler {
 
   void Invalidate() override;
 
+  // The following methods are convenience wrappers for methods on v8::Object
+  // with the corresponding names.
+  void SetPrivate(v8::Local<v8::Object> obj,
+                  const char* key,
+                  v8::Local<v8::Value> value);
+  static void SetPrivate(v8::Local<v8::Context> context,
+                         v8::Local<v8::Object> obj,
+                         const char* key,
+                         v8::Local<v8::Value> value);
+  bool GetPrivate(v8::Local<v8::Object> obj,
+                  const char* key,
+                  v8::Local<v8::Value>* result);
+  static bool GetPrivate(v8::Local<v8::Context> context,
+                         v8::Local<v8::Object> obj,
+                         const char* key,
+                         v8::Local<v8::Value>* result);
+  void DeletePrivate(v8::Local<v8::Object> obj, const char* key);
+  static void DeletePrivate(v8::Local<v8::Context> context,
+                            v8::Local<v8::Object> obj,
+                            const char* key);
+
  private:
   // Callback for RouteFunction which routes the V8 call to the correct
   // base::Bound callback.
diff --git a/extensions/renderer/safe_builtins.cc b/extensions/renderer/safe_builtins.cc
index bf44e96..3bfa01a 100644
--- a/extensions/renderer/safe_builtins.cc
+++ b/extensions/renderer/safe_builtins.cc
@@ -125,23 +125,27 @@ const char kScript[] =
     "\n"
     "}());\n";
 
-v8::Local<v8::String> MakeKey(const char* name, v8::Isolate* isolate) {
-  return ToV8StringUnsafe(isolate,
-                          base::StringPrintf("%s::%s", kClassName, name));
+v8::Local<v8::Private> MakeKey(const char* name, v8::Isolate* isolate) {
+  return v8::Private::ForApi(
+      isolate, ToV8StringUnsafe(
+                   isolate, base::StringPrintf("%s::%s", kClassName, name)));
 }
 
 void SaveImpl(const char* name,
               v8::Local<v8::Value> value,
               v8::Local<v8::Context> context) {
   CHECK(!value.IsEmpty() && value->IsObject()) << name;
-  context->Global()->SetHiddenValue(MakeKey(name, context->GetIsolate()),
-                                    value);
+  context->Global()
+      ->SetPrivate(context, MakeKey(name, context->GetIsolate()), value)
+      .FromJust();
 }
 
 v8::Local<v8::Object> Load(const char* name, v8::Local<v8::Context> context) {
   v8::Local<v8::Value> value =
-      context->Global()->GetHiddenValue(MakeKey(name, context->GetIsolate()));
-  CHECK(!value.IsEmpty() && value->IsObject()) << name;
+      context->Global()
+          ->GetPrivate(context, MakeKey(name, context->GetIsolate()))
+          .ToLocalChecked();
+  CHECK(value->IsObject()) << name;
   return v8::Local<v8::Object>::Cast(value);
 }
 
diff --git a/extensions/renderer/wake_event_page.cc b/extensions/renderer/wake_event_page.cc
index 7a64d1a..ae98acc 100644
--- a/extensions/renderer/wake_event_page.cc
+++ b/extensions/renderer/wake_event_page.cc
@@ -115,12 +115,13 @@ v8::Local<v8::Function> WakeEventPage::GetForContext(ScriptContext* context) {
 
   // Cache the imported function as a hidden property on the global object of
   // |v8_context|. Creating it isn't free.
-  v8::Local<v8::String> kWakeEventPageKey =
-      ToV8StringUnsafe(isolate, "WakeEventPage");
-  v8::Local<v8::Value> wake_event_page =
-      v8_context->Global()->GetHiddenValue(kWakeEventPageKey);
-
-  if (wake_event_page.IsEmpty()) {
+  v8::Local<v8::Private> kWakeEventPageKey =
+      v8::Private::ForApi(isolate, ToV8StringUnsafe(isolate, "WakeEventPage"));
+  v8::Local<v8::Value> wake_event_page;
+  if (!v8_context->Global()
+           ->GetPrivate(v8_context, kWakeEventPageKey)
+           .ToLocal(&wake_event_page) ||
+      wake_event_page->IsUndefined()) {
     // Implement this using a NativeHandler, which requires a function name
     // (arbitrary in this case). Handles own lifetime.
     const char* kFunctionName = "WakeEventPage";
@@ -132,7 +133,9 @@ v8::Local<v8::Function> WakeEventPage::GetForContext(ScriptContext* context) {
     // Extract and cache the wake-event-page function from the native handler.
     wake_event_page = GetPropertyUnsafe(
         v8_context, native_handler->NewInstance(), kFunctionName);
-    v8_context->Global()->SetHiddenValue(kWakeEventPageKey, wake_event_page);
+    v8_context->Global()
+        ->SetPrivate(v8_context, kWakeEventPageKey, wake_event_page)
+        .FromJust();
   }
 
   CHECK(wake_event_page->IsFunction());