Set RuntimeAgent evaluate to use DebugEvaluate::Global

Bug: chromium:810176
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_chromium_rel_ng
Change-Id: I330fa0bdf81d0bb926cf6db794736e89c069f8f2
Reviewed-on: https://chromium-review.googlesource.com/907707
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Commit-Queue: Erik Luo <luoe@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51333}

src/api.cc

@@ -33,6 +33,7 @@
 #include "src/conversions-inl.h"
 #include "src/counters.h"
 #include "src/debug/debug-coverage.h"
+#include "src/debug/debug-evaluate.h"
 #include "src/debug/debug-type-profile.h"
 #include "src/debug/debug.h"
 #include "src/deoptimizer.h"
@@ -9735,6 +9736,18 @@ v8::Local<debug::GeneratorObject> debug::GeneratorObject::Cast(
   return ToApiHandle<debug::GeneratorObject>(Utils::OpenHandle(*value));
 }
 
+MaybeLocal<v8::Value> debug::EvaluateGlobal(v8::Isolate* isolate,
+                                            v8::Local<v8::String> source) {
+  i::Isolate* internal_isolate = reinterpret_cast<i::Isolate*>(isolate);
+  PREPARE_FOR_DEBUG_INTERFACE_EXECUTION_WITH_ISOLATE(internal_isolate, Value);
+  Local<Value> result;
+  has_pending_exception = !ToLocal<Value>(
+      i::DebugEvaluate::Global(internal_isolate, Utils::OpenHandle(*source)),
+      &result);
+  RETURN_ON_FAILED_EXECUTION(Value);
+  RETURN_ESCAPED(result);
+}
+
 void debug::QueryObjects(v8::Local<v8::Context> v8_context,
                          QueryObjectPredicate* predicate,
                          PersistentValueVector<v8::Object>* objects) {

src/debug/debug-evaluate.cc

@@ -21,29 +21,25 @@
 namespace v8 {
 namespace internal {
 
-static inline bool IsDebugContext(Isolate* isolate, Context* context) {
-  return context->native_context() == *isolate->debug()->debug_context();
-}
-
 MaybeHandle<Object> DebugEvaluate::Global(Isolate* isolate,
                                           Handle<String> source) {
-  // Handle the processing of break.
-  DisableBreak disable_break_scope(isolate->debug());
-
-  // Enter the top context from before the debugger was invoked.
-  SaveContext save(isolate);
-  SaveContext* top = &save;
-  while (top != nullptr && IsDebugContext(isolate, *top->context())) {
-    top = top->prev();
-  }
-  if (top != nullptr) isolate->set_context(*top->context());
-
-  // Get the native context now set to the top context from before the
-  // debugger was invoked.
   Handle<Context> context = isolate->native_context();
-  Handle<JSObject> receiver(context->global_proxy());
-  Handle<SharedFunctionInfo> outer_info(context->closure()->shared(), isolate);
-  return Evaluate(isolate, outer_info, context, receiver, source, false);
+  ScriptOriginOptions origin_options(false, true);
+  MaybeHandle<SharedFunctionInfo> maybe_function_info =
+      Compiler::GetSharedFunctionInfoForScript(
+          source, isolate->factory()->empty_string(), 0, 0, origin_options,
+          MaybeHandle<Object>(), context, nullptr, nullptr,
+          ScriptCompiler::kNoCompileOptions, ScriptCompiler::kNoCacheNoReason,
+          NOT_NATIVES_CODE, MaybeHandle<FixedArray>());
+
+  Handle<SharedFunctionInfo> shared_info;
+  if (!maybe_function_info.ToHandle(&shared_info)) return MaybeHandle<Object>();
+
+  Handle<JSFunction> fun =
+      isolate->factory()->NewFunctionFromSharedFunctionInfo(shared_info,
+                                                            context);
+  return Execution::Call(isolate, fun,
+                         Handle<JSObject>(context->global_proxy()), 0, nullptr);
 }
 
 MaybeHandle<Object> DebugEvaluate::Local(Isolate* isolate,

src/debug/debug-interface.h

@@ -499,6 +499,9 @@ int GetNativeAccessorDescriptor(v8::Local<v8::Context> context,
 
 int64_t GetNextRandomInt64(v8::Isolate* isolate);
 
+v8::MaybeLocal<v8::Value> EvaluateGlobal(v8::Isolate* isolate,
+                                         v8::Local<v8::String> source);
+
 }  // namespace debug
 }  // namespace v8
 

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

@@ -255,13 +255,12 @@ void V8RuntimeAgentImpl::evaluate(
   if (evalIsDisabled) scope.context()->AllowCodeGenerationFromStrings(true);
 
   v8::MaybeLocal<v8::Value> maybeResultValue;
-  v8::Local<v8::Script> script;
-  if (m_inspector->compileScript(scope.context(), expression, String16())
-          .ToLocal(&script)) {
+  {
     v8::MicrotasksScope microtasksScope(m_inspector->isolate(),
                                         v8::MicrotasksScope::kRunMicrotasks);
-    maybeResultValue = script->Run(scope.context());
-  }
+    maybeResultValue = v8::debug::EvaluateGlobal(
+        m_inspector->isolate(), toV8String(m_inspector->isolate(), expression));
+  }  // Run microtasks before returning result.
 
   if (evalIsDisabled) scope.context()->AllowCodeGenerationFromStrings(false);
 

test/cctest/test-debug.cc

@@ -2352,72 +2352,6 @@ TEST(ConditionalBreakpointWithCodeGenerationDisallowed) {
 }
 
 
-bool checkedDebugEvals = true;
-v8::Local<v8::Function> checkGlobalEvalFunction;
-v8::Local<v8::Function> checkFrameEvalFunction;
-static void CheckDebugEval(const v8::Debug::EventDetails& eventDetails) {
-  if (eventDetails.GetEvent() == v8::Break) {
-    ++debugEventCount;
-    v8::HandleScope handleScope(CcTest::isolate());
-
-    v8::Local<v8::Value> args[] = {eventDetails.GetExecutionState()};
-    CHECK(
-        checkGlobalEvalFunction->Call(eventDetails.GetEventContext(),
-                                      eventDetails.GetEventContext()->Global(),
-                                      1, args)
-            .ToLocalChecked()
-            ->IsTrue());
-    CHECK(checkFrameEvalFunction->Call(eventDetails.GetEventContext(),
-                                       eventDetails.GetEventContext()->Global(),
-                                       1, args)
-              .ToLocalChecked()
-              ->IsTrue());
-  }
-}
-
-
-// Test that the evaluation of expressions when a break point is hit generates
-// the correct results in case code generation from strings is disallowed in the
-// debugee context.
-TEST(DebugEvaluateWithCodeGenerationDisallowed) {
-  DebugLocalContext env;
-  v8::HandleScope scope(env->GetIsolate());
-  env.ExposeDebug();
-
-  SetDebugEventListener(env->GetIsolate(), CheckDebugEval);
-
-  v8::Local<v8::Context> context = env.context();
-  v8::Local<v8::Function> foo = CompileFunction(&env,
-    "var global = 'Global';\n"
-    "function foo(x) {\n"
-    "  var local = 'Local';\n"
-    "  debugger;\n"
-    "  return local + x;\n"
-    "}",
-    "foo");
-  checkGlobalEvalFunction = CompileFunction(&env,
-    "function checkGlobalEval(exec_state) {\n"
-    "  return exec_state.evaluateGlobal('global').value() === 'Global';\n"
-    "}",
-    "checkGlobalEval");
-
-  checkFrameEvalFunction = CompileFunction(&env,
-    "function checkFrameEval(exec_state) {\n"
-    "  return exec_state.frame(0).evaluate('local').value() === 'Local';\n"
-    "}",
-    "checkFrameEval");
-  debugEventCount = 0;
-  env->AllowCodeGenerationFromStrings(false);
-  foo->Call(context, env->Global(), 0, nullptr).ToLocalChecked();
-  CHECK_EQ(1, debugEventCount);
-
-  checkGlobalEvalFunction.Clear();
-  checkFrameEvalFunction.Clear();
-  SetDebugEventListener(env->GetIsolate(), nullptr);
-  CheckDebuggerUnloaded();
-}
-
-
 // Simple test of the stepping mechanism using only store ICs.
 TEST(DebugStepLinear) {
   DebugLocalContext env;

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

@@ -0,0 +1,32 @@
+Tests that evaluation works when code generation from strings is not allowed.
+
+Running test: testEvaluateNotPaused
+{
+    id : <messageId>
+    result : {
+        result : {
+            type : string
+            value : Global1
+        }
+    }
+}
+
+Running test: testEvaluatePaused
+{
+    id : <messageId>
+    result : {
+        result : {
+            type : string
+            value : Global2
+        }
+    }
+}
+{
+    id : <messageId>
+    result : {
+        result : {
+            type : string
+            value : Local
+        }
+    }
+}

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

@@ -0,0 +1,38 @@
+// Copyright 2018 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+let {session, contextGroup, Protocol} = InspectorTest.start(
+    'Tests that evaluation works when code generation from strings is not allowed.');
+
+Protocol.Debugger.enable();
+Protocol.Runtime.enable();
+
+InspectorTest.runAsyncTestSuite([
+  async function testEvaluateNotPaused() {
+    contextGroup.addScript(`inspector.setAllowCodeGenerationFromStrings(false);
+    var global1 = 'Global1';`);
+    await Protocol.Debugger.onceScriptParsed();
+    InspectorTest.logMessage(
+        await Protocol.Runtime.evaluate({expression: 'global1'}));
+  },
+
+  async function testEvaluatePaused() {
+    contextGroup.addScript(`inspector.setAllowCodeGenerationFromStrings(false);
+    var global2 = 'Global2';
+    function foo(x) {
+      var local = 'Local';
+      debugger;
+      return local + x;
+    }
+    foo();`);
+    let {params: {callFrames: [{callFrameId}]}} =
+        await Protocol.Debugger.oncePaused();
+
+    InspectorTest.logMessage(
+        await Protocol.Runtime.evaluate({expression: 'global2'}));
+    InspectorTest.logMessage(await Protocol.Debugger.evaluateOnCallFrame(
+        {callFrameId, expression: 'local'}));
+    await Protocol.Debugger.resume();
+  }
+]);

test/inspector/inspector-test.cc

@@ -707,6 +707,10 @@ class InspectorExtension : public IsolateData::SetupGlobalTask {
     inspector->Set(ToV8String(isolate, "scheduleWithAsyncStack"),
                    v8::FunctionTemplate::New(
                        isolate, &InspectorExtension::ScheduleWithAsyncStack));
+    inspector->Set(
+        ToV8String(isolate, "setAllowCodeGenerationFromStrings"),
+        v8::FunctionTemplate::New(
+            isolate, &InspectorExtension::SetAllowCodeGenerationFromStrings));
     global->Set(ToV8String(isolate, "inspector"), inspector);
   }
 
@@ -957,6 +961,17 @@ class InspectorExtension : public IsolateData::SetupGlobalTask {
                                     v8::Local<v8::Function>::Cast(args[0])));
     if (with_empty_stack) context->Enter();
   }
+
+  static void SetAllowCodeGenerationFromStrings(
+      const v8::FunctionCallbackInfo<v8::Value>& args) {
+    if (args.Length() != 1 || !args[0]->IsBoolean()) {
+      fprintf(stderr,
+              "Internal error: setAllowCodeGenerationFromStrings(allow).");
+      Exit();
+    }
+    args.GetIsolate()->GetCurrentContext()->AllowCodeGenerationFromStrings(
+        args[0].As<v8::Boolean>()->Value());
+  }
 };
 
 }  //  namespace

