[promises] Move PromiseReactionJob to c++

src/isolate.cc

 // Copyright 2012 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/isolate.h"
 
 #include <stdlib.h>
 
 #include <fstream>  // NOLINT(readability/streams)
 #include <sstream>
 
 #include "src/ast/context-slot-cache.h"
 #include "src/base/hashmap.h"
 #include "src/base/platform/platform.h"
 #include "src/base/sys-info.h"
 #include "src/base/utils/random-number-generator.h"
 #include "src/basic-block-profiler.h"
 #include "src/bootstrapper.h"
 #include "src/cancelable-task.h"
 #include "src/codegen.h"
 #include "src/compilation-cache.h"
 #include "src/compilation-statistics.h"
 #include "src/compiler-dispatcher/optimizing-compile-dispatcher.h"
 #include "src/crankshaft/hydrogen.h"
 #include "src/debug/debug.h"
 #include "src/deoptimizer.h"
+#include "src/elements.h"
 #include "src/external-reference-table.h"
 #include "src/frames-inl.h"
 #include "src/ic/access-compiler-data.h"
 #include "src/ic/stub-cache.h"
 #include "src/interface-descriptors.h"
 #include "src/interpreter/interpreter.h"
 #include "src/isolate-inl.h"
 #include "src/libsampler/sampler.h"
 #include "src/log.h"
 #include "src/messages.h"
@@ skipping 2964 matching lines @@
   if (promise_reject_callback_ == NULL) return;
   Handle<JSArray> stack_trace;
   if (event == v8::kPromiseRejectWithNoHandler && value->IsJSObject()) {
     stack_trace = GetDetailedStackTrace(Handle<JSObject>::cast(value));
   }
   promise_reject_callback_(v8::PromiseRejectMessage(
       v8::Utils::PromiseToLocal(promise), event, v8::Utils::ToLocal(value),
       v8::Utils::StackTraceToLocal(stack_trace)));
 }
 
+void Isolate::PromiseReactionJob(Handle<PromiseHandleInfo> promise_handle_info,
+                                 MaybeHandle<Object>* result,
+                                 MaybeHandle<Object>* maybe_exception) {
+  if (debug()->is_active()) {

[adamk, 2016/10/11 23:14:30]

I think you could create a helper class that'd encapsulate this work. You could
put it on the stack, passing |this|, before_debug_event, and after_debug_event,
and it would take care of firing the before_debug_event at construction and the
after_debug_event on destruction.

[gsathya, 2016/10/12 02:00:43]

Done. But it's the same LoC 

+    Handle<Object> before_debug_event(promise_handle_info->before_debug_event(),
+                                      this);
+    if (before_debug_event->IsJSObject()) {
+      debug()->OnAsyncTaskEvent(Handle<JSObject>::cast(before_debug_event));
+    }
+  }
+
+  Handle<Object> value(promise_handle_info->value(), this);
+  Handle<Object> tasks(promise_handle_info->tasks(), this);
+
+  if (tasks->IsJSArray()) {

[adamk, 2016/10/11 23:14:30]

Can you add a comment here about the two possible states tasks might be in?

[gsathya, 2016/10/12 02:00:43]

Done.

+    Handle<JSArray> array = Handle<JSArray>::cast(tasks);
+    DCHECK(array->length()->IsSmi());
+    Handle<FixedArrayBase> elements(array->elements(), this);

[adamk, 2016/10/11 23:14:30]

This is now unused.

[gsathya, 2016/10/12 02:00:44]

Done.

+    int length = Smi::cast(array->length())->value();
+    ElementsAccessor* accessor = array->GetElementsAccessor();
+    for (int i = 0; i < length; i += 2) {

[adamk, 2016/10/11 23:14:30]

Please add a DCHECK that length is even (otherwise this loop is unsafe)

[gsathya, 2016/10/12 02:00:44]

Done.

+      Handle<Object> argv[] = {value, accessor->Get(array, i),

[adamk, 2016/10/11 23:14:30]

Please add the DCHECK for Has(i) and Has(i+1).

[gsathya, 2016/10/12 02:00:44]

Done.

+                               accessor->Get(array, i + 1)};
+      *result = Execution::TryCall(this, promise_handle(),

[adamk, 2016/10/11 23:14:30]

I'd hoist this out of the loop, you could store it in a handle the same place
where you create |value| and |tasks|.

[gsathya, 2016/10/12 02:00:43]

Done.

+                                   factory()->undefined_value(),

[adamk, 2016/10/11 23:14:30]

Same as the above, I'd hoist this out.

[gsathya, 2016/10/12 02:00:43]

Done.

+                                   arraysize(argv), argv, maybe_exception);

[adamk, 2016/10/11 23:14:30]

I'm not sure the same logic about result and maybe_exception holds as it did
before. This is only going to remember the last call.

[gsathya, 2016/10/12 02:00:43]

Done.

+    }
+  } else {
+    Handle<Object> deferred(promise_handle_info->deferred(), this);
+    Handle<Object> argv[] = {value, tasks, deferred};
+    *result =
+        Execution::TryCall(this, promise_handle(), factory()->undefined_value(),
+                           arraysize(argv), argv, maybe_exception);
+  }
+
+  if (debug()->is_active()) {
+    Handle<Object> after_debug_event(promise_handle_info->after_debug_event(),
+                                     this);
+    if (after_debug_event->IsJSObject()) {
+      debug()->OnAsyncTaskEvent(Handle<JSObject>::cast(after_debug_event));
+    }
+  }
+}
+
 void Isolate::PromiseResolveThenableJob(Handle<PromiseContainer> container,
                                         MaybeHandle<Object>* result,
                                         MaybeHandle<Object>* maybe_exception) {
   if (debug()->is_active()) {
     Handle<Object> before_debug_event(container->before_debug_event(), this);
     if (before_debug_event->IsJSObject()) {
       debug()->OnAsyncTaskEvent(Handle<JSObject>::cast(before_debug_event));
     }
   }
 
@@ skipping 16 matching lines @@
 
   if (debug()->is_active()) {
     Handle<Object> after_debug_event(container->after_debug_event(), this);
     if (after_debug_event->IsJSObject()) {
       debug()->OnAsyncTaskEvent(Handle<JSObject>::cast(after_debug_event));
     }
   }
 }
 
 void Isolate::EnqueueMicrotask(Handle<Object> microtask) {
+  // TODO(gsathya): remove IsJSFunction?

[adamk, 2016/10/11 23:14:30]

You'd have to change the V8 API before you could do that, so I don't think this
TODO belongs here.

[gsathya, 2016/10/12 02:00:43]

Done.

   DCHECK(microtask->IsJSFunction() || microtask->IsCallHandlerInfo() ||
-         microtask->IsPromiseContainer());
+         microtask->IsPromiseContainer() || microtask->IsPromiseHandleInfo());
   Handle<FixedArray> queue(heap()->microtask_queue(), this);
   int num_tasks = pending_microtask_count();
   DCHECK(num_tasks <= queue->length());
   if (num_tasks == 0) {
     queue = factory()->NewFixedArray(8);
     heap()->set_microtask_queue(*queue);
   } else if (num_tasks == queue->length()) {
     queue = factory()->CopyFixedArrayAndGrow(queue, num_tasks);
     heap()->set_microtask_queue(*queue);
   }
