[inspector] expose throwOnSideEffect for Runtime.evaluate

Bug: chromium:810176
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_chromium_rel_ng
Change-Id: I16e4148434f5cbf44058e1aa5f01693bcba82d0a
Reviewed-on: https://chromium-review.googlesource.com/932943
Commit-Queue: Erik Luo <luoe@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51640}

src/api.cc

@@ -9677,12 +9677,14 @@ v8::Local<debug::GeneratorObject> debug::GeneratorObject::Cast(
 }
 
 MaybeLocal<v8::Value> debug::EvaluateGlobal(v8::Isolate* isolate,
-                                            v8::Local<v8::String> source) {
+                                            v8::Local<v8::String> source,
+                                            bool throw_on_side_effect) {
   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)),
+      i::DebugEvaluate::Global(internal_isolate, Utils::OpenHandle(*source),
+                               throw_on_side_effect),
       &result);
   RETURN_ON_FAILED_EXECUTION(Value);
   RETURN_ESCAPED(result);

src/debug/debug-evaluate.cc

@@ -22,7 +22,8 @@ namespace v8 {
 namespace internal {
 
 MaybeHandle<Object> DebugEvaluate::Global(Isolate* isolate,
-                                          Handle<String> source) {
+                                          Handle<String> source,
+                                          bool throw_on_side_effect) {
   Handle<Context> context = isolate->native_context();
   ScriptOriginOptions origin_options(false, true);
   MaybeHandle<SharedFunctionInfo> maybe_function_info =
@@ -37,6 +38,7 @@ MaybeHandle<Object> DebugEvaluate::Global(Isolate* isolate,
   Handle<JSFunction> fun =
       isolate->factory()->NewFunctionFromSharedFunctionInfo(shared_info,
                                                             context);
+  NoSideEffectScope no_side_effect(isolate, throw_on_side_effect);
   return Execution::Call(isolate, fun,
                          Handle<JSObject>(context->global_proxy()), 0, nullptr);
 }

src/debug/debug-evaluate.h

@@ -18,7 +18,8 @@ class FrameInspector;
 
 class DebugEvaluate : public AllStatic {
  public:
-  static MaybeHandle<Object> Global(Isolate* isolate, Handle<String> source);
+  static MaybeHandle<Object> Global(Isolate* isolate, Handle<String> source,
+                                    bool throw_on_side_effect);
 
   // Evaluate a piece of JavaScript in the context of a stack frame for
   // debugging.  Things that need special attention are:

src/debug/debug-interface.h

@@ -498,7 +498,8 @@ 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);
+                                         v8::Local<v8::String> source,
+                                         bool throw_on_side_effect);
 
 }  // namespace debug
 }  // namespace v8

src/inspector/js_protocol.json

@@ -2573,6 +2573,13 @@
                             "description": "Whether execution should `await` for resulting value and return once awaited promise is\nresolved.",
                             "optional": true,
                             "type": "boolean"
+                        },
+                        {
+                            "name": "throwOnSideEffect",
+                            "description": "Whether to throw an exception if side effect cannot be ruled out during evaluation.",
+                            "experimental": true,
+                            "optional": true,
+                            "type": "boolean"
                         }
                     ],
                     "returns": [

src/inspector/js_protocol.pdl

@@ -1188,6 +1188,8 @@ domain Runtime
       # Whether execution should `await` for resulting value and return once awaited promise is
       # resolved.
       optional boolean awaitPromise
+      # Whether to throw an exception if side effect cannot be ruled out during evaluation.
+      experimental optional boolean throwOnSideEffect
     returns
       # Evaluation result.
       RemoteObject result

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

@@ -227,7 +227,8 @@ void V8RuntimeAgentImpl::evaluate(
     Maybe<bool> includeCommandLineAPI, Maybe<bool> silent,
     Maybe<int> executionContextId, Maybe<bool> returnByValue,
     Maybe<bool> generatePreview, Maybe<bool> userGesture,
-    Maybe<bool> awaitPromise, std::unique_ptr<EvaluateCallback> callback) {
+    Maybe<bool> awaitPromise, Maybe<bool> throwOnSideEffect,
+    std::unique_ptr<EvaluateCallback> callback) {
   TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("devtools.timeline"),
                "EvaluateScript");
   int contextId = 0;
@@ -259,7 +260,8 @@ void V8RuntimeAgentImpl::evaluate(
     v8::MicrotasksScope microtasksScope(m_inspector->isolate(),
                                         v8::MicrotasksScope::kRunMicrotasks);
     maybeResultValue = v8::debug::EvaluateGlobal(
-        m_inspector->isolate(), toV8String(m_inspector->isolate(), expression));
+        m_inspector->isolate(), toV8String(m_inspector->isolate(), expression),
+        throwOnSideEffect.fromMaybe(false));
   }  // Run microtasks before returning result.
 
   if (evalIsDisabled) scope.context()->AllowCodeGenerationFromStrings(false);

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

