DevTools: add support for injecting bindings by context name

caseq · Commit Bot · commit abacd4c11518 · 2020-10-01T17:20:04.000Z
This adds support for injecting binding into contexts other than main based on the context name (AKA isolated world name in Blink terms). This would simplify a common use case for addBinding in Puppeteer and other automation tools that use addBinding to expose a back-channel for extension code running in an isolated world by making bindings available to such code at an early stage and in a race-free manner (currently, we can only inject a binding into specific context after the creation of the context has been reported to the client, which typically introduces a race with other evals the client may be running in the context). Change-Id: I66454954491a47a0c9aa4864f0aace4da2e67d3a Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2440984 Reviewed-by: Simon Zünd <szuend@chromium.org> Reviewed-by: Pavel Feldman <pfeldman@chromium.org> Commit-Queue: Andrey Kosyakov <caseq@chromium.org> Cr-Commit-Position: refs/heads/master@{#70266}
diff --git a/include/js_protocol.pdl b/include/js_protocol.pdl
@@ -1542,15 +1542,23 @@ domain Runtime
   # If executionContextId is empty, adds binding with the given name on the
   # global objects of all inspected contexts, including those created later,
   # bindings survive reloads.
-  # If executionContextId is specified, adds binding only on global object of
-  # given execution context.
   # Binding function takes exactly one argument, this argument should be string,
   # in case of any other input, function throws an exception.
   # Each binding function call produces Runtime.bindingCalled notification.
   experimental command addBinding
     parameters
       string name
+      # If specified, the binding would only be exposed to the specified
+      # execution context. If omitted and `executionContextName` is not set,
+      # the binding is exposed to all execution contexts of the target.
+      # This parameter is mutually exclusive with `executionContextName`.
       optional ExecutionContextId executionContextId
+      # If specified, the binding is exposed to the executionContext with
+      # matching name, even for contexts created after the binding is added.
+      # See also `ExecutionContext.name` and `worldName` parameter to
+      # `Page.addScriptToEvaluateOnNewDocument`.
+      # This parameter is mutually exclusive with `executionContextId`.
+      experimental optional string executionContextName
 
   # This method does not remove binding function from global object but
   # unsubscribes current runtime agent from Runtime.bindingCalled notifications.
diff --git a/src/inspector/v8-runtime-agent-impl.cc b/src/inspector/v8-runtime-agent-impl.cc
@@ -56,6 +56,7 @@ static const char customObjectFormatterEnabled[] =
     "customObjectFormatterEnabled";
 static const char runtimeEnabled[] = "runtimeEnabled";
 static const char bindings[] = "bindings";
+static const char globalBindingsKey[] = "";
 }  // namespace V8RuntimeAgentImplState
 
 using protocol::Runtime::RemoteObject;
@@ -663,32 +664,61 @@ void V8RuntimeAgentImpl::terminateExecution(
   m_inspector->debugger()->terminateExecution(std::move(callback));
 }
 
