Use "entered or microtask context" instead of "entered or current context"

Use "entered or microtask context" instead of "entered or current context"

R=haraken@chromium.org
BUG=

Review-Url: https://codereview.chromium.org/2738533003
Cr-Commit-Position: refs/heads/master@{#455264}
Committed: https://chromium.googlesource.com/chromium/src/+/4c5dedc05add1990762f67a148118fedc78db487

[jochen (gone - plz use gerrit), 2017-03-07 11:55:07]

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

[commit-bot: I haz the power, 2017-03-07 11:55:21]

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

[jochen (gone - plz use gerrit), 2017-03-07 11:55:49]

jochen@chromium.org changed reviewers:
+ adamk@chromium.org

[jochen (gone - plz use gerrit), 2017-03-07 11:55:49]

+adamk

alternatively, we could make microtasks enter a context (which I'd actually
prefer now that I think about it)

[jochen (gone - plz use gerrit), 2017-03-07 11:56:12]

jochen@chromium.org changed reviewers:
+ domenic@chromium.org

[haraken, 2017-03-07 11:59:18]

LGTM in short term.

On 2017/03/07 11:55:49, jochen wrote:
> +adamk
> 
> alternatively, we could make microtasks enter a context (which I'd actually
> prefer now that I think about it)

Agreed. This sounds nicer.

[jochen (gone - plz use gerrit), 2017-03-07 13:15:43]

The CQ bit was unchecked by jochen@chromium.org

[jochen (gone - plz use gerrit), 2017-03-07 13:15:46]

The CQ bit was checked by jochen@chromium.org

[commit-bot: I haz the power, 2017-03-07 13:16:02]

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

[domenic, 2017-03-07 14:36:10]

On 2017/03/07 at 11:55:49, jochen wrote:
> +adamk
> 
> alternatively, we could make microtasks enter a context (which I'd actually
prefer now that I think about it)

I think you only want to do it for ES jobs. Other microtasks, e.g. mutation
observer microtasks, should be fine already since they call Web IDL callbacks
(which should have the entered context machinery in place already). Maybe you
only *need* to do it for ES jobs, and for other microtasks it's just redundant.

The question is, what context they would enter. Boris and I discussed some ideas
for promises at https://github.com/whatwg/html/pull/1189#issuecomment-224990884
but none of them seem very generic.

If your patch here is something you think could be reasonable for the spec, I'd
love an explanation of it at https://github.com/whatwg/html/issues/1426. I tried
to understand the code but I don't know when
MicrotaskContextIsLastEnteredContext() is true or what MicrotaskContext() is.

---

Also it would be ideal to have some tests to show what this changes.

[jochen (gone - plz use gerrit), 2017-03-07 14:45:15]

On 2017/03/07 at 14:36:10, domenic wrote:
> On 2017/03/07 at 11:55:49, jochen wrote:
> > +adamk
> > 
> > alternatively, we could make microtasks enter a context (which I'd actually
prefer now that I think about it)
> 
> I think you only want to do it for ES jobs. Other microtasks, e.g. mutation
observer microtasks, should be fine already since they call Web IDL callbacks
(which should have the entered context machinery in place already). Maybe you
only *need* to do it for ES jobs, and for other microtasks it's just redundant.
> 
> The question is, what context they would enter. Boris and I discussed some
ideas for promises at
https://github.com/whatwg/html/pull/1189#issuecomment-224990884 but none of them
seem very generic.

they'd enter the creation context of the promise constructor.

> 
> If your patch here is something you think could be reasonable for the spec,
I'd love an explanation of it at https://github.com/whatwg/html/issues/1426. I
tried to understand the code but I don't know when
MicrotaskContextIsLastEnteredContext() is true or what MicrotaskContext() is.


It's true if the microtask didn't enter a context itself, and the context is the
creation context of the promise constructor.

> 
> ---
> 
> Also it would be ideal to have some tests to show what this changes.

yeah, let me try to write some

[commit-bot: I haz the power, 2017-03-07 15:45:11]

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

[commit-bot: I haz the power, 2017-03-07 15:45:12]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[adamk, 2017-03-07 18:51:44]

On 2017/03/07 11:59:18, haraken wrote:
> LGTM in short term.
> 
> On 2017/03/07 11:55:49, jochen wrote:
> > +adamk
> > 
> > alternatively, we could make microtasks enter a context (which I'd actually
> > prefer now that I think about it)
> 
> Agreed. This sounds nicer.

I could swear that I tried this before and ran into some problem. Perhaps
domenic's response points at some pitfalls...

[jochen (gone - plz use gerrit), 2017-03-07 20:01:29]

The CQ bit was checked by jochen@chromium.org

[jochen (gone - plz use gerrit), 2017-03-07 20:01:30]

two weeks ago or so, service workers stopped depending on the entered context -
that's why it should work now

[commit-bot: I haz the power, 2017-03-07 20:04:02]

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-03-07 22:45:13]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1488916889885360, "parent_rev":
"3108ddbf30b50765f5c3e10460cda36f53f62671", "commit_rev":
"4c5dedc05add1990762f67a148118fedc78db487"}

[commit-bot: I haz the power, 2017-03-07 22:45:56]

Description was changed from

==========
Use "entered or microtask context" instead of "entered or current context"

R=haraken@chromium.org
BUG=
==========

to

==========
Use "entered or microtask context" instead of "entered or current context"

R=haraken@chromium.org
BUG=

Review-Url: https://codereview.chromium.org/2738533003
Cr-Commit-Position: refs/heads/master@{#455264}
Committed:
https://chromium.googlesource.com/chromium/src/+/4c5dedc05add1990762f67a14811...
==========

[commit-bot: I haz the power, 2017-03-07 22:45:57]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/4c5dedc05add1990762f67a14811...