[inspector] do not call native accessor in Rumtime.getProperties

In current implementation Object.getOwnPropertyDescriptor calls native
getter. It can produce side effects. We can avoid calling it.
DevTools frontend will show clickable dots and on click returns value.
This CL does not affect Blink and only affect several Node.js
properties, e.g. process.title.

R=yangguo@chromium.org

Bug: v8:6945
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_chromium_rel_ng
Change-Id: I5764c779ceed4d50832edf68b2b4c6ee2c2dd65c
Reviewed-on: https://chromium-review.googlesource.com/754223
Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49152}

src/api.cc

@@ -10194,6 +10194,41 @@ void debug::SetReturnValue(v8::Isolate* v8_isolate,
   isolate->debug()->set_return_value(*Utils::OpenHandle(*value));
 }
 
+int debug::GetNativeAccessorDescriptor(v8::Local<v8::Context> context,
+                                       v8::Local<v8::Object> v8_object,
+                                       v8::Local<v8::Name> v8_name) {
+  i::Handle<i::JSReceiver> object = Utils::OpenHandle(*v8_object);
+  i::Handle<i::Name> name = Utils::OpenHandle(*v8_name);
+  uint32_t index;
+  if (name->AsArrayIndex(&index)) {
+    return static_cast<int>(debug::NativeAccessorType::None);
+  }
+  i::LookupIterator it =
+      i::LookupIterator(object, name, i::LookupIterator::OWN);
+  if (!it.IsFound()) return static_cast<int>(debug::NativeAccessorType::None);
+  if (it.state() != i::LookupIterator::ACCESSOR) {
+    return static_cast<int>(debug::NativeAccessorType::None);
+  }
+  i::Handle<i::Object> structure = it.GetAccessors();
+  if (!structure->IsAccessorInfo()) {
+    return static_cast<int>(debug::NativeAccessorType::None);
+  }
+  auto isolate = reinterpret_cast<i::Isolate*>(context->GetIsolate());
+  int result = 0;
+#define IS_BUILTIN_ACESSOR(name, _)                         \
+  if (*structure == *isolate->factory()->name##_accessor()) \
+    result |= static_cast<int>(debug::NativeAccessorType::IsBuiltin);
+  ACCESSOR_INFO_LIST(IS_BUILTIN_ACESSOR)
+#undef IS_BUILTIN_ACESSOR
+  i::Handle<i::AccessorInfo> accessor_info =
+      i::Handle<i::AccessorInfo>::cast(structure);
+  if (accessor_info->getter())
+    result |= static_cast<int>(debug::NativeAccessorType::HasGetter);
+  if (accessor_info->setter())
+    result |= static_cast<int>(debug::NativeAccessorType::HasSetter);
+  return result;
+}
+
 Local<String> CpuProfileNode::GetFunctionName() const {
   const i::ProfileNode* node = reinterpret_cast<const i::ProfileNode*>(this);
   i::Isolate* isolate = node->isolate();

src/debug/debug-interface.h

@@ -487,6 +487,17 @@ void GlobalLexicalScopeNames(v8::Local<v8::Context> context,
 
 void SetReturnValue(v8::Isolate* isolate, v8::Local<v8::Value> value);
 
+enum class NativeAccessorType {
+  None = 0,
+  HasGetter = 1 << 0,
+  HasSetter = 1 << 1,
+  IsBuiltin = 1 << 2
+};
+
+int GetNativeAccessorDescriptor(v8::Local<v8::Context> context,
+                                v8::Local<v8::Object> object,
+                                v8::Local<v8::Name> name);
+
 }  // namespace debug
 }  // namespace v8
 

src/inspector/injected-script-source.js

@@ -380,9 +380,18 @@ InjectedScript.prototype = {
 
                 var descriptor;
                 try {
-                    descriptor = InjectedScriptHost.getOwnPropertyDescriptor(o, property);
-                    if (descriptor) {
-                        InjectedScriptHost.nullifyPrototype(descriptor);
+                    var nativeAccessorDescriptor = InjectedScriptHost.nativeAccessorDescriptor(o, property);
+                    if (nativeAccessorDescriptor && !nativeAccessorDescriptor.isBuiltin) {
+                        descriptor = { __proto__: null };
+                        if (nativeAccessorDescriptor.hasGetter)
+                            descriptor.get = function nativeGetter() { return o[property]; };
+                        if (nativeAccessorDescriptor.hasSetter)
+                            descriptor.set = function nativeSetter(v) { o[property] = v; };
+                    } else {
+                        descriptor = InjectedScriptHost.getOwnPropertyDescriptor(o, property);
+                        if (descriptor) {
+                            InjectedScriptHost.nullifyPrototype(descriptor);
+                        }
                     }
                     var isAccessorProperty = descriptor && ("get" in descriptor || "set" in descriptor);
                     if (accessorPropertiesOnly && !isAccessorProperty)

src/inspector/injected-script.cc

@@ -279,8 +279,9 @@ std::unique_ptr<InjectedScript> InjectedScript::create(
   if (!inspectedContext->inspector()
            ->compileAndRunInternalScript(
                context, toV8String(isolate, injectedScriptSource))
-           .ToLocal(&value))
+           .ToLocal(&value)) {
     return nullptr;
+  }
   DCHECK(value->IsFunction());
   v8::Local<v8::Object> scriptHostWrapper =
       V8InjectedScriptHost::create(context, inspectedContext->inspector());

src/inspector/injected_script_externs.js

@@ -101,6 +101,13 @@ InjectedScriptHostClass.prototype.getOwnPropertyNames = function(obj) {}
  */
 InjectedScriptHostClass.prototype.getOwnPropertySymbols = function(obj) {}
 
+/**
+ * @param {!Object} obj
+ * @param {string|symbol} name
+ * @return {{isBuiltin:boolean, hasGetter:boolean, hasSetter:boolean}|undefined}
+ */
+InjectedScriptHostClass.prototype.nativeAccessorDescriptor = function(obj, name) {}
+
 /** @type {!InjectedScriptHostClass} */
 var InjectedScriptHost;
 /** @type {!Window} */

src/inspector/v8-injected-script-host.cc

@@ -5,6 +5,7 @@
 #include "src/inspector/v8-injected-script-host.h"
 
 #include "src/base/macros.h"
+#include "src/debug/debug-interface.h"
 #include "src/inspector/injected-script.h"
 #include "src/inspector/string-util.h"
 #include "src/inspector/v8-debugger.h"
@@ -80,6 +81,9 @@ v8::Local<v8::Object> V8InjectedScriptHost::create(
   setFunctionProperty(context, injectedScriptHost, "proxyTargetValue",
                       V8InjectedScriptHost::proxyTargetValueCallback,
                       debuggerExternal);
+  setFunctionProperty(context, injectedScriptHost, "nativeAccessorDescriptor",
+                      V8InjectedScriptHost::nativeAccessorDescriptorCallback,
+                      debuggerExternal);
   createDataProperty(context, injectedScriptHost,
                      toV8StringInternalized(isolate, "keys"),
                      v8::debug::GetBuiltin(isolate, v8::debug::kObjectKeys));
@@ -337,4 +341,37 @@ void V8InjectedScriptHost::proxyTargetValueCallback(
   info.GetReturnValue().Set(target);
 }
 
+void V8InjectedScriptHost::nativeAccessorDescriptorCallback(
+    const v8::FunctionCallbackInfo<v8::Value>& info) {
+  v8::Isolate* isolate = info.GetIsolate();
+  if (info.Length() != 2 || !info[0]->IsObject() || !info[1]->IsName()) {
+    info.GetReturnValue().Set(v8::Undefined(isolate));
+    return;
+  }
+  v8::Local<v8::Context> context = isolate->GetCurrentContext();
+  int flags = v8::debug::GetNativeAccessorDescriptor(
+      context, v8::Local<v8::Object>::Cast(info[0]),
+      v8::Local<v8::Name>::Cast(info[1]));
+  if (flags == static_cast<int>(v8::debug::NativeAccessorType::None)) {
+    info.GetReturnValue().Set(v8::Undefined(isolate));
+    return;
+  }
+
+  bool isBuiltin =
+      flags & static_cast<int>(v8::debug::NativeAccessorType::IsBuiltin);
+  bool hasGetter =
+      flags & static_cast<int>(v8::debug::NativeAccessorType::HasGetter);
+  bool hasSetter =
+      flags & static_cast<int>(v8::debug::NativeAccessorType::HasSetter);
+  v8::Local<v8::Object> result = v8::Object::New(isolate);
+  result->SetPrototype(context, v8::Null(isolate)).ToChecked();
+  createDataProperty(context, result, toV8String(isolate, "isBuiltin"),
+                     v8::Boolean::New(isolate, isBuiltin));
+  createDataProperty(context, result, toV8String(isolate, "hasGetter"),
+                     v8::Boolean::New(isolate, hasGetter));
+  createDataProperty(context, result, toV8String(isolate, "hasSetter"),
+                     v8::Boolean::New(isolate, hasSetter));
+  info.GetReturnValue().Set(result);
+}
+
 }  // namespace v8_inspector

src/inspector/v8-injected-script-host.h

@@ -42,6 +42,8 @@ class V8InjectedScriptHost {
   static void bindCallback(const v8::FunctionCallbackInfo<v8::Value>&);
   static void proxyTargetValueCallback(
       const v8::FunctionCallbackInfo<v8::Value>&);
+  static void nativeAccessorDescriptorCallback(
+      const v8::FunctionCallbackInfo<v8::Value>&);
 };
 
 }  // namespace v8_inspector

test/inspector/inspector-test.cc

@@ -688,6 +688,9 @@ class InspectorExtension : public IsolateData::SetupGlobalTask {
         ToV8String(isolate, "markObjectAsNotInspectable"),
         v8::FunctionTemplate::New(
             isolate, &InspectorExtension::MarkObjectAsNotInspectable));
+    inspector->Set(ToV8String(isolate, "createObjectWithAccessor"),
+                   v8::FunctionTemplate::New(
+                       isolate, &InspectorExtension::CreateObjectWithAccessor));
     global->Set(ToV8String(isolate, "inspector"), inspector);
   }
 
@@ -827,6 +830,39 @@ class InspectorExtension : public IsolateData::SetupGlobalTask {
                      v8::True(isolate))
         .ToChecked();
   }
+
+  static void CreateObjectWithAccessor(
+      const v8::FunctionCallbackInfo<v8::Value>& args) {
+    if (args.Length() != 2 || !args[0]->IsString() || !args[1]->IsBoolean()) {
+      fprintf(stderr,
+              "Internal error: createObjectWithAccessor('accessor name', "
+              "hasSetter)\n");
+      Exit();
+    }
+    v8::Isolate* isolate = args.GetIsolate();
+    v8::Local<v8::ObjectTemplate> templ = v8::ObjectTemplate::New(isolate);
+    if (args[1].As<v8::Boolean>()->Value()) {
+      templ->SetAccessor(v8::Local<v8::String>::Cast(args[0]), AccessorGetter,
+                         AccessorSetter);
+    } else {
+      templ->SetAccessor(v8::Local<v8::String>::Cast(args[0]), AccessorGetter);
+    }
+    args.GetReturnValue().Set(
+        templ->NewInstance(isolate->GetCurrentContext()).ToLocalChecked());
+  }
+
+  static void AccessorGetter(v8::Local<v8::String> property,
+                             const v8::PropertyCallbackInfo<v8::Value>& info) {
+    v8::Isolate* isolate = info.GetIsolate();
+    isolate->ThrowException(ToV8String(isolate, "Getter is called"));
+  }
+
+  static void AccessorSetter(v8::Local<v8::String> property,
+                             v8::Local<v8::Value> value,
+                             const v8::PropertyCallbackInfo<void>& info) {
+    v8::Isolate* isolate = info.GetIsolate();
+    isolate->ThrowException(ToV8String(isolate, "Setter is called"));
+  }
 };
 
 }  //  namespace

