Fix addBinding when called for multiple contexts

Bug: chromium:1169639
Change-Id: I3939b2e8568f0df12ecce192edca6df2b33e3835
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2839551Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Alex Rudenko <alexrudenko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74177}

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

@@ -694,7 +694,6 @@ protocol::DictionaryValue* getOrCreateDictionary(
 Response V8RuntimeAgentImpl::addBinding(const String16& name,
                                         Maybe<int> executionContextId,
                                         Maybe<String16> executionContextName) {
-  if (m_activeBindings.count(name)) return Response::Success();
   if (executionContextId.isJust()) {
     if (executionContextName.isJust()) {
       return Response::InvalidParams(
@@ -764,6 +763,10 @@ void V8RuntimeAgentImpl::bindingCallback(
 
 void V8RuntimeAgentImpl::addBinding(InspectedContext* context,
                                     const String16& name) {
+  auto it = m_activeBindings.find(name);
+  if (it != m_activeBindings.end() && it->second.count(context->contextId())) {
+    return;
+  }
   v8::HandleScope handles(m_inspector->isolate());
   v8::Local<v8::Context> localContext = context->context();
   v8::Local<v8::Object> global = localContext->Global();
@@ -775,7 +778,12 @@ void V8RuntimeAgentImpl::addBinding(InspectedContext* context,
           .ToLocal(&functionValue)) {
     v8::Maybe<bool> success = global->Set(localContext, v8Name, functionValue);
     USE(success);
-    m_activeBindings.insert(name);
+    if (it == m_activeBindings.end()) {
+      m_activeBindings.emplace(name,
+                               std::unordered_set<int>(context->contextId()));
+    } else {
+      m_activeBindings.at(name).insert(context->contextId());
+    }
   }
 }
 

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

@@ -148,7 +148,8 @@ class V8RuntimeAgentImpl : public protocol::Runtime::Backend {
   bool m_enabled;
   std::unordered_map<String16, std::unique_ptr<v8::Global<v8::Script>>>
       m_compiledScripts;
-  std::set<String16> m_activeBindings;
+  // Binding name -> executionContextIds mapping.
+  std::unordered_map<String16, std::unordered_set<int>> m_activeBindings;
 };
 
 }  // namespace v8_inspector

test/inspector/runtime/add-binding-expected.txt

@@ -157,6 +157,50 @@ binding called in session1
 }
 Call binding in newly created context (binding should NOT be exposed)
 
+Running test: testAddBindingToMultipleContextsById
+Call binding in default context (binding should be exposed)
+binding called in session1
+{
+    method : Runtime.bindingCalled
+    params : {
+        executionContextId : <executionContextId>
+        name : frobnicate
+        payload : message
+    }
+}
+Call binding in target context (binding should be exposed)
+binding called in session1
+{
+    method : Runtime.bindingCalled
+    params : {
+        executionContextId : <executionContextId>
+        name : frobnicate
+        payload : message
+    }
+}
+
+Running test: testAddBindingToMultipleContextsInDifferentContextGroups
+Call binding in default context (binding should be exposed)
+binding called in group1/session1
+{
+    method : Runtime.bindingCalled
+    params : {
+        executionContextId : <executionContextId>
+        name : frobnicate
+        payload : message
+    }
+}
+Call binding in target context (binding should be exposed)
+binding called in group2/session1
+{
+    method : Runtime.bindingCalled
+    params : {
+        executionContextId : <executionContextId>
+        name : frobnicate
+        payload : message
+    }
+}
+
 Running test: testAddBindingToContextByName
 Call binding in default context (binding should NOT be exposed)
 Call binding in Foo (binding should be exposed)

test/inspector/runtime/add-binding.js

@@ -87,6 +87,44 @@ InspectorTest.runAsyncTestSuite([
     await session.Protocol.Runtime.evaluate({expression, contextId: contextId3});
   },
 
+  async function testAddBindingToMultipleContextsById() {
+    const {contextGroup, sessions: [session]} = setupSessions(1);
+    const contextId1 = (await session.Protocol.Runtime.onceExecutionContextCreated()).params.context.id;
+
+    contextGroup.createContext();
+    const contextId2 = (await session.Protocol.Runtime.onceExecutionContextCreated()).params.context.id;
+
+    await session.Protocol.Runtime.addBinding({name: 'frobnicate', executionContextId: contextId1});
+    await session.Protocol.Runtime.addBinding({name: 'frobnicate', executionContextId: contextId2});
+    const expression = `frobnicate('message')`;
+
+    InspectorTest.log('Call binding in default context (binding should be exposed)');
+    await session.Protocol.Runtime.evaluate({expression});
+
+    InspectorTest.log('Call binding in target context (binding should be exposed)');
+    await session.Protocol.Runtime.evaluate({expression, contextId: contextId2});
+  },
+
+  async function testAddBindingToMultipleContextsInDifferentContextGroups() {
+    const sessions1 = setupSessions(1, 'group1/');
+    const session1 = sessions1.sessions[0];
+    const contextId1 = (await session1.Protocol.Runtime.onceExecutionContextCreated()).params.context.id;
+
+    const sessions2 = setupSessions(1, 'group2/');
+    const session2 = sessions2.sessions[0];
+    const contextId2 = (await session2.Protocol.Runtime.onceExecutionContextCreated()).params.context.id;
+
+    await session1.Protocol.Runtime.addBinding({name: 'frobnicate', executionContextId: contextId1});
+    await session2.Protocol.Runtime.addBinding({name: 'frobnicate', executionContextId: contextId2});
+    const expression = `frobnicate('message')`;
+
+    InspectorTest.log('Call binding in default context (binding should be exposed)');
+    await session1.Protocol.Runtime.evaluate({expression, contextId: contextId1});
+
+    InspectorTest.log('Call binding in target context (binding should be exposed)');
+    await session2.Protocol.Runtime.evaluate({expression, contextId: contextId2});
+  },
+
   async function testAddBindingToContextByName() {
     const {contextGroup, sessions: [session]} = setupSessions(1);
     const defaultContext = (await session.Protocol.Runtime.onceExecutionContextCreated()).params.context.id;
@@ -134,7 +172,7 @@ InspectorTest.runAsyncTestSuite([
 
 ]);
 
-function setupSessions(num) {
+function setupSessions(num, prefix = '') {
   const contextGroup = new InspectorTest.ContextGroup();
   const sessions = [];
   for (let i = 0; i < num; ++i) {
@@ -142,7 +180,7 @@ function setupSessions(num) {
     sessions.push(session);
     session.Protocol.Runtime.enable();
     session.Protocol.Runtime.onBindingCalled(msg => {
-      InspectorTest.log(`binding called in session${i + 1}`);
+      InspectorTest.log(`binding called in ${prefix}session${i + 1}`);
       InspectorTest.logMessage(msg);
     });
   }