@@ skipping 31 matching lines @@
 
       if (microtask->IsCallHandlerInfo()) {
         Handle<CallHandlerInfo> callback_info =
             Handle<CallHandlerInfo>::cast(microtask);
         v8::MicrotaskCallback callback =
             v8::ToCData<v8::MicrotaskCallback>(callback_info->callback());
         void* data = v8::ToCData<void*>(callback_info->data());
         callback(data);
       } else {
         SaveContext save(this);
-        Context* context = microtask->IsJSFunction()
-                               ? Handle<JSFunction>::cast(microtask)->context()
-                               : Handle<PromiseContainer>::cast(microtask)
-                                     ->resolve()
-                                     ->context();
+        Context* context;
+        if (microtask->IsJSFunction()) {
+          context = Handle<JSFunction>::cast(microtask)->context();
+        } else if (microtask->IsPromiseContainer()) {
+          context =
+              Handle<PromiseContainer>::cast(microtask)->resolve()->context();
+        } else {
+          context =
+              Handle<PromiseHandleInfo>::cast(microtask)->reaction_context();
+        }
+
         set_context(context->native_context());
         handle_scope_implementer_->EnterMicrotaskContext(
             Handle<Context>(context, this));
 
         MaybeHandle<Object> result;
         MaybeHandle<Object> maybe_exception;
 
         if (microtask->IsJSFunction()) {
           Handle<JSFunction> microtask_function =
               Handle<JSFunction>::cast(microtask);
           result = Execution::TryCall(this, microtask_function,
                                       factory()->undefined_value(), 0, NULL,
                                       &maybe_exception);
         } else {
-          PromiseResolveThenableJob(Handle<PromiseContainer>::cast(microtask),
-                                    &result, &maybe_exception);
+          if (microtask->IsPromiseContainer()) {

[adamk, 2016/10/11 23:14:30]

This should be merged with the else above

[gsathya, 2016/10/12 02:00:43]

Done.

+            PromiseResolveThenableJob(Handle<PromiseContainer>::cast(microtask),
+                                      &result, &maybe_exception);
+          } else {
+            PromiseReactionJob(Handle<PromiseHandleInfo>::cast(microtask),
+                               &result, &maybe_exception);
+          }
         }
 
         handle_scope_implementer_->LeaveMicrotaskContext();
 
         // If execution is terminating, just bail out.
         if (result.is_null() && maybe_exception.is_null()) {
           // Clear out any remaining callbacks in the queue.
           heap()->set_microtask_queue(heap()->empty_fixed_array());
           set_pending_microtask_count(0);
           return;
@@ skipping 192 matching lines @@
   // Then check whether this scope intercepts.
   if ((flag & intercept_mask_)) {
     intercepted_flags_ |= flag;
     return true;
   }
   return false;
 }
 
 }  // namespace internal
 }  // namespace v8