test/inspector/runtime/runtime-get-properties-and-accessor-expected.txt

+Runtime.getProperties for objects with accessor
+title property with getter and setter:
+{
+    configurable : false
+    enumerable : false
+    get : {
+        className : Function
+        description : function nativeGetter() { [native code] }
+        objectId : <objectId>
+        type : function
+    }
+    isOwn : true
+    name : title
+    set : {
+        className : Function
+        description : function nativeSetter() { [native code] }
+        objectId : <objectId>
+        type : function
+    }
+}
+title property with getter only:
+{
+    configurable : false
+    enumerable : false
+    get : {
+        className : Function
+        description : function nativeGetter() { [native code] }
+        objectId : <objectId>
+        type : function
+    }
+    isOwn : true
+    name : title
+}

test/inspector/runtime/runtime-get-properties-and-accessor.js

+// Copyright 2017 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+let {session, contextGroup, Protocol} =
+  InspectorTest.start('Runtime.getProperties for objects with accessor');
+
+(async function test() {
+  let {result:{result:{objectId}}} = await Protocol.Runtime.evaluate({
+    expression: 'inspector.createObjectWithAccessor(\'title\', true)'
+  });
+  let {result:{result}} = await Protocol.Runtime.getProperties({
+    objectId,
+    ownProperties: true
+  });
+  InspectorTest.log('title property with getter and setter:');
+  InspectorTest.logMessage(result.find(property => property.name === 'title'));
+
+  ({result:{result:{objectId}}} = await Protocol.Runtime.evaluate({
+    expression: 'inspector.createObjectWithAccessor(\'title\', false)'
+  }));
+  ({result:{result}} = await Protocol.Runtime.getProperties({
+    objectId,
+    ownProperties: true
+  }));
+  InspectorTest.log('title property with getter only:');
+  InspectorTest.logMessage(result.find(property => property.name === 'title'));
+  InspectorTest.completeTest();
+})()