+namespace {
+protocol::DictionaryValue* getOrCreateDictionary(
+    protocol::DictionaryValue* dict, const String16& key) {
+  if (protocol::DictionaryValue* bindings = dict->getObject(key))
+    return bindings;
+  dict->setObject(key, protocol::DictionaryValue::create());
+  return dict->getObject(key);
+}
+}  // namespace
+
 Response V8RuntimeAgentImpl::addBinding(const String16& name,
-                                        Maybe<int> executionContextId) {
+                                        Maybe<int> executionContextId,
+                                        Maybe<String16> executionContextName) {
   if (m_activeBindings.count(name)) return Response::Success();
   if (executionContextId.isJust()) {
+    if (executionContextName.isJust()) {
+      return Response::InvalidParams(
+          "executionContextName is mutually exclusive with executionContextId");
+    }
     int contextId = executionContextId.fromJust();
     InspectedContext* context =
         m_inspector->getContext(m_session->contextGroupId(), contextId);
     if (!context) {
-      return Response::ServerError(
+      return Response::InvalidParams(
           "Cannot find execution context with given executionContextId");
     }
     addBinding(context, name);
     return Response::Success();
   }
+
+  // If it's a globally exposed binding, i.e. no context name specified, use
+  // a special value for the context name.
+  String16 contextKey = V8RuntimeAgentImplState::globalBindingsKey;
+  if (executionContextName.isJust()) {
+    contextKey = executionContextName.fromJust();
+    if (contextKey == V8RuntimeAgentImplState::globalBindingsKey) {
+      return Response::InvalidParams("Invalid executionContextName");
+    }
+  }
   // Only persist non context-specific bindings, as contextIds don't make
   // any sense when state is restored in a different process.
-  if (!m_state->getObject(V8RuntimeAgentImplState::bindings)) {
-    m_state->setObject(V8RuntimeAgentImplState::bindings,
-                       protocol::DictionaryValue::create());
-  }
   protocol::DictionaryValue* bindings =
-      m_state->getObject(V8RuntimeAgentImplState::bindings);
-  bindings->setBoolean(name, true);
+      getOrCreateDictionary(m_state, V8RuntimeAgentImplState::bindings);
+  protocol::DictionaryValue* contextBindings =
+      getOrCreateDictionary(bindings, contextKey);
+  contextBindings->setBoolean(name, true);
+
   m_inspector->forEachContext(
       m_session->contextGroupId(),
-      [&name, this](InspectedContext* context) { addBinding(context, name); });
+      [&name, &executionContextName, this](InspectedContext* context) {
+        if (executionContextName.isJust() &&
+            executionContextName.fromJust() != context->humanReadableName())
+          return;
+        addBinding(context, name);
+      });
   return Response::Success();
 }
 
@@ -750,12 +780,23 @@ void V8RuntimeAgentImpl::bindingCalled(const String16& name,
 }
 
 void V8RuntimeAgentImpl::addBindings(InspectedContext* context) {
+  const String16 contextName = context->humanReadableName();
   if (!m_enabled) return;
   protocol::DictionaryValue* bindings =
       m_state->getObject(V8RuntimeAgentImplState::bindings);
   if (!bindings) return;
-  for (size_t i = 0; i < bindings->size(); ++i)
-    addBinding(context, bindings->at(i).first);
+  protocol::DictionaryValue* globalBindings =
+      bindings->getObject(V8RuntimeAgentImplState::globalBindingsKey);
+  if (globalBindings) {
+    for (size_t i = 0; i < globalBindings->size(); ++i)
+      addBinding(context, globalBindings->at(i).first);
+  }
+  protocol::DictionaryValue* contextBindings =
+      contextName.isEmpty() ? nullptr : bindings->getObject(contextName);
+  if (contextBindings) {
+    for (size_t i = 0; i < contextBindings->size(); ++i)
+      addBinding(context, contextBindings->at(i).first);
+  }
 }
 
 void V8RuntimeAgentImpl::restore() {
diff --git a/src/inspector/v8-runtime-agent-impl.h b/src/inspector/v8-runtime-agent-impl.h
@@ -117,8 +117,8 @@ class V8RuntimeAgentImpl : public protocol::Runtime::Backend {
   void terminateExecution(
       std::unique_ptr<TerminateExecutionCallback> callback) override;
 
-  Response addBinding(const String16& name,
-                      Maybe<int> executionContextId) override;
+  Response addBinding(const String16& name, Maybe<int> executionContextId,
+                      Maybe<String16> executionContextName) override;
   Response removeBinding(const String16& name) override;
   void addBindings(InspectedContext* context);
 
diff --git a/test/inspector/runtime/add-binding-expected.txt b/test/inspector/runtime/add-binding-expected.txt
@@ -156,3 +156,51 @@ binding called in session1
     }
 }
 Call binding in newly created context (binding should NOT be exposed)
