Enable V8 Promises.

Enable V8 Promises.

Enable V8 Promises.
This CL sets --harmony-promises flag for the V8 implementation.
Because V8 Promises depend on weak collections, this CL also introduces
WeakMap and WeakSet.

This CL makes ScriptPromise and ScriptPromiseResolver use V8 Promises instead
of Blink Promises. Now ScriptPromiseResolverWithContext runs V8 tasks manually
when resolve or reject was called from a non script execution environment.

BUG=352597, 352552
R=jochen@chromium.org, adamk@chromium.org

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=172299

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=172542

[jochen (gone - plz use gerrit), 2014-04-22 06:49:41]

lgtm with comment

https://codereview.chromium.org/243703007/diff/40001/Source/bindings/v8/Scrip...
File Source/bindings/v8/ScriptPromiseResolverWithContext.h (right):

https://codereview.chromium.org/243703007/diff/40001/Source/bindings/v8/Scrip...
Source/bindings/v8/ScriptPromiseResolverWithContext.h:78: class RetainThis;
we'd usually just ref() and deref()

[yhirano, 2014-04-22 07:49:05]

Thanks, Jochen.
I've created this new CL as suggested by Adam in the old CL[1].

Adam, can you take a look at this CL?

1: https://codereview.chromium.org/209853010/

https://codereview.chromium.org/243703007/diff/40001/Source/bindings/v8/Scrip...
File Source/bindings/v8/ScriptPromiseResolverWithContext.h (right):

https://codereview.chromium.org/243703007/diff/40001/Source/bindings/v8/Scrip...
Source/bindings/v8/ScriptPromiseResolverWithContext.h:78: class RetainThis;
On 2014/04/22 06:49:41, jochen wrote:
> we'd usually just ref() and deref()

Done.

[adamk, 2014-04-22 18:09:22]

lgtm

[yhirano, 2014-04-22 23:15:11]

The CQ bit was checked by yhirano@chromium.org

[commit-bot: I haz the power, 2014-04-22 23:15:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yhirano@chromium.org/243703007/60001

[commit-bot: I haz the power, 2014-04-22 23:49:41]

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

[commit-bot: I haz the power, 2014-04-22 23:49:42]

Try jobs failed on following builders:
  tryserver.blink on mac_blink_rel

[yhirano, 2014-04-23 00:01:42]

The CQ bit was checked by yhirano@chromium.org

[commit-bot: I haz the power, 2014-04-23 00:02:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yhirano@chromium.org/243703007/60001

[commit-bot: I haz the power, 2014-04-23 00:33:12]

Change committed as 172299

[yhirano, 2014-04-23 05:40:34]

Reverted due to the FileManagerBrowserTest breakage.
+hirono@chromium.org

[yhirano, 2014-04-23 06:58:02]

Hirono-san (not Hirano :) ) is looking into the breakage. Thanks!

By the way, I chatted with haraken this morning and He gave me an idea which is
posting a ExcecutionContextTask that merely calls v8::V8::RunMicroTasks.
The benefit is, we can ease the restriction that forbids the developers to enter
a V8 context manually.
By talking with some Promises users, I think the restriction is very hard to
understand and it's great if we can ease it.
I implemented so at PS6. Jochen, Adam, What do you think about the idea and the
implementation?

[jochen (gone - plz use gerrit), 2014-04-23 07:07:46]

fine by me

lgtm

[adamk, 2014-04-23 18:07:01]

On 2014/04/23 06:58:02, yhirano wrote:
> Hirono-san (not Hirano :) ) is looking into the breakage. Thanks!
> 
> By the way, I chatted with haraken this morning and He gave me an idea which
is
> posting a ExcecutionContextTask that merely calls v8::V8::RunMicroTasks.

