[promises] Refactor debug code

src/runtime/runtime-promise.cc

 // Copyright 2016 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.
 #include "src/runtime/runtime-utils.h"
 
 #include "src/debug/debug.h"
 #include "src/elements.h"
 #include "src/promise-utils.h"
 
 namespace v8 {
@@ skipping 42 matching lines @@
   HandleScope scope(isolate);
   CONVERT_ARG_HANDLE_CHECKED(JSPromise, promise, 0);
   // At this point, no revocation has been issued before
   CHECK(!promise->has_handler());
   isolate->ReportPromiseReject(promise, Handle<Object>(),
                                v8::kPromiseHandlerAddedAfterReject);
   return isolate->heap()->undefined_value();
 }
 
 namespace {
+
+void PromiseFulfillDebugInfo(Isolate* isolate,

[adamk, 2016/12/30 00:10:04]

This doesn't "fulfill" anything...should this be called "SetDebugInfo"?

[gsathya, 2016/12/30 01:48:51]

Done.

+                             Handle<PromiseReactionJobInfo> info, int status) {
+  int id = isolate->GetNextDebugMicrotaskId();
+  DCHECK(status != v8::Promise::kPending);
+  PromiseDebugActionName name = status == v8::Promise::kFulfilled
+                                    ? DEBUG_PROMISE_RESOLVE
+                                    : DEBUG_PROMISE_REJECT;
+
+  isolate->debug()->OnAsyncTaskEvent(DEBUG_ENQUEUE, id, name);
+  info->set_debug_id(id);
+  info->set_debug_name(name);
+}
+
+// In an async function, reuse the existing stack related to the outer
+// Promise. Otherwise, e.g. in a direct call to then, save a new stack.
+// Promises with multiple reactions with one or more of them being async
+// functions will not get a good stack trace, as async functions require
+// different stacks from direct Promise use, but we save and restore a
+// stack once for all reactions.
+//
+// If this isn't a case of async function, then we fall back to normal
+// promise resolution debug events.
+//
+// TODO(littledan): Improve this case.
+void AsyncFunctionDebugInfo(Isolate* isolate,

[adamk, 2016/12/30 00:10:04]

This function name reads oddly to me, it seems to be missing a verb. What does
the function _do_? Maybe "EmitAsyncFunctionDebugInfo"?

[gsathya, 2016/12/30 01:48:51]

Done.

+                            Handle<PromiseReactionJobInfo> info, int status) {
+  // deferred_promise can be Undefined, FixedArray or userland promise object.
+  if (!info->deferred_promise()->IsJSPromise()) {
+    PromiseFulfillDebugInfo(isolate, info, status);
+    return;

[adamk, 2016/12/30 00:10:04]

I think it would be clearer to factor these multiple returns as, instead, a
helper function that takes care of determining if we're dealing with an async
function.

[gsathya, 2016/12/30 01:48:51]

Done.

+  }
+
+  Handle<JSPromise> deferred_promise(JSPromise::cast(info->deferred_promise()),
+                                     isolate);
+  Handle<Symbol> handled_by_symbol =
+      isolate->factory()->promise_handled_by_symbol();
+  Handle<Object> handled_by_promise =
+      JSObject::GetDataProperty(deferred_promise, handled_by_symbol);
+
+  if (!handled_by_promise->IsJSPromise()) {
+    PromiseFulfillDebugInfo(isolate, info, status);
+    return;
+  }
+
+  Handle<JSPromise> handled_by_promise_js =
+      Handle<JSPromise>::cast(handled_by_promise);
+  Handle<Symbol> async_stack_id_symbol =
+      isolate->factory()->promise_async_stack_id_symbol();
+  Handle<Object> id =
+      JSObject::GetDataProperty(handled_by_promise_js, async_stack_id_symbol);
+
+  // id can be Undefined or Smi.
+  if (!id->IsSmi()) {
+    PromiseFulfillDebugInfo(isolate, info, status);
+    return;
+  }
+
+  info->set_debug_id(Handle<Smi>::cast(id)->value());
+  info->set_debug_name(DEBUG_ASYNC_FUNCTION);
+}
+
 void EnqueuePromiseReactionJob(Isolate* isolate,
                                Handle<PromiseReactionJobInfo> info,
-                               Handle<Object> status) {
+                               int status) {
   if (isolate->debug()->is_active()) {
-    MaybeHandle<Object> maybe_result;
-    Handle<Object> deferred_obj(info->deferred_promise(), isolate);
+    AsyncFunctionDebugInfo(isolate, info, status);
+  }
 
-    if (info->deferred_promise()->IsFixedArray()) {
-      deferred_obj = isolate->factory()->undefined_value();
-    }
-
-    Handle<Object> argv[] = {deferred_obj, status};
-    maybe_result = Execution::TryCall(
-        isolate, isolate->promise_debug_get_info(),
-        isolate->factory()->undefined_value(), arraysize(argv), argv);
-
-    Handle<Object> result;
-    if ((maybe_result).ToHandle(&result)) {
-      CHECK(result->IsJSArray());
-      Handle<JSArray> array = Handle<JSArray>::cast(result);
-      ElementsAccessor* accessor = array->GetElementsAccessor();
-      DCHECK(accessor->HasElement(array, 0));
-      DCHECK(accessor->HasElement(array, 1));
-      info->set_debug_id(*accessor->Get(array, 0));
-      info->set_debug_name(*accessor->Get(array, 1));
-    }
-  }
   isolate->EnqueueMicrotask(info);
 }
 
 void PromiseSet(Isolate* isolate, Handle<JSPromise> promise, int status,
                 Handle<Object> result) {
   promise->set_status(status);
   promise->set_result(*result);
   promise->set_deferred_promise(isolate->heap()->undefined_value());
   promise->set_deferred_on_resolve(isolate->heap()->undefined_value());
   promise->set_deferred_on_reject(isolate->heap()->undefined_value());
   promise->set_fulfill_reactions(isolate->heap()->undefined_value());
   promise->set_reject_reactions(isolate->heap()->undefined_value());
 }
 