test/inspector/runtime/evaluate-run-microtasks-expected.txt

@@ -0,0 +1,39 @@
+Tests that microtasks run before the Runtime.evaluate response is sent
+{
+    method : Runtime.consoleAPICalled
+    params : {
+        args : [
+            [0] : {
+                description : 42
+                type : number
+                value : 42
+            }
+        ]
+        executionContextId : <executionContextId>
+        stackTrace : {
+            callFrames : [
+                [0] : {
+                    columnNumber : 37
+                    functionName : Promise.resolve.then
+                    lineNumber : 0
+                    scriptId : <scriptId>
+                    url : 
+                }
+            ]
+        }
+        timestamp : <timestamp>
+        type : log
+    }
+}
+{
+    id : <messageId>
+    result : {
+        result : {
+            className : Promise
+            description : Promise
+            objectId : <objectId>
+            subtype : promise
+            type : object
+        }
+    }
+}

test/inspector/runtime/evaluate-run-microtasks.js

@@ -0,0 +1,13 @@
+// Copyright 2018 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+const {Protocol} = InspectorTest.start(
+    'Tests that microtasks run before the Runtime.evaluate response is sent');
+
+Protocol.Runtime.enable();
+Protocol.Runtime.onConsoleAPICalled(InspectorTest.logMessage);
+Protocol.Runtime
+    .evaluate({expression: 'Promise.resolve().then(() => console.log(42))'})
+    .then(InspectorTest.logMessage)
+    .then(InspectorTest.completeTest);