My worry with this is that it takes us even further away from the "correct"
timing (wherein Promises resolve within the same task they're resolved in).

> The benefit is, we can ease the restriction that forbids the developers to
enter
> a V8 context manually.

I must have missed something, how did this restriction come about?

> By talking with some Promises users, I think the restriction is very hard to
> understand and it's great if we can ease it.
> I implemented so at PS6. Jochen, Adam, What do you think about the idea and
the
> implementation?

[yhirano, 2014-04-24 01:22:17]

> > The benefit is, we can ease the restriction that forbids the developers to
> enter
> > a V8 context manually.
> 
> I must have missed something, how did this restriction come about?
> 
PS4 implementation does:
 1. If resolve() is called in a V8 context, it calles
v8::Promise::Resolver::Resolve.
 2. If resolve() is called out of a V8 context, it sets a timer that does:
  2a. call v8::Promise::Resolver::Resolve.
  2b. call v8::V8::RunMicrotasks.
If a ScriptPromiseResolverWithContext user enters a V8 context manually in a
blink task trigerred by an IPC and calls resolve(), no one would run microtasks.
So it is important to not enter a V8 context manually.

> > By the way, I chatted with haraken this morning and He gave me an idea which
> is
> > posting a ExcecutionContextTask that merely calls v8::V8::RunMicroTasks.
> 
> My worry with this is that it takes us even further away from the "correct"
> timing (wherein Promises resolve within the same task they're resolved in).
> 
PS6 implementation calls v8::Promise::Resolver::Resolve immediately (except the
suspended case).
If it is called in a script execution environment, the posted microtasks will
run at the end of the blink task as usual. The posted blink task will do
nothing. There is no difference with the first case in PS4.
Otherwise, the posted microtasks may run at the posted blink task. This is
similar to the second case in PS4.

[adamk, 2014-04-24 17:33:58]

On 2014/04/24 01:22:17, yhirano wrote:
> > > The benefit is, we can ease the restriction that forbids the developers to
> > enter
> > > a V8 context manually.
> > 
> > I must have missed something, how did this restriction come about?
> > 
> PS4 implementation does:
>  1. If resolve() is called in a V8 context, it calles
> v8::Promise::Resolver::Resolve.
>  2. If resolve() is called out of a V8 context, it sets a timer that does:
>   2a. call v8::Promise::Resolver::Resolve.
>   2b. call v8::V8::RunMicrotasks.
> If a ScriptPromiseResolverWithContext user enters a V8 context manually in a
> blink task trigerred by an IPC and calls resolve(), no one would run
microtasks.
> So it is important to not enter a V8 context manually.
> 
> > > By the way, I chatted with haraken this morning and He gave me an idea
which
> > is
> > > posting a ExcecutionContextTask that merely calls v8::V8::RunMicroTasks.
> > 
> > My worry with this is that it takes us even further away from the "correct"
> > timing (wherein Promises resolve within the same task they're resolved in).
> > 
> PS6 implementation calls v8::Promise::Resolver::Resolve immediately (except
the
> suspended case).
> If it is called in a script execution environment, the posted microtasks will
> run at the end of the blink task as usual. The posted blink task will do
> nothing. There is no difference with the first case in PS4.
> Otherwise, the posted microtasks may run at the posted blink task. This is
> similar to the second case in PS4.

Thanks for the detailed response (and the explanation of how this works if
resolve is called inside a scripting context), lgtm

[adamk, 2014-04-24 17:34:16]

https://codereview.chromium.org/243703007/diff/120001/Source/bindings/v8/Scri...
File Source/bindings/v8/ScriptPromiseResolverWithContext.h (right):

https://codereview.chromium.org/243703007/diff/120001/Source/bindings/v8/Scri...
Source/bindings/v8/ScriptPromiseResolverWithContext.h:31: // If you use
ScriptPromiseResolverWithContext::resolve or reject from a
Is this comment now dead?

[yhirano, 2014-04-24 21:20:01]

https://codereview.chromium.org/243703007/diff/120001/Source/bindings/v8/Scri...
File Source/bindings/v8/ScriptPromiseResolverWithContext.h (right):

https://codereview.chromium.org/243703007/diff/120001/Source/bindings/v8/Scri...
Source/bindings/v8/ScriptPromiseResolverWithContext.h:31: // If you use
ScriptPromiseResolverWithContext::resolve or reject from a
On 2014/04/24 17:34:17, adamk wrote:
> Is this comment now dead?

Yes, Thanks!

[yhirano, 2014-04-24 21:20:09]

The CQ bit was checked by yhirano@chromium.org

[commit-bot: I haz the power, 2014-04-24 21:20:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yhirano@chromium.org/243703007/140001

[commit-bot: I haz the power, 2014-04-24 22:57:49]

Change committed as 172542

[kouhei (in TOK), 2014-04-25 04:28:53]

Looks like all LayoutTests using Promises are leaking after this patch.
Investigating.

[yhirano, 2014-04-25 04:54:35]

Thanks.
Do fast/js/Promise-native-* tests leak as well?

[kouhei (in TOK), 2014-04-25 05:20:40]

On 2014/04/25 04:54:35, yhirano wrote:
> Thanks.
> Do fast/js/Promise-native-* tests leak as well?

fast/js/Promise-native-resolve.html leaks
fast/js/Promise-native-create.html do not.

[kouhei (in TOK), 2014-04-25 05:52:29]

FYI: minimal repro case is:
<script>
internals.createResolvedPromise(null);
</script>