DevTools: Runtime.callFunctionOn should allow evals

This patch teaches Runtime.callFunctionOn to run evals inside its
task. This is aligned with how Runtime.evaluate works.

R=kozy

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I3189beebc9020703d2b2e23a7c828a2394d43910
Reviewed-on: https://chromium-review.googlesource.com/994070
Commit-Queue: Andrey Lushnikov <lushnikov@chromium.org>
Reviewed-by: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52339}

src/inspector/injected-script.cc

@@ -654,6 +654,7 @@ InjectedScript::Scope::Scope(V8InspectorSessionImpl* session)
       m_ignoreExceptionsAndMuteConsole(false),
       m_previousPauseOnExceptionsState(v8::debug::NoBreakOnException),
       m_userGesture(false),
+      m_allowEval(false),
       m_contextGroupId(session->contextGroupId()),
       m_sessionId(session->sessionId()) {}
 
@@ -666,6 +667,7 @@ Response InjectedScript::Scope::initialize() {
   if (!response.isSuccess()) return response;
   m_context = m_injectedScript->context()->context();
   m_context->Enter();
+  if (m_allowEval) m_context->AllowCodeGenerationFromStrings(true);
   return Response::OK();
 }
 
@@ -701,9 +703,17 @@ void InjectedScript::Scope::pretendUserGesture() {
   m_inspector->client()->beginUserGesture();
 }
 
+void InjectedScript::Scope::allowCodeGenerationFromStrings() {
+  DCHECK(!m_allowEval);
+  if (m_context->IsCodeGenerationFromStringsAllowed()) return;
+  m_allowEval = true;
+  m_context->AllowCodeGenerationFromStrings(true);
+}
+
 void InjectedScript::Scope::cleanup() {
   m_commandLineAPIScope.reset();
   if (!m_context.IsEmpty()) {
+    if (m_allowEval) m_context->AllowCodeGenerationFromStrings(false);
     m_context->Exit();
     m_context.Clear();
   }

src/inspector/injected-script.h

@@ -120,6 +120,7 @@ class InjectedScript final {
     void installCommandLineAPI();
     void ignoreExceptionsAndMuteConsole();
     void pretendUserGesture();
+    void allowCodeGenerationFromStrings();
     v8::Local<v8::Context> context() const { return m_context; }
     InjectedScript* injectedScript() const { return m_injectedScript; }
     const v8::TryCatch& tryCatch() const { return m_tryCatch; }
@@ -144,6 +145,7 @@ class InjectedScript final {
     bool m_ignoreExceptionsAndMuteConsole;
     v8::debug::ExceptionBreakState m_previousPauseOnExceptionsState;
     bool m_userGesture;
+    bool m_allowEval;
     int m_contextGroupId;
     int m_sessionId;
   };

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

@@ -136,6 +136,9 @@ void innerCallFunctionOn(
   if (silent) scope.ignoreExceptionsAndMuteConsole();
   if (userGesture) scope.pretendUserGesture();
 
+  // Temporarily enable allow evals for inspector.
+  scope.allowCodeGenerationFromStrings();
+
   v8::MaybeLocal<v8::Value> maybeFunctionValue;
   v8::Local<v8::Script> functionScript;
   if (inspector
@@ -253,9 +256,8 @@ void V8RuntimeAgentImpl::evaluate(
 
   if (includeCommandLineAPI.fromMaybe(false)) scope.installCommandLineAPI();
 
-  bool evalIsDisabled = !scope.context()->IsCodeGenerationFromStringsAllowed();
   // Temporarily enable allow evals for inspector.
-  if (evalIsDisabled) scope.context()->AllowCodeGenerationFromStrings(true);
+  scope.allowCodeGenerationFromStrings();
 
   v8::MaybeLocal<v8::Value> maybeResultValue;
   {
@@ -266,8 +268,6 @@ void V8RuntimeAgentImpl::evaluate(
         throwOnSideEffect.fromMaybe(false));
   }  // Run microtasks before returning result.
 
-  if (evalIsDisabled) scope.context()->AllowCodeGenerationFromStrings(false);
-
   // Re-initialize after running client's code, as it could have destroyed
   // context or session.
   response = scope.initialize();

test/inspector/debugger/eval-without-codegen-expected.txt

@@ -30,3 +30,23 @@ Running test: testEvaluatePaused
         }
     }
 }
+
+Running test: testCallFunctionOn
+{
+    id : <messageId>
+    result : {
+        result : {
+            description : 3
+            type : number
+            value : 3
+        }
+    }
+}
+{
+    id : <messageId>
+    result : {
+        result : {
+            type : undefined
+        }
+    }
+}

test/inspector/debugger/eval-without-codegen.js

@@ -34,5 +34,15 @@ InspectorTest.runAsyncTestSuite([
     InspectorTest.logMessage(await Protocol.Debugger.evaluateOnCallFrame(
         {callFrameId, expression: 'local'}));
     await Protocol.Debugger.resume();
+  },
+
+  async function testCallFunctionOn() {
+    await contextGroup.addScript(`inspector.setAllowCodeGenerationFromStrings(false);`);
+    const globalObject = await Protocol.Runtime.evaluate({expression: 'this'});
+    const objectId = globalObject.result.result.objectId;
+    InspectorTest.logMessage(await Protocol.Runtime.callFunctionOn({objectId, functionDeclaration: 'function() { return eval("1 + 2"); }'}));
+
+    await contextGroup.addScript(`this.value = eval("1 + 2");`);
+    InspectorTest.logMessage(await Protocol.Runtime.evaluate({expression: 'this.value'}));
   }
 ]);