[DevTools] Support CommandLineAPI in workers and Node.js

third_party/WebKit/Source/platform/v8_inspector/V8Console.cpp

Index: third_party/WebKit/Source/platform/v8_inspector/V8Console.cpp
diff --git a/third_party/WebKit/Source/platform/v8_inspector/V8Console.cpp b/third_party/WebKit/Source/platform/v8_inspector/V8Console.cpp
index 342df6fbe2e0faf912006eadc6a07c88562eb20c..7c3e66b514cbf75b39405d9379a8ee00e565ad49 100644
--- a/third_party/WebKit/Source/platform/v8_inspector/V8Console.cpp
+++ b/third_party/WebKit/Source/platform/v8_inspector/V8Console.cpp
@@ -666,6 +666,12 @@ v8::Local<v8::Object> V8Console::createConsole(InspectedContext* inspectedContex
     return console;
 }
 
+void V8Console::clearInspectedContextIfNeeded(v8::Local<v8::Context> context, v8::Local<v8::Object> console)
+{
+    v8::Isolate* isolate = context->GetIsolate();
+    console->SetPrivate(context, inspectedContextPrivateKey(isolate), v8::External::New(isolate, nullptr));
+}
+
 v8::Local<v8::Object> V8Console::createCommandLineAPI(InspectedContext* inspectedContext)
 {
     v8::Local<v8::Context> context = inspectedContext->context();
@@ -702,33 +708,86 @@ v8::Local<v8::Object> V8Console::createCommandLineAPI(InspectedContext* inspecte
     return commandLineAPI;
 }
 
-void V8Console::clearInspectedContextIfNeeded(v8::Local<v8::Context> context, v8::Local<v8::Object> console)
+static bool isCommandLineAPIGetter(const String16& name)
 {
-    v8::Isolate* isolate = context->GetIsolate();
-    console->SetPrivate(context, inspectedContextPrivateKey(isolate), v8::External::New(isolate, nullptr));
+    DEFINE_STATIC_LOCAL(protocol::HashSet<String16>, getters, ());
+    if (getters.size() == 0) {
+        const char* members[] = { "$0", "$1", "$2", "$3", "$4", "$_" };
+        for (size_t i = 0; i < WTF_ARRAY_LENGTH(members); ++i)
+            getters.add(members[i]);
+    }
+    return getters.find(name) != getters.end();
 }
 
-bool V8Debugger::isCommandLineAPIMethod(const String16& name)
+void V8Console::CommandLineAPIScope::accessorGetterCallback(v8::Local<v8::Name> name, const v8::PropertyCallbackInfo<v8::Value>& info)
 {
-    DEFINE_STATIC_LOCAL(protocol::HashSet<String16>, methods, ());
-    if (methods.size() == 0) {
-        const char* members[] = { "dir", "dirxml", "keys", "values", "profile", "profileEnd", "inspect",
-            "copy", "clear", "debug", "undebug", "monitor", "unmonitor", "table" };
-        for (size_t i = 0; i < WTF_ARRAY_LENGTH(members); ++i)
-            methods.add(members[i]);
+    CommandLineAPIScope* scope = static_cast<CommandLineAPIScope*>(info.Data().As<v8::External>()->Value());
+    DCHECK(!scope);

[dgozman, 2016/06/01 19:41:28]

DCHECK(scope)

[kozy, 2016/06/02 00:50:42]

Done.

+    v8::Local<v8::Object> commandLineAPI = scope->m_commandLineAPI;
+    v8::Local<v8::Context> context = info.GetIsolate()->GetCurrentContext();
+
+    v8::Local<v8::Value> value;
+    if (!commandLineAPI->Get(context, name).ToLocal(&value))
+        return;
+
+    if (isCommandLineAPIGetter(toProtocolStringWithTypeCheck(name)) && value->IsFunction()) {

[dgozman, 2016/06/01 19:41:28]

It must be a function.

[kozy, 2016/06/02 00:50:42]

Done.

+        v8::MicrotasksScope microtasks(info.GetIsolate(), v8::MicrotasksScope::kDoNotRunMicrotasks);
+        if (value.As<v8::Function>()->Call(context, commandLineAPI, 0, nullptr).ToLocal(&value))
+            info.GetReturnValue().Set(value);
+    } else {
+        info.GetReturnValue().Set(value);
     }
-    return methods.find(name) != methods.end();
 }
 
-bool V8Debugger::isCommandLineAPIGetter(const String16& name)
+void V8Console::CommandLineAPIScope::accessorSetterCallback(v8::Local<v8::Name> name, v8::Local<v8::Value> value, const v8::PropertyCallbackInfo<void>& info)
 {
-    DEFINE_STATIC_LOCAL(protocol::HashSet<String16>, getters, ());
-    if (getters.size() == 0) {
-        const char* members[] = { "$0", "$1", "$2", "$3", "$4", "$_" };
-        for (size_t i = 0; i < WTF_ARRAY_LENGTH(members); ++i)
-            getters.add(members[i]);
+    CommandLineAPIScope* scope = static_cast<CommandLineAPIScope*>(info.Data().As<v8::External>()->Value());
+    v8::Local<v8::Context> context = info.GetIsolate()->GetCurrentContext();
+    if (!info.Holder()->Delete(context, name).FromMaybe(false))
+        return;
+    if (!info.Holder()->CreateDataProperty(context, name, value).FromMaybe(false))

[dgozman, 2016/06/01 19:41:28]

Let's add "var dir = 1" to the test.

[kozy, 2016/06/02 00:50:42]

Done.

+        return;
+    bool removed = scope->m_installedMethods->Delete(context, name).FromMaybe(false);
+    DCHECK(removed);
+}
+
+V8Console::CommandLineAPIScope::CommandLineAPIScope(v8::Local<v8::Context> context, v8::Local<v8::Object> commandLineAPI, v8::Local<v8::Object> global)
+    : m_context(context)
+    , m_commandLineAPI(commandLineAPI)
+    , m_global(global)
+    , m_installedMethods(v8::Set::New(context->GetIsolate()))
+{
+    v8::Local<v8::Array> names;
+    if (!m_commandLineAPI->GetOwnPropertyNames(context).ToLocal(&names))
+        return;
+    for (size_t i = 0; i < names->Length(); ++i) {
+        v8::Local<v8::Value> name;
+        if (!names->Get(context, i).ToLocal(&name) || !name->IsName())
+            continue;
+        if (m_global->Has(context, name).FromMaybe(true))

[dgozman, 2016/06/01 19:41:28]

Do we account for non-enumerable properties here? Don't we hide any bindings?

[kozy, 2016/06/02 00:50:42]

We don't hide bindings here.

+            continue;
+        if (!m_installedMethods->Add(context, name).ToLocal(&m_installedMethods))
+            continue;
+        if (!m_global->SetAccessor(context, v8::Local<v8::Name>::Cast(name), CommandLineAPIScope::accessorGetterCallback,

[dgozman, 2016/06/01 19:41:28]

name.As<> ?

[kozy, 2016/06/02 00:50:43]

We can't use As for function arguments because it produces temporary variable.

+            CommandLineAPIScope::accessorSetterCallback, v8::External::New(context->GetIsolate(), this),

[dgozman, 2016/06/01 19:41:28]

Can we have a single external |this| per scope?

[kozy, 2016/06/02 00:50:43]

Done.

+            v8::DEFAULT, v8::DontEnum).FromMaybe(false)) {
+            bool removed = m_installedMethods->Delete(context, name).FromMaybe(false);
+            DCHECK(removed);
+            continue;
+        }
+    }
+}
+
+V8Console::CommandLineAPIScope::~CommandLineAPIScope()
+{
+    v8::Local<v8::Array> names = m_installedMethods->AsArray();
+    for (size_t i = 0; i < names->Length(); ++i) {
+        v8::Local<v8::Value> name;
+        if (!names->Get(m_context, i).ToLocal(&name) || !name->IsName())
+            continue;
+        bool removed = m_global->Delete(m_context, name).FromMaybe(false);

[dgozman, 2016/06/01 19:41:28]

Let's try to definePorperty in the test and check we don't remove it.

[kozy, 2016/06/02 00:50:43]

Done.
When defineProperty is called with value then it will go accessor otherwise it
will add setter or getter.

+        DCHECK(removed);
     }
-    return getters.find(name) != getters.end();
 }
 
 } // namespace blink