@@ -63,7 +63,7 @@ class V8RuntimeAgentImpl : public protocol::Runtime::Backend {
                 Maybe<bool> includeCommandLineAPI, Maybe<bool> silent,
                 Maybe<int> executionContextId, Maybe<bool> returnByValue,
                 Maybe<bool> generatePreview, Maybe<bool> userGesture,
-                Maybe<bool> awaitPromise,
+                Maybe<bool> awaitPromise, Maybe<bool> throwOnSideEffect,
                 std::unique_ptr<EvaluateCallback>) override;
   void awaitPromise(const String16& promiseObjectId, Maybe<bool> returnByValue,
                     Maybe<bool> generatePreview,

src/runtime/runtime-debug.cc

@@ -1143,7 +1143,8 @@ RUNTIME_FUNCTION(Runtime_DebugEvaluateGlobal) {
 
   CONVERT_ARG_HANDLE_CHECKED(String, source, 1);
 
-  RETURN_RESULT_OR_FAILURE(isolate, DebugEvaluate::Global(isolate, source));
+  RETURN_RESULT_OR_FAILURE(isolate,
+                           DebugEvaluate::Global(isolate, source, false));
 }
 
 

test/inspector/runtime/evaluate-without-side-effects-expected.txt

@@ -0,0 +1,50 @@
+Tests that Runtime.evaluate can run without side effects.
+Test throwOnSideEffect: false
+{
+    id : <messageId>
+    result : {
+        result : {
+            description : 2
+            type : number
+            value : 2
+        }
+    }
+}
+Test expression with side-effect, with throwOnSideEffect: true
+{
+    id : <messageId>
+    result : {
+        exceptionDetails : {
+            columnNumber : -1
+            exception : {
+                className : EvalError
+                description : EvalError: Possible side-effect in debug-evaluate
+                objectId : <objectId>
+                subtype : error
+                type : object
+            }
+            exceptionId : <exceptionId>
+            lineNumber : -1
+            scriptId : <scriptId>
+            text : Uncaught
+        }
+        result : {
+            className : EvalError
+            description : EvalError: Possible side-effect in debug-evaluate
+            objectId : <objectId>
+            subtype : error
+            type : object
+        }
+    }
+}
+Test expression without side-effect, with throwOnSideEffect: true
+{
+    id : <messageId>
+    result : {
+        result : {
+            description : 4
+            type : number
+            value : 4
+        }
+    }
+}

test/inspector/runtime/evaluate-without-side-effects.js

@@ -0,0 +1,29 @@
+// 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 Runtime.evaluate can run without side effects.");
+
+Protocol.Runtime.enable();
+Protocol.Debugger.enable();
+(async function() {
+  InspectorTest.log("Test throwOnSideEffect: false");
+  InspectorTest.logMessage(await Protocol.Runtime.evaluate({
+    expression: "var x = 2; x;",
+    throwOnSideEffect: false
+  }));
+
+  InspectorTest.log("Test expression with side-effect, with throwOnSideEffect: true");
+  InspectorTest.logMessage(await Protocol.Runtime.evaluate({
+    expression: "x = 3; x;",
+    throwOnSideEffect: true
+  }));
+
+  InspectorTest.log("Test expression without side-effect, with throwOnSideEffect: true");
+  InspectorTest.logMessage(await Protocol.Runtime.evaluate({
+    expression: "x * 2",
+    throwOnSideEffect: true
+  }));
+
+  InspectorTest.completeTest();
+})();