 Chromium Code Reviews
 Chromium Code Reviews Issue 2314903004:
  [promises] Move PromiseResolveThenableJob to c++  (Closed)
    
  
    Issue 2314903004:
  [promises] Move PromiseResolveThenableJob to c++  (Closed) 
  | Index: src/isolate.cc | 
| diff --git a/src/isolate.cc b/src/isolate.cc | 
| index c7b7486d1e6be0e650d1357e7ca9357efc445745..97b7fb6b9156497426307bcd8b6613f84e5a3f7a 100644 | 
| --- a/src/isolate.cc | 
| +++ b/src/isolate.cc | 
| @@ -2914,9 +2914,42 @@ void Isolate::ReportPromiseReject(Handle<JSObject> promise, | 
| v8::Utils::StackTraceToLocal(stack_trace))); | 
| } | 
| +void Isolate::PromiseResolveThenableJob(Handle<PromiseContainer> container) { | 
| + MaybeHandle<Object> maybe_exception; | 
| 
adamk
2016/09/08 22:28:18
This should be declared right before it's used.
 
gsathya
2016/09/14 00:39:14
Done.
 | 
| + Handle<JSObject> promise(container->promise(), this); | 
| + Handle<JSObject> thenable(container->thenable(), this); | 
| + Handle<JSFunction> then(container->then(), this); | 
| + Handle<JSFunction> resolve(container->resolve(), this); | 
| + Handle<JSFunction> reject(container->reject(), this); | 
| + Handle<Object> before_debug_event(container->before_debug_event(), this); | 
| + Handle<Object> after_debug_event(container->after_debug_event(), this); | 
| 
adamk
2016/09/08 22:28:18
I don't think you need all these handles up front,
 
gsathya
2016/09/14 00:39:13
Done.
 | 
| + Handle<Object> argv[] = {resolve, reject}; | 
| 
adamk
2016/09/08 22:28:18
This can also move down to right before the call
 
gsathya
2016/09/14 00:39:13
Done.
 | 
| + | 
| + if (debug()->is_active() && (*before_debug_event)->IsJSObject()) { | 
| 
adamk
2016/09/08 22:28:17
Handles already overload operator-> so you don't n
 
gsathya
2016/09/14 00:39:13
Done.
 | 
| + Handle<JSObject> event_data(JSObject::cast(*before_debug_event), this); | 
| 
adamk
2016/09/08 22:28:17
No need to create a new handle here...
 
gsathya
2016/09/14 00:39:13
Done.
 | 
| + debug()->OnAsyncTaskEvent(event_data); | 
| 
adamk
2016/09/08 22:28:17
...instead use Handle<JSObject>::cast(before_debug
 
gsathya
2016/09/14 00:39:13
Done.
 | 
| + } | 
| + | 
| + MaybeHandle<Object> result = Execution::TryCall( | 
| + this, then, thenable, arraysize(argv), argv, &maybe_exception); | 
| + | 
| + Handle<Object> reason; | 
| + if (maybe_exception.ToHandle(&reason)) { | 
| + Handle<Object> reason_arg[] = {reason}; | 
| + MaybeHandle<Object> status = | 
| + Execution::Call(this, reject, factory()->undefined_value(), | 
| + arraysize(reason_arg), reason_arg); | 
| + } | 
| + | 
| + if (debug()->is_active() && (*after_debug_event)->IsJSObject()) { | 
| 
adamk
2016/09/08 22:28:17
Same applies here and below to the handling of aft
 
gsathya
2016/09/14 00:39:13
Done.
 | 
| + Handle<JSObject> event_data(JSObject::cast(*after_debug_event), this); | 
| + debug()->OnAsyncTaskEvent(event_data); | 
| + } | 
| +} | 
| void Isolate::EnqueueMicrotask(Handle<Object> microtask) { | 
| - DCHECK(microtask->IsJSFunction() || microtask->IsCallHandlerInfo()); | 
| + DCHECK(microtask->IsJSFunction() || microtask->IsCallHandlerInfo() || | 
| + microtask->IsPromiseContainer()); | 
| Handle<FixedArray> queue(heap()->microtask_queue(), this); | 
| int num_tasks = pending_microtask_count(); | 
| DCHECK(num_tasks <= queue->length()); | 
| @@ -2977,13 +3010,17 @@ void Isolate::RunMicrotasksInternal() { | 
| set_pending_microtask_count(0); | 
| return; | 
| } | 
| - } else { | 
| + } else 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 { | 
| + Handle<PromiseContainer> container = | 
| + Handle<PromiseContainer>::cast(microtask); | 
| + PromiseResolveThenableJob(container); | 
| } | 
| }); | 
| } |