-void PromiseFulfill(Isolate* isolate, Handle<JSPromise> promise,
-                    Handle<Smi> status, Handle<Object> value) {
+void PromiseFulfill(Isolate* isolate, Handle<JSPromise> promise, int status,
+                    Handle<Object> value) {
   // Check if there are any callbacks.
   if (!promise->deferred_promise()->IsUndefined(isolate)) {
-    Handle<Object> tasks((status->value() == v8::Promise::kFulfilled)
+    Handle<Object> tasks((status == v8::Promise::kFulfilled)
                              ? promise->fulfill_reactions()
                              : promise->reject_reactions(),
                          isolate);
     Handle<PromiseReactionJobInfo> info =
         isolate->factory()->NewPromiseReactionJobInfo(
             promise, value, tasks, handle(promise->deferred_promise(), isolate),
             handle(promise->deferred_on_resolve(), isolate),
             handle(promise->deferred_on_reject(), isolate),
             isolate->native_context());
     EnqueuePromiseReactionJob(isolate, info, status);
   }
 
-  PromiseSet(isolate, promise, status->value(), value);
+  PromiseSet(isolate, promise, status, value);
 }
 
 }  // namespace
 
 RUNTIME_FUNCTION(Runtime_PromiseReject) {
   DCHECK(args.length() == 3);
   HandleScope scope(isolate);
   CONVERT_ARG_HANDLE_CHECKED(JSPromise, promise, 0);
   CONVERT_ARG_HANDLE_CHECKED(Object, reason, 1);
   CONVERT_BOOLEAN_ARG_CHECKED(debug_event, 2);
 
   PromiseRejectEvent(isolate, promise, promise, reason, debug_event);
+  PromiseFulfill(isolate, promise, v8::Promise::kRejected, reason);
 
-  Handle<Smi> status(Smi::FromInt(v8::Promise::kRejected), isolate);
-  PromiseFulfill(isolate, promise, status, reason);
   return isolate->heap()->undefined_value();
 }
 
 RUNTIME_FUNCTION(Runtime_PromiseFulfill) {
   DCHECK(args.length() == 3);
   HandleScope scope(isolate);
   CONVERT_ARG_HANDLE_CHECKED(JSPromise, promise, 0);
-  CONVERT_ARG_HANDLE_CHECKED(Smi, status, 1);
+  CONVERT_SMI_ARG_CHECKED(status, 1);
   CONVERT_ARG_HANDLE_CHECKED(Object, value, 2);
   PromiseFulfill(isolate, promise, status, value);
   return isolate->heap()->undefined_value();
 }
 
 RUNTIME_FUNCTION(Runtime_EnqueuePromiseReactionJob) {
   HandleScope scope(isolate);
   DCHECK(args.length() == 2);
   CONVERT_ARG_HANDLE_CHECKED(PromiseReactionJobInfo, info, 0);
-  CONVERT_ARG_HANDLE_CHECKED(Object, status, 1);
+  CONVERT_SMI_ARG_CHECKED(status, 1);
   EnqueuePromiseReactionJob(isolate, info, status);
   return isolate->heap()->undefined_value();
 }
 
 RUNTIME_FUNCTION(Runtime_EnqueuePromiseResolveThenableJob) {
   HandleScope scope(isolate);
   DCHECK(args.length() == 3);
   CONVERT_ARG_HANDLE_CHECKED(JSObject, promise, 0);
   CONVERT_ARG_HANDLE_CHECKED(JSReceiver, resolution, 1);
   CONVERT_ARG_HANDLE_CHECKED(JSReceiver, then, 2);
 
   // TODO(gsathya): Add fast path for native promises with unmodified
   // PromiseThen (which don't need these resolving functions, but
   // instead can just call resolve/reject directly).
   Handle<JSFunction> resolve, reject;
   PromiseUtils::CreateResolvingFunctions(
       isolate, promise, isolate->factory()->false_value(), &resolve, &reject);
 
-  Handle<Object> debug_id, debug_name;
+  int debug_id = 0;

[adamk, 2016/12/30 00:10:04]

Please define a constant somewhere for this 0, something like
kInvalidPromiseDebugId.

[gsathya, 2016/12/30 01:48:51]

Done.

+  PromiseDebugActionName debug_name = DEBUG_NOT_ACTIVE;
   if (isolate->debug()->is_active()) {
-    debug_id =
-        handle(Smi::FromInt(isolate->GetNextDebugMicrotaskId()), isolate);
-    debug_name = isolate->factory()->PromiseResolveThenableJob_string();
-    isolate->debug()->OnAsyncTaskEvent(isolate->factory()->enqueue_string(),
-                                       debug_id,
-                                       Handle<String>::cast(debug_name));
-  } else {
-    debug_id = isolate->factory()->undefined_value();
-    debug_name = isolate->factory()->undefined_value();
+    debug_id = isolate->GetNextDebugMicrotaskId();
+    debug_name = DEBUG_PROMISE_RESOLVE_THENABLE_JOB;
+    isolate->debug()->OnAsyncTaskEvent(DEBUG_ENQUEUE, debug_id, debug_name);
   }
 
   Handle<PromiseResolveThenableJobInfo> info =
       isolate->factory()->NewPromiseResolveThenableJobInfo(
           resolution, then, resolve, reject, debug_id, debug_name,
           isolate->native_context());
   isolate->EnqueueMicrotask(info);
 
   return isolate->heap()->undefined_value();
 }
@@ skipping 131 matching lines @@
   HandleScope scope(isolate);
   DCHECK_EQ(1, args.length());
   CONVERT_ARG_HANDLE_CHECKED(JSPromise, promise, 0);
   isolate->RunPromiseHook(PromiseHookType::kAfter, promise,
                           isolate->factory()->undefined_value());
   return isolate->heap()->undefined_value();
 }
 
 }  // namespace internal
 }  // namespace v8