[inspector] improved Runtime.queryObjects

- simplify prototype traversal.
- use V8InspectorClient::isInspectableHeapObject since some embedders
  on JavaScript heap contains not inspectable objects, e.g. wrapper
  boilerplates in blink.
- Runtime.queryObjects takes prototype object as argument for more
  flexibility.

R=alph@chromium.org

Bug: v8:6732
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I06f0d5c987150c80c3e9b05e7f6ad195985fc539
Reviewed-on: https://chromium-review.googlesource.com/627577
Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47559}

src/debug/debug-interface.h

@@ -395,10 +395,6 @@ class QueryObjectPredicate {
  public:
   virtual ~QueryObjectPredicate() = default;
   virtual bool Filter(v8::Local<v8::Object> object) = 0;
-
- protected:
-  // This method can be used only inside of Filter function.
-  v8::MaybeLocal<v8::Function> GetConstructor(v8::Object* object);
 };
 
 void QueryObjects(v8::Local<v8::Context> context,

src/inspector/js_protocol.json

@@ -347,7 +347,7 @@
             {
                 "name": "queryObjects",
                 "parameters": [
-                    { "name": "constructorObjectId", "$ref": "RemoteObjectId", "description": "Identifier of the constructor to return objects for." }
+                    { "name": "prototypeObjectId", "$ref": "RemoteObjectId", "description": "Identifier of the prototype to return objects for." }
                 ],
                 "returns": [
                     { "name": "objects", "$ref": "RemoteObject", "description": "Array with objects." }

src/inspector/v8-console.cc

@@ -594,7 +594,7 @@ void V8Console::copyCallback(const v8::FunctionCallbackInfo<v8::Value>& info,
 
 void V8Console::queryObjectsCallback(
     const v8::FunctionCallbackInfo<v8::Value>& info, int sessionId) {
-  if (info.Length() < 1 || !info[0]->IsFunction()) return;
+  if (info.Length() < 1 || !info[0]->IsObject()) return;
   inspectImpl(info, sessionId, kQueryObjects, m_inspector);
 }
 

src/inspector/v8-debugger.cc

@@ -4,8 +4,6 @@
 
 #include "src/inspector/v8-debugger.h"
 
-#include <unordered_set>
-
 #include "src/inspector/inspected-context.h"
 #include "src/inspector/protocol/Protocol.h"
 #include "src/inspector/script-breakpoint.h"
@@ -132,63 +130,30 @@ void cleanupExpiredWeakPointers(Map& map) {
   }
 }
 
-class QueryObjectPredicate : public v8::debug::QueryObjectPredicate {
+class MatchPrototypePredicate : public v8::debug::QueryObjectPredicate {
  public:
-  QueryObjectPredicate(v8::Local<v8::Context> context,
-                       v8::Local<v8::Function> constructor)
-      : m_context(context), m_constructor(constructor) {}
+  MatchPrototypePredicate(V8InspectorImpl* inspector,
+                          v8::Local<v8::Context> context,
+                          v8::Local<v8::Object> prototype)
+      : m_inspector(inspector), m_context(context), m_prototype(prototype) {}
 
   bool Filter(v8::Local<v8::Object> object) override {
-    if (CheckObject(*object)) return true;
-    std::vector<v8::Object*> prototypeChain;
-    v8::Local<v8::Value> prototype;
+    v8::Local<v8::Context> objectContext = object->CreationContext();
+    if (objectContext != m_context) return false;
+    if (!m_inspector->client()->isInspectableHeapObject(object)) return false;
     // Get prototype chain for current object until first visited prototype.
-    for (prototype = object->GetPrototype(); IsUnvisitedPrototype(prototype);
+    for (v8::Local<v8::Value> prototype = object->GetPrototype();
+         prototype->IsObject();
          prototype = prototype.As<v8::Object>()->GetPrototype()) {
-      prototypeChain.push_back(v8::Object::Cast(*prototype));
+      if (m_prototype == prototype) return true;
     }
-    // Include first visited prototype if any.
-    if (prototype->IsObject()) {
-      prototypeChain.push_back(v8::Object::Cast(*prototype));
-    }
-    bool hasMatched = false;
-    // Go from last prototype to first one, mark all prototypes as matched after
-    // first matched prototype.
-    for (auto it = prototypeChain.rbegin(); it != prototypeChain.rend(); ++it) {
-      hasMatched = hasMatched || CheckObject(*it);
-      if (hasMatched) m_matchedPrototypes.insert(*it);
-      m_visitedPrototypes.insert(*it);
-    }
-    return hasMatched;
+    return false;
   }
 
  private:
-  bool CheckObject(v8::Object* object) {
-    if (m_matchedPrototypes.find(object) != m_matchedPrototypes.end())
-      return true;
-    if (m_visitedPrototypes.find(object) != m_visitedPrototypes.end())
-      return false;
-    v8::Local<v8::Context> objectContext = object->CreationContext();
-    if (objectContext != m_context) return false;
-    v8::Local<v8::Function> constructor;
-    if (!GetConstructor(object).ToLocal(&constructor)) return false;
-    return constructor == m_constructor;
-  }
-
-  bool IsUnvisitedPrototype(v8::Local<v8::Value> prototypeValue) {
-    if (!prototypeValue->IsObject()) return false;
-    v8::Object* prototypeObject = v8::Object::Cast(*prototypeValue);
-    v8::Local<v8::Context> prototypeContext =
-        prototypeObject->CreationContext();
-    if (prototypeContext != m_context) return false;
-    return m_visitedPrototypes.find(prototypeObject) ==
-           m_visitedPrototypes.end();
-  }
-
+  V8InspectorImpl* m_inspector;
   v8::Local<v8::Context> m_context;
-  v8::Local<v8::Function> m_constructor;
-  std::unordered_set<v8::Object*> m_visitedPrototypes;
-  std::unordered_set<v8::Object*> m_matchedPrototypes;
+  v8::Local<v8::Value> m_prototype;
 };
 
 }  // namespace
@@ -722,11 +687,11 @@ v8::MaybeLocal<v8::Array> V8Debugger::internalProperties(
   return properties;
 }
 
-v8::Local<v8::Array> V8Debugger::queryObjects(
-    v8::Local<v8::Context> context, v8::Local<v8::Function> constructor) {
+v8::Local<v8::Array> V8Debugger::queryObjects(v8::Local<v8::Context> context,
+                                              v8::Local<v8::Object> prototype) {
   v8::Isolate* isolate = context->GetIsolate();
   v8::PersistentValueVector<v8::Object> v8Objects(isolate);
-  QueryObjectPredicate predicate(context, constructor);
+  MatchPrototypePredicate predicate(m_inspector, context, prototype);
   v8::debug::QueryObjects(context, &predicate, &v8Objects);
 
   v8::MicrotasksScope microtasksScope(isolate,

src/inspector/v8-debugger.h

@@ -89,7 +89,7 @@ class V8Debugger : public v8::debug::DebugDelegate {
                                                v8::Local<v8::Value>);
 
   v8::Local<v8::Array> queryObjects(v8::Local<v8::Context> context,
-                                    v8::Local<v8::Function> constructor);
+                                    v8::Local<v8::Object> prototype);
 
   void asyncTaskScheduled(const StringView& taskName, void* task,
                           bool recurring);

src/inspector/v8-runtime-agent-impl.cc

@@ -523,16 +523,16 @@ void V8RuntimeAgentImpl::runScript(
 }
 
 Response V8RuntimeAgentImpl::queryObjects(
-    const String16& constructorObjectId,
+    const String16& prototypeObjectId,
     std::unique_ptr<protocol::Runtime::RemoteObject>* objects) {
-  InjectedScript::ObjectScope scope(m_session, constructorObjectId);
+  InjectedScript::ObjectScope scope(m_session, prototypeObjectId);
   Response response = scope.initialize();
   if (!response.isSuccess()) return response;
-  if (!scope.object()->IsFunction()) {
-    return Response::Error("Constructor should be instance of Function");
+  if (!scope.object()->IsObject()) {
+    return Response::Error("Prototype should be instance of Object");
   }
   v8::Local<v8::Array> resultArray = m_inspector->debugger()->queryObjects(
-      scope.context(), v8::Local<v8::Function>::Cast(scope.object()));
+      scope.context(), v8::Local<v8::Object>::Cast(scope.object()));
   return scope.injectedScript()->wrapObject(
       resultArray, scope.objectGroupName(), false, false, objects);
 }

src/inspector/v8-runtime-agent-impl.h

@@ -98,7 +98,7 @@ class V8RuntimeAgentImpl : public protocol::Runtime::Backend {
                  Maybe<bool> generatePreview, Maybe<bool> awaitPromise,
                  std::unique_ptr<RunScriptCallback>) override;
   Response queryObjects(
-      const String16& constructorObjectId,
+      const String16& prototypeObjectId,
       std::unique_ptr<protocol::Runtime::RemoteObject>* objects) override;
 
   void reset();

src/profiler/heap-profiler.cc

@@ -12,19 +12,6 @@
 #include "src/profiler/sampling-heap-profiler.h"
 
 namespace v8 {
-
-v8::MaybeLocal<v8::Function> debug::QueryObjectPredicate::GetConstructor(
-    v8::Object* v8_object) {
-  internal::Handle<internal::JSReceiver> object(Utils::OpenHandle(v8_object));
-  internal::Handle<internal::Map> map(object->map());
-  internal::Object* maybe_constructor = map->GetConstructor();
-  if (maybe_constructor->IsJSFunction()) {
-    return Utils::ToLocal(
-        internal::handle(internal::JSFunction::cast(maybe_constructor)));
-  }
-  return v8::MaybeLocal<v8::Function>();
-}
-
 namespace internal {
 
 HeapProfiler::HeapProfiler(Heap* heap)
@@ -225,11 +212,11 @@ void HeapProfiler::QueryObjects(Handle<Context> context,
                                 PersistentValueVector<v8::Object>* objects) {
   // We should return accurate information about live objects, so we need to
   // collect all garbage first.
-  isolate()->heap()->CollectAllAvailableGarbage(
+  heap()->CollectAllAvailableGarbage(
       GarbageCollectionReason::kLowMemoryNotification);
   heap()->CollectAllGarbage(Heap::kMakeHeapIterableMask,
                             GarbageCollectionReason::kHeapProfiler);
-  HeapIterator heap_iterator(heap(), HeapIterator::kFilterUnreachable);
+  HeapIterator heap_iterator(heap());
   HeapObject* heap_obj;
   while ((heap_obj = heap_iterator.next()) != nullptr) {
     if (!heap_obj->IsJSObject() || heap_obj->IsExternal()) continue;

test/inspector/inspector-test.cc

@@ -668,6 +668,10 @@ class InspectorExtension : public IsolateData::SetupGlobalTask {
     inspector->Set(ToV8String(isolate, "allowAccessorFormatting"),
                    v8::FunctionTemplate::New(
                        isolate, &InspectorExtension::AllowAccessorFormatting));
+    inspector->Set(
+        ToV8String(isolate, "markObjectAsNotInspectable"),
+        v8::FunctionTemplate::New(
+            isolate, &InspectorExtension::MarkObjectAsNotInspectable));
     global->Set(ToV8String(isolate, "inspector"), inspector);
   }
 
@@ -791,6 +795,22 @@ class InspectorExtension : public IsolateData::SetupGlobalTask {
                      v8::Null(isolate))
         .ToChecked();
   }
+
+  static void MarkObjectAsNotInspectable(
+      const v8::FunctionCallbackInfo<v8::Value>& args) {
+    if (args.Length() != 1 || !args[0]->IsObject()) {
+      fprintf(stderr, "Internal error: markObjectAsNotInspectable(object).");
+      Exit();
+    }
+    v8::Local<v8::Object> object = args[0].As<v8::Object>();
+    v8::Isolate* isolate = args.GetIsolate();
+    v8::Local<v8::Private> notInspectablePrivate =
+        v8::Private::ForApi(isolate, ToV8String(isolate, "notInspectable"));
+    object
+        ->SetPrivate(isolate->GetCurrentContext(), notInspectablePrivate,
+                     v8::True(isolate))
+        .ToChecked();
+  }
 };
 
 }  //  namespace

test/inspector/isolate-data.cc

@@ -70,6 +70,12 @@ IsolateData::IsolateData(TaskRunner* task_runner,
     isolate_->SetPromiseRejectCallback(&IsolateData::PromiseRejectHandler);
     inspector_ = v8_inspector::V8Inspector::create(isolate_, this);
   }
+  v8::HandleScope handle_scope(isolate_);
+  not_inspectable_private_.Reset(
+      isolate_, v8::Private::ForApi(isolate_, v8::String::NewFromUtf8(
+                                                  isolate_, "notInspectable",
+                                                  v8::NewStringType::kNormal)
+                                                  .ToLocalChecked()));
 }
 
 IsolateData* IsolateData::FromContext(v8::Local<v8::Context> context) {
@@ -332,6 +338,15 @@ bool IsolateData::formatAccessorsAsProperties(v8::Local<v8::Value> object) {
       .FromMaybe(false);
 }
 
+bool IsolateData::isInspectableHeapObject(v8::Local<v8::Object> object) {
+  v8::Isolate* isolate = v8::Isolate::GetCurrent();
+  v8::Local<v8::Context> context = isolate->GetCurrentContext();
+  v8::MicrotasksScope microtasks_scope(
+      isolate, v8::MicrotasksScope::kDoNotRunMicrotasks);
+  return !object->HasPrivate(context, not_inspectable_private_.Get(isolate))
+              .FromMaybe(false);
+}
+
 v8::Local<v8::Context> IsolateData::ensureDefaultContextInGroup(
     int context_group_id) {
   return GetContext(context_group_id);

test/inspector/isolate-data.h

@@ -105,6 +105,7 @@ class IsolateData : public v8_inspector::V8InspectorClient {
                          const v8_inspector::StringView& url,
                          unsigned lineNumber, unsigned columnNumber,
                          v8_inspector::V8StackTrace*) override;
+  bool isInspectableHeapObject(v8::Local<v8::Object>) override;
 
   TaskRunner* task_runner_;
   SetupGlobalTasks setup_global_tasks_;
@@ -122,6 +123,7 @@ class IsolateData : public v8_inspector::V8InspectorClient {
   bool current_time_set_ = false;
   double current_time_ = 0.0;
   bool log_console_api_message_calls_ = false;
+  v8::Global<v8::Private> not_inspectable_private_;
 
   DISALLOW_COPY_AND_ASSIGN(IsolateData);
 };

test/inspector/runtime/query-objects-expected.txt

@@ -3,71 +3,78 @@ Checks Runtime.queryObjects
 Running test: testClass
 Declare class Foo & store its constructor.
 Create object with class Foo.
-Query objects with Foo constructor.
+Query objects with Foo prototype.
 Dump each object constructor name.
 [
-    [0] : Foo
-    [1] : Foo
+    [0] : Foo,object
 ]
 Create object with class Foo.
-Query objects with Foo constructor.
+Query objects with Foo prototype.
 Dump each object constructor name.
 [
-    [0] : Foo
-    [1] : Foo
-    [2] : Foo
+    [0] : Foo,object
+    [1] : Foo,object
 ]
 
 Running test: testDerivedNewClass
 Declare class Foo & store its constructor.
 Declare class Boo extends Foo & store its constructor.
-Query objects with Foo constructor.
+Query objects with Foo prototype.
 Dump each object constructor name.
 [
-    [0] : Boo
-    [1] : Foo
+    [0] : Boo,object
 ]
-Query objects with Boo constructor.
+Query objects with Boo prototype.
 Dump each object constructor name.
 [
-    [0] : Boo
 ]
 Create object with class Foo
-Query objects with Foo constructor.
+Query objects with Foo prototype.
 Dump each object constructor name.
 [
-    [0] : Boo
-    [1] : Foo
-    [2] : Foo
+    [0] : Boo,object
+    [1] : Foo,object
 ]
 Create object with class Boo
-Query objects with Foo constructor.
+Query objects with Foo prototype.
 Dump each object constructor name.
 [
-    [0] : Boo
-    [1] : Boo
-    [2] : Foo
-    [3] : Foo
+    [0] : Boo,object
+    [1] : Boo,object
+    [2] : Foo,object
 ]
-Query objects with Boo constructor.
+Query objects with Boo prototype.
 Dump each object constructor name.
 [
-    [0] : Boo
-    [1] : Boo
+    [0] : Boo,object
 ]
 
 Running test: testNewFunction
 Declare Foo & store it.
 Create object using Foo.
-Query objects with Foo constructor.
+Query objects with Foo prototype.
 Dump each object constructor name.
 [
-    [0] : Foo
+    [0] : Foo,object
 ]
 Create object using Foo.
-Query objects with Foo constructor.
+Query objects with Foo prototype.
+Dump each object constructor name.
+[
+    [0] : Foo,object
+    [1] : Foo,object
+]
+
+Running test: testNonInspectable
+Declare Foo & store it.
+Create object using Foo.
+Query objects with Foo prototype.
+Dump each object constructor name.
+[
+    [0] : Foo,object
+]
+Mark object as not inspectable.
+Query objects with Foo prototype.
 Dump each object constructor name.
 [
-    [0] : Foo
-    [1] : Foo
 ]

test/inspector/runtime/query-objects.js

@@ -16,7 +16,7 @@ InspectorTest.runAsyncTestSuite([
       expression: 'class Foo{constructor(){}};'
     });
     let {result:{result:{objectId}}} = await Protocol.Runtime.evaluate({
-      expression: 'Foo'
+      expression: 'Foo.prototype'
     });
 
     for (let i = 0; i < 2; ++i) {
@@ -36,14 +36,14 @@ InspectorTest.runAsyncTestSuite([
     InspectorTest.log('Declare class Foo & store its constructor.');
     Protocol.Runtime.evaluate({expression: 'class Foo{};'});
     let {result:{result:{objectId}}} = await Protocol.Runtime.evaluate({
-      expression: 'Foo'
+      expression: 'Foo.prototype'
     });
     let fooConstructorId = objectId;
 
     InspectorTest.log('Declare class Boo extends Foo & store its constructor.');
     Protocol.Runtime.evaluate({expression: 'class Boo extends Foo{};'});
     ({result:{result:{objectId}}} = await Protocol.Runtime.evaluate({
-      expression: 'Boo'
+      expression: 'Boo.prototype'
     }));
     let booConstructorId = objectId;
 
@@ -70,7 +70,7 @@ InspectorTest.runAsyncTestSuite([
     InspectorTest.log('Declare Foo & store it.');
     Protocol.Runtime.evaluate({expression: 'function Foo(){}'});
     let {result:{result:{objectId}}} = await Protocol.Runtime.evaluate({
-      expression: 'Foo'
+      expression: 'Foo.prototype'
     });
 
     for (let i = 0; i < 2; ++i) {
@@ -79,19 +79,39 @@ InspectorTest.runAsyncTestSuite([
       await queryObjects(session, objectId, 'Foo');
     }
     session.disconnect();
+  },
+
+  async function testNonInspectable() {
+    let contextGroup = new InspectorTest.ContextGroup();
+    let session = contextGroup.connect();
+    let Protocol = session.Protocol;
+
+    InspectorTest.log('Declare Foo & store it.');
+    Protocol.Runtime.evaluate({expression: 'function Foo(){}'});
+    let {result:{result:{objectId}}} = await Protocol.Runtime.evaluate({
+      expression: 'Foo.prototype'
+    });
+
+    InspectorTest.log('Create object using Foo.');
+    Protocol.Runtime.evaluate({expression: 'a = new Foo()'});
+    await queryObjects(session, objectId, 'Foo');
+    InspectorTest.log('Mark object as not inspectable.')
+    Protocol.Runtime.evaluate({expression: 'inspector.markObjectAsNotInspectable(a)'});
+    await queryObjects(session, objectId, 'Foo');
+    session.disconnect();
   }
 ]);
 
 const constructorsNameFunction = `
 function() {
-  return this.map(o => o.constructor.name).sort();
+  return this.map(o => o.constructor.name + ',' + typeof o).sort();
 }`;
 
-async function queryObjects(sesion, constructorObjectId, name) {
+async function queryObjects(sesion, prototypeObjectId, name) {
   let {result:{objects}} = await sesion.Protocol.Runtime.queryObjects({
-    constructorObjectId
+    prototypeObjectId
   });
-  InspectorTest.log(`Query objects with ${name} constructor.`);
+  InspectorTest.log(`Query objects with ${name} prototype.`);
   let {result:{result:{value}}} = await sesion.Protocol.Runtime.callFunctionOn({
     objectId: objects.objectId,
     functionDeclaration: constructorsNameFunction,