[promisehook] Pass deferred promise to Before/After callback

[promisehook] Pass deferred promise to Before/After callback

Before, in `var p1 = p.then(() => {}) we would trigger the
before/after callbacks with p as the associated promise, but we must
call it with p1.

Also removes promise from PromiseReactionJobInfo.

Review-Url: https://codereview.chromium.org/2633443002
Cr-Commit-Position: refs/heads/master@{#42295}
Committed: https://chromium.googlesource.com/v8/v8/+/687b60c874227906b987758885c2e8d3e6858c52

[gsathya, 2017-01-12 08:12:10]

The CQ bit was checked by gsathya@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-12 08:12:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-12 08:17:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-12 08:17:29]

Dry run: Try jobs failed on following builders:
  v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)

[gsathya, 2017-01-12 08:45:58]

Description was changed from

==========
[promisehook] Pass deferred promise to Before/After callback

Before, in `var p1 = p.then(() => {}` we would trigger the
before/after callbacks with p as the associated promise, but we must
call it with p1.
==========

to

==========
[promisehook] Pass deferred promise to Before/After callback

Before, in `var p1 = p.then(() => {}` we would trigger the
before/after callbacks with p as the associated promise, but we must
call it with p1.

Also removes promise from PromiseReactionJobInfo.
==========

[gsathya, 2017-01-12 08:45:58]

gsathya@chromium.org changed reviewers:
+ adamk@chromium.org, littledan@chromium.org

[gsathya, 2017-01-12 08:47:13]

The CQ bit was checked by gsathya@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-12 08:47:20]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[gsathya, 2017-01-12 09:03:52]

The CQ bit was checked by gsathya@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-12 09:04:01]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-12 09:31:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-12 09:31:11]

Dry run: This issue passed the CQ dry run.

[adamk, 2017-01-12 18:24:27]

lgtm once comments are resolved

https://codereview.chromium.org/2633443002/diff/40001/src/bootstrapper.cc
File src/bootstrapper.cc (right):

https://codereview.chromium.org/2633443002/diff/40001/src/bootstrapper.cc#new...
src/bootstrapper.cc:1950: isolate, factory->empty_string(),
Builtins::kPromiseHandle, 5, true);
While you're modifying this line...why does this need an adapter frame?

https://codereview.chromium.org/2633443002/diff/40001/src/runtime/runtime-pro...
File src/runtime/runtime-promise.cc (right):

https://codereview.chromium.org/2633443002/diff/40001/src/runtime/runtime-pro...
src/runtime/runtime-promise.cc:196: DCHECK_EQ(3, args.length());
I think you also need to update runtime.h

[gsathya, 2017-01-12 19:13:15]

The CQ bit was checked by gsathya@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-12 19:13:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[gsathya, 2017-01-12 19:21:47]

https://codereview.chromium.org/2633443002/diff/40001/src/bootstrapper.cc
File src/bootstrapper.cc (right):

https://codereview.chromium.org/2633443002/diff/40001/src/bootstrapper.cc#new...
src/bootstrapper.cc:1950: isolate, factory->empty_string(),
Builtins::kPromiseHandle, 5, true);
On 2017/01/12 18:24:26, adamk wrote:
> While you're modifying this line...why does this need an adapter frame?

Done.

https://codereview.chromium.org/2633443002/diff/40001/src/runtime/runtime-pro...
File src/runtime/runtime-promise.cc (right):

https://codereview.chromium.org/2633443002/diff/40001/src/runtime/runtime-pro...
src/runtime/runtime-promise.cc:196: DCHECK_EQ(3, args.length());
On 2017/01/12 18:24:27, adamk wrote:
> I think you also need to update runtime.h

That was always wrong, now it's correct /o\

[adamk, 2017-01-12 19:42:01]

https://codereview.chromium.org/2633443002/diff/40001/src/runtime/runtime-pro...
File src/runtime/runtime-promise.cc (right):

https://codereview.chromium.org/2633443002/diff/40001/src/runtime/runtime-pro...
src/runtime/runtime-promise.cc:196: DCHECK_EQ(3, args.length());
On 2017/01/12 19:21:47, gsathya wrote:
> On 2017/01/12 18:24:27, adamk wrote:
> > I think you also need to update runtime.h
> 
> That was always wrong, now it's correct /o\

Hmm, I guess the length in the .h file is only used when the function is called
from JavaScript. So it's becoming more and more of a no-op.

[commit-bot: I haz the power, 2017-01-12 19:59:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-12 19:59:47]

Dry run: This issue passed the CQ dry run.

[esprehn, 2017-01-12 21:59:16]

Description was changed from

==========
[promisehook] Pass deferred promise to Before/After callback

Before, in `var p1 = p.then(() => {}` we would trigger the
before/after callbacks with p as the associated promise, but we must
call it with p1.

Also removes promise from PromiseReactionJobInfo.
==========

to

==========
[promisehook] Pass deferred promise to Before/After callback

Before, in `var p1 = p.then(() => {}) we would trigger the
before/after callbacks with p as the associated promise, but we must
call it with p1.

Also removes promise from PromiseReactionJobInfo.
==========

[gsathya, 2017-01-12 22:04:32]

The CQ bit was checked by gsathya@chromium.org

[gsathya, 2017-01-12 22:04:33]

The patchset sent to the CQ was uploaded after l-g-t-m from adamk@chromium.org
Link to the patchset: https://codereview.chromium.org/2633443002/#ps60001
(title: "Remove adaptor")

[commit-bot: I haz the power, 2017-01-12 22:04:38]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-12 22:06:57]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1484258672792790,
"parent_rev": "aee244b0e115f4c3aef56055b739777dde94c3ac", "commit_rev":
"687b60c874227906b987758885c2e8d3e6858c52"}

[commit-bot: I haz the power, 2017-01-12 22:07:02]

Description was changed from

==========
[promisehook] Pass deferred promise to Before/After callback

Before, in `var p1 = p.then(() => {}) we would trigger the
before/after callbacks with p as the associated promise, but we must
call it with p1.

Also removes promise from PromiseReactionJobInfo.
==========

to

==========
[promisehook] Pass deferred promise to Before/After callback

Before, in `var p1 = p.then(() => {}) we would trigger the
before/after callbacks with p as the associated promise, but we must
call it with p1.

Also removes promise from PromiseReactionJobInfo.

Review-Url: https://codereview.chromium.org/2633443002
Cr-Commit-Position: refs/heads/master@{#42295}
Committed:
https://chromium.googlesource.com/v8/v8/+/687b60c874227906b987758885c2e8d3e68...
==========

[commit-bot: I haz the power, 2017-01-12 22:07:04]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/v8/v8/+/687b60c874227906b987758885c2e8d3e68...