+
+Running test: testAddBindingToContextByName
+Call binding in default context (binding should NOT be exposed)
+Call binding in Foo (binding should be exposed)
+binding called in session1
+{
+    method : Runtime.bindingCalled
+    params : {
+        executionContextId : <executionContextId>
+        name : frobnicate
+        payload : message
+    }
+}
+Call binding in Bar (binding should NOT be exposed)
+Call binding in newly-created Foo (binding should be exposed)
+binding called in session1
+{
+    method : Runtime.bindingCalled
+    params : {
+        executionContextId : <executionContextId>
+        name : frobnicate
+        payload : message
+    }
+}
+Call binding in newly-created Bazz (binding should NOT be exposed)
+
+Running test: testErrors
+{
+    error : {
+        code : -32602
+        message : Invalid executionContextName
+    }
+    id : <messageId>
+}
+{
+    error : {
+        code : -32602
+        message : executionContextName is mutually exclusive with executionContextId
+    }
+    id : <messageId>
+}
+{
+    error : {
+        code : -32602
+        message : Cannot find execution context with given executionContextId
+    }
+    id : <messageId>
+}
diff --git a/test/inspector/runtime/add-binding.js b/test/inspector/runtime/add-binding.js
@@ -85,7 +85,53 @@ InspectorTest.runAsyncTestSuite([
     contextGroup.createContext();
     const contextId3 = (await session.Protocol.Runtime.onceExecutionContextCreated()).params.context.id;
     await session.Protocol.Runtime.evaluate({expression, contextId: contextId3});
+  },
+
+  async function testAddBindingToContextByName() {
+    const {contextGroup, sessions: [session]} = setupSessions(1);
+    const defaultContext = (await session.Protocol.Runtime.onceExecutionContextCreated()).params.context.id;
+
+    contextGroup.createContext("foo");
+    const contextFoo = (await session.Protocol.Runtime.onceExecutionContextCreated()).params.context.id;
+
+    contextGroup.createContext("bar");
+    const contextBar = (await session.Protocol.Runtime.onceExecutionContextCreated()).params.context.id;
+
+    await session.Protocol.Runtime.addBinding({name: 'frobnicate', executionContextName: 'foo'});
+    const expression = `frobnicate('message')`;
+
+    InspectorTest.log('Call binding in default context (binding should NOT be exposed)');
+    await session.Protocol.Runtime.evaluate({expression});
+
+    InspectorTest.log('Call binding in Foo (binding should be exposed)');
+    await session.Protocol.Runtime.evaluate({expression, contextId: contextFoo});
+
+    InspectorTest.log('Call binding in Bar (binding should NOT be exposed)');
+    await session.Protocol.Runtime.evaluate({expression, contextId: contextBar});
+
+    contextGroup.createContext("foo");
+    const contextFoo2 = (await session.Protocol.Runtime.onceExecutionContextCreated()).params.context.id;
+
+    InspectorTest.log('Call binding in newly-created Foo (binding should be exposed)');
+    await session.Protocol.Runtime.evaluate({expression, contextId: contextFoo2});
+
+    contextGroup.createContext("bazz");
+    const contextBazz = (await session.Protocol.Runtime.onceExecutionContextCreated()).params.context.id;
+
+    InspectorTest.log('Call binding in newly-created Bazz (binding should NOT be exposed)');
+    await session.Protocol.Runtime.evaluate({expression, contextId: contextBazz});
+  },
+
+  async function testErrors() {
+    const {contextGroup, sessions: [session]} = setupSessions(1);
+    let err = await session.Protocol.Runtime.addBinding({name: 'frobnicate', executionContextName: ''});
+    InspectorTest.logMessage(err);
+    err = await session.Protocol.Runtime.addBinding({name: 'frobnicate', executionContextName: 'foo', executionContextId: 1});
+    InspectorTest.logMessage(err);
+    err = await session.Protocol.Runtime.addBinding({name: 'frobnicate', executionContextId: 2128506});
+    InspectorTest.logMessage(err);
   }
+
 ]);
 
 function setupSessions(num) {
