inspector: mark all pauses on promise rejection with proper reason

Sometimes we do not have promise on stack, e.g. Promise.reject call,
but we need to attribute this pause with promise rejection.

TBR=yangguo@chromium.org

Bug: chromium:755728
Cq-Include-Trybots: luci.chromium.try:linux_chromium_headless_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I03ca1e1cd6c21677f0a12ece626e2c8a1938437b
Reviewed-on: https://chromium-review.googlesource.com/1249942Reviewed-by: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56293}

src/debug/debug-interface.h

@@ -154,6 +154,8 @@ void GetLoadedScripts(Isolate* isolate, PersistentValueVector<Script>& scripts);
 MaybeLocal<UnboundScript> CompileInspectorScript(Isolate* isolate,
                                                  Local<String> source);
 
+enum ExceptionType { kException, kPromiseRejection };
+
 class DebugDelegate {
  public:
   virtual ~DebugDelegate() = default;
@@ -166,8 +168,8 @@ class DebugDelegate {
       const std::vector<debug::BreakpointId>& inspector_break_points_hit) {}
   virtual void ExceptionThrown(v8::Local<v8::Context> paused_context,
                                v8::Local<v8::Value> exception,
-                               v8::Local<v8::Value> promise, bool is_uncaught) {
-  }
+                               v8::Local<v8::Value> promise, bool is_uncaught,
+                               ExceptionType exception_type) {}
   virtual bool IsFunctionBlackboxed(v8::Local<debug::Script> script,
                                     const debug::Location& start,
                                     const debug::Location& end) {

src/debug/debug.cc

@@ -1653,7 +1653,10 @@ void Debug::OnThrow(Handle<Object> exception) {
     scheduled_exception = handle(isolate_->scheduled_exception(), isolate_);
     isolate_->clear_scheduled_exception();
   }
-  OnException(exception, isolate_->GetPromiseOnStackOnThrow());
+  Handle<Object> maybe_promise = isolate_->GetPromiseOnStackOnThrow();
+  OnException(exception, maybe_promise,
+              maybe_promise->IsJSPromise() ? v8::debug::kPromiseRejection
+                                           : v8::debug::kException);
   if (!scheduled_exception.is_null()) {
     isolate_->thread_local_top()->scheduled_exception_ = *scheduled_exception;
   }
@@ -1668,7 +1671,7 @@ void Debug::OnPromiseReject(Handle<Object> promise, Handle<Object> value) {
   if (!promise->IsJSObject() ||
       JSReceiver::GetDataProperty(Handle<JSObject>::cast(promise), key)
           ->IsUndefined(isolate_)) {
-    OnException(value, promise);
+    OnException(value, promise, v8::debug::kPromiseRejection);
   }
 }
 
@@ -1693,7 +1696,8 @@ bool Debug::IsFrameBlackboxed(JavaScriptFrame* frame) {
   return true;
 }
 
-void Debug::OnException(Handle<Object> exception, Handle<Object> promise) {
+void Debug::OnException(Handle<Object> exception, Handle<Object> promise,
+                        v8::debug::ExceptionType exception_type) {
   // TODO(kozyatinskiy): regress-662674.js test fails on arm without this.
   if (!AllowJavascriptExecution::IsAllowed(isolate_)) return;
 
@@ -1739,9 +1743,9 @@ void Debug::OnException(Handle<Object> exception, Handle<Object> promise) {
   DisableBreak no_recursive_break(this);
 
   Handle<Context> native_context(isolate_->native_context());
-  debug_delegate_->ExceptionThrown(v8::Utils::ToLocal(native_context),
-                                   v8::Utils::ToLocal(exception),
-                                   v8::Utils::ToLocal(promise), uncaught);
+  debug_delegate_->ExceptionThrown(
+      v8::Utils::ToLocal(native_context), v8::Utils::ToLocal(exception),
+      v8::Utils::ToLocal(promise), uncaught, exception_type);
 }
 
 void Debug::OnDebugBreak(Handle<FixedArray> break_points_hit) {

src/debug/debug.h

@@ -410,7 +410,8 @@ class Debug {
 
   bool IsExceptionBlackboxed(bool uncaught);
 
-  void OnException(Handle<Object> exception, Handle<Object> promise);
+  void OnException(Handle<Object> exception, Handle<Object> promise,
+                   v8::debug::ExceptionType exception_type);
 
   void ProcessCompileEvent(bool has_compile_error, Handle<Script> script);
 

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

@@ -333,7 +333,7 @@ void V8DebuggerAgentImpl::enableImpl() {
 
   if (isPaused()) {
     didPause(0, v8::Local<v8::Value>(), std::vector<v8::debug::BreakpointId>(),
-             false, false, false, false);
+             v8::debug::kException, false, false, false);
   }
 }
 
@@ -1506,7 +1506,8 @@ void V8DebuggerAgentImpl::didParseSource(
 void V8DebuggerAgentImpl::didPause(
     int contextId, v8::Local<v8::Value> exception,
     const std::vector<v8::debug::BreakpointId>& hitBreakpoints,
-    bool isPromiseRejection, bool isUncaught, bool isOOMBreak, bool isAssert) {
+    v8::debug::ExceptionType exceptionType, bool isUncaught, bool isOOMBreak,
+    bool isAssert) {
   v8::HandleScope handles(m_isolate);
 
   std::vector<BreakReason> hitReasons;
@@ -1522,7 +1523,7 @@ void V8DebuggerAgentImpl::didPause(
     m_session->findInjectedScript(contextId, injectedScript);
     if (injectedScript) {
       String16 breakReason =
-          isPromiseRejection
+          exceptionType == v8::debug::kPromiseRejection
               ? protocol::Debugger::Paused::ReasonEnum::PromiseRejection
               : protocol::Debugger::Paused::ReasonEnum::Exception;
       std::unique_ptr<protocol::Runtime::RemoteObject> obj;

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

@@ -142,8 +142,8 @@ class V8DebuggerAgentImpl : public protocol::Debugger::Backend {
   // Interface for V8InspectorImpl
   void didPause(int contextId, v8::Local<v8::Value> exception,
                 const std::vector<v8::debug::BreakpointId>& hitBreakpoints,
-                bool isPromiseRejection, bool isUncaught, bool isOOMBreak,
-                bool isAssert);
+                v8::debug::ExceptionType exceptionType, bool isUncaught,
+                bool isOOMBreak, bool isAssert);
   void didContinue();
   void didParseSource(std::unique_ptr<V8DebuggerScript>, bool success);
 

src/inspector/v8-debugger.cc

@@ -480,7 +480,7 @@ void V8Debugger::clearContinueToLocation() {
 void V8Debugger::handleProgramBreak(
     v8::Local<v8::Context> pausedContext, v8::Local<v8::Value> exception,
     const std::vector<v8::debug::BreakpointId>& breakpointIds,
-    bool isPromiseRejection, bool isUncaught) {
+    v8::debug::ExceptionType exceptionType, bool isUncaught) {
   // Don't allow nested breaks.
   if (isPaused()) return;
 
@@ -523,12 +523,12 @@ void V8Debugger::handleProgramBreak(
 
   m_inspector->forEachSession(
       contextGroupId, [&pausedContext, &exception, &breakpointIds,
-                       &isPromiseRejection, &isUncaught, &scheduledOOMBreak,
+                       &exceptionType, &isUncaught, &scheduledOOMBreak,
                        &scheduledAssertBreak](V8InspectorSessionImpl* session) {
         if (session->debuggerAgent()->acceptsPause(scheduledOOMBreak)) {
           session->debuggerAgent()->didPause(
               InspectedContext::contextId(pausedContext), exception,
-              breakpointIds, isPromiseRejection, isUncaught, scheduledOOMBreak,
+              breakpointIds, exceptionType, isUncaught, scheduledOOMBreak,
               scheduledAssertBreak);
         }
       });
@@ -608,12 +608,11 @@ void V8Debugger::BreakProgramRequested(
 
 void V8Debugger::ExceptionThrown(v8::Local<v8::Context> pausedContext,
                                  v8::Local<v8::Value> exception,
-                                 v8::Local<v8::Value> promise,
-                                 bool isUncaught) {
-  bool isPromiseRejection = promise->IsPromise();
+                                 v8::Local<v8::Value> promise, bool isUncaught,
+                                 v8::debug::ExceptionType exceptionType) {
   std::vector<v8::debug::BreakpointId> break_points_hit;
-  handleProgramBreak(pausedContext, exception, break_points_hit,
-                     isPromiseRejection, isUncaught);
+  handleProgramBreak(pausedContext, exception, break_points_hit, exceptionType,
+                     isUncaught);
 }
 
 bool V8Debugger::IsFunctionBlackboxed(v8::Local<v8::debug::Script> script,

src/inspector/v8-debugger.h

@@ -145,7 +145,8 @@ class V8Debugger : public v8::debug::DebugDelegate,
   void handleProgramBreak(
       v8::Local<v8::Context> pausedContext, v8::Local<v8::Value> exception,
       const std::vector<v8::debug::BreakpointId>& hitBreakpoints,
-      bool isPromiseRejection = false, bool isUncaught = false);
+      v8::debug::ExceptionType exception_type = v8::debug::kException,
+      bool isUncaught = false);
 
   enum ScopeTargetKind {
     FUNCTION,
@@ -181,7 +182,8 @@ class V8Debugger : public v8::debug::DebugDelegate,
       const std::vector<v8::debug::BreakpointId>& break_points_hit) override;
   void ExceptionThrown(v8::Local<v8::Context> paused_context,
                        v8::Local<v8::Value> exception,
-                       v8::Local<v8::Value> promise, bool is_uncaught) override;
+                       v8::Local<v8::Value> promise, bool is_uncaught,
+                       v8::debug::ExceptionType exception_type) override;
   bool IsFunctionBlackboxed(v8::Local<v8::debug::Script> script,
                             const v8::debug::Location& start,
                             const v8::debug::Location& end) override;

test/cctest/test-debug.cc

@@ -3068,8 +3068,8 @@ class ContextCheckEventListener : public v8::debug::DebugDelegate {
   }
   void ExceptionThrown(v8::Local<v8::Context> paused_context,
                        v8::Local<v8::Value> exception,
-                       v8::Local<v8::Value> promise,
-                       bool is_uncaught) override {
+                       v8::Local<v8::Value> promise, bool is_uncaught,
+                       v8::debug::ExceptionType) override {
     CheckContext();
   }
   bool IsFunctionBlackboxed(v8::Local<v8::debug::Script> script,
@@ -3838,8 +3838,8 @@ class DebugEventExpectNoException : public v8::debug::DebugDelegate {
  public:
   void ExceptionThrown(v8::Local<v8::Context> paused_context,
                        v8::Local<v8::Value> exception,
-                       v8::Local<v8::Value> promise,
-                       bool is_uncaught) override {
+                       v8::Local<v8::Value> promise, bool is_uncaught,
+                       v8::debug::ExceptionType) override {
     CHECK(false);
   }
 };

test/inspector/debugger/pause-on-promise-rejections-expected.txt

+Test Debugger.paused reason for promise rejections
+Check Promise.reject in script:
+promiseRejection
+
+Check Promise.reject in Runtime.evaluate:
+promiseRejection
+
+Check Promise.reject in async function:
+promiseRejection
+
+Check throw in async function:
+promiseRejection
+
+Check reject from constructor:
+promiseRejection
+
+Check reject from thenable job:
+promiseRejection
+
+Check caught exception in async function (should be exception):
+exception
+

test/inspector/debugger/pause-on-promise-rejections.js

+// 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 {session, contextGroup, Protocol} =
+    InspectorTest.start('Test Debugger.paused reason for promise rejections');
+
+(async function test() {
+  Protocol.Debugger.enable();
+  Protocol.Debugger.setPauseOnExceptions({state: 'all'});
+  InspectorTest.log('Check Promise.reject in script:');
+  contextGroup.addScript(`Promise.reject(new Error())`);
+  await logPausedReason();
+  await Protocol.Debugger.resume();
+
+  InspectorTest.log('Check Promise.reject in Runtime.evaluate:');
+  Protocol.Runtime.evaluate({expression: `Promise.reject(new Error())`});
+  await logPausedReason();
+  await Protocol.Debugger.resume();
+
+  InspectorTest.log('Check Promise.reject in async function:');
+  Protocol.Runtime.evaluate(
+      {expression: `(async function() { await Promise.reject(); })()`});
+  await logPausedReason();
+  await Protocol.Debugger.resume();
+
+  InspectorTest.log('Check throw in async function:');
+  Protocol.Runtime.evaluate({
+    expression: `(async function() { await Promise.resolve(); throw 42; })()`
+  });
+  await logPausedReason();
+  await Protocol.Debugger.resume();
+
+  InspectorTest.log('Check reject from constructor:');
+  Protocol.Runtime.evaluate({
+    expression: 'new Promise((_, reject) => reject(new Error())).catch(e => {})'
+  });
+  await logPausedReason();
+  await Protocol.Debugger.resume();
+
+  InspectorTest.log('Check reject from thenable job:');
+  Protocol.Runtime.evaluate({
+    expression:
+        `Promise.resolve().then(() => Promise.reject(new Error())).catch(e => 0)`
+  });
+  await logPausedReason();
+  await Protocol.Debugger.resume();
+
+  InspectorTest.log(
+      'Check caught exception in async function (should be exception):');
+  Protocol.Runtime.evaluate({
+    expression: `(async function() {
+      await Promise.resolve();
+      try {
+        throw 42;
+      } catch (e) {}
+    })()`
+  });
+  await logPausedReason();
+  await Protocol.Debugger.resume();
+
+  InspectorTest.completeTest();
+})();
+
+async function logPausedReason() {
+  const {params: {reason}} = await Protocol.Debugger.oncePaused();
+  InspectorTest.log(reason + '\n');
+}