[Extension Bindings] Use v8::Object::SetLazyDataProperty for replaceable API properties.

This removes the custom workaround to emulate this behavior.

A unit test is adjusted to reflect the fact that a previously generated API binding
will continue to be returned.

Bug: 653596
Change-Id: Ib158164234e111180a00750ba736cd303be391c0
Reviewed-on: https://chromium-review.googlesource.com/867517
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532186}

extensions/renderer/native_extension_bindings_system.cc

@@ -325,27 +325,6 @@ v8::Local<v8::Object> CreateFullBinding(
   return root_binding;
 }
 
-// A setter for allowing scripts to override the binding on an object. This
-// records a new set value as a private property of the object, so that the
-// accessor can check for it and return it if present.
-// This would be unnecessary if there were a SetLazyDataProperty on v8::Object
-// (rather than just v8::Template).
-void BindingSetter(v8::Local<v8::Name> property,
-                   v8::Local<v8::Value> value,
-                   const v8::PropertyCallbackInfo<void>& info) {
-  v8::Isolate* isolate = info.GetIsolate();
-  v8::HandleScope handle_scope(isolate);
-  v8::Local<v8::Object> object = info.Holder();
-  v8::Local<v8::Context> context = object->CreationContext();
-
-  v8::Local<v8::String> api_name = info.Data().As<v8::String>();
-
-  v8::Maybe<bool> success = object->SetPrivate(
-      context, v8::Private::ForApi(isolate, api_name), value);
-  DCHECK(success.IsJust());
-  DCHECK(success.FromJust());
-}
-
 }  // namespace
 
 NativeExtensionBindingsSystem::NativeExtensionBindingsSystem(
@@ -440,8 +419,8 @@ void NativeExtensionBindingsSystem::UpdateBindingsForContext(
                        v8_context](base::StringPiece accessor_name) {
     v8::Local<v8::String> api_name =
         gin::StringToSymbol(isolate, accessor_name);
-    v8::Maybe<bool> success = chrome->SetAccessor(
-        v8_context, api_name, &BindingAccessor, &BindingSetter, api_name);
+    v8::Maybe<bool> success = chrome->SetLazyDataProperty(
+        v8_context, api_name, &BindingAccessor, api_name);
     return success.IsJust() && success.FromJust();
   };
 
@@ -586,31 +565,11 @@ void NativeExtensionBindingsSystem::BindingAccessor(
     const v8::PropertyCallbackInfo<v8::Value>& info) {
   v8::Isolate* isolate = info.GetIsolate();
   v8::HandleScope handle_scope(isolate);
-  v8::Local<v8::Object> object = info.Holder();
-  v8::Local<v8::Context> context = object->CreationContext();
+  v8::Local<v8::Context> context = info.Holder()->CreationContext();
 
   // We use info.Data() to store a real name here instead of using the provided
   // one to handle any weirdness from the caller (non-existent strings, etc).
   v8::Local<v8::String> api_name = info.Data().As<v8::String>();
-
-  v8::Local<v8::Private> key = v8::Private::ForApi(isolate, api_name);
-  v8::Maybe<bool> has_private = object->HasPrivate(context, key);
-  if (!has_private.IsJust()) {
-    NOTREACHED();
-    return;
-  }
-
-  if (has_private.FromJust()) {
-    v8::Local<v8::Value> overridden_value;
-    if (!object->GetPrivate(context, v8::Private::ForApi(isolate, api_name))
-             .ToLocal(&overridden_value)) {
-      NOTREACHED();
-      return;
-    }
-    info.GetReturnValue().Set(overridden_value);
-    return;
-  }
-
   v8::Local<v8::Object> binding = GetAPIHelper(context, api_name);
   if (!binding.IsEmpty())
     info.GetReturnValue().Set(binding);

extensions/renderer/native_extension_bindings_system_unittest.cc

@@ -217,7 +217,7 @@ TEST_F(NativeExtensionBindingsSystemUnittest, APIObjectsAreEqual) {
 }
 
 // Tests that referencing APIs after the context data is disposed is safe (and
-// returns undefined).
+// returns undefined if not yet instantiated).
 TEST_F(NativeExtensionBindingsSystemUnittest,
        ReferencingAPIAfterDisposingContext) {
   scoped_refptr<Extension> extension =
@@ -247,8 +247,7 @@ TEST_F(NativeExtensionBindingsSystemUnittest,
     // Check an API that was instantiated....
     v8::Local<v8::Value> second_idle_object =
         V8ValueFromScriptSource(context, "chrome.idle");
-    ASSERT_FALSE(second_idle_object.IsEmpty());
-    EXPECT_TRUE(second_idle_object->IsUndefined());
+    EXPECT_EQ(first_idle_object, second_idle_object);
     // ... and also one that wasn't.
     v8::Local<v8::Value> power_object =
         V8ValueFromScriptSource(context, "chrome.power");