[ES6 modules] Introduce Modulator "module map settings object"

[ES6 modules] Introduce Modulator "module map settings object"

This CL introduces Modulator class, which implements a HTML spec concept
"module map settings object". Modulator instance is owned by V8PerContextData.

BUG=594639
Review-Url: https://codereview.chromium.org/2639893003
Cr-Commit-Position: refs/heads/master@{#444308}
Committed: https://chromium.googlesource.com/chromium/src/+/f8f6553f21647ef987f423bb30ed260c5133a08b

[kouhei (in TOK), 2017-01-18 06:47:03]

kouhei@chromium.org changed reviewers:
+ dominicc@chromium.org, haraken@chromium.org, yhirano@chromium.org

[kouhei (in TOK), 2017-01-18 06:47:04]

kouhei@chromium.org changed required reviewers:
+ haraken@chromium.org

[kouhei (in TOK), 2017-01-18 06:48:09]

Description was changed from

==========
[ES6 modules] Introduce Modulator "module map settings object"j

This CL introduces Modulator class, which implements a HTML spec concept
"module map settings object". Modulator instance is owned by V8PerContextData.

BUG=594639
==========

to

==========
[ES6 modules] Introduce Modulator "module map settings object"

This CL introduces Modulator class, which implements a HTML spec concept
"module map settings object". Modulator instance is owned by V8PerContextData.

BUG=594639
==========

[kouhei (in TOK), 2017-01-18 06:48:16]

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

[commit-bot: I haz the power, 2017-01-18 06:48:27]

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

[haraken, 2017-01-18 06:55:56]

https://codereview.chromium.org/2639893003/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp (right):

https://codereview.chromium.org/2639893003/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp:252:
m_scriptState->perContextData()->clearModulator();

Why do you want to clear a modulator here?

https://codereview.chromium.org/2639893003/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/core/v8/V8PerContextData.h (right):

https://codereview.chromium.org/2639893003/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/V8PerContextData.h:98: void
setModulator(Modulator*);

Instead of eagerly setting a modulator, can we instantiate the modulator lazily?
i.e., instantiate the modulator on the first access to modulator().

[dominicc (has gone to gerrit), 2017-01-18 06:59:01]

lgtm

https://codereview.chromium.org/2639893003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/Modulator.cpp (right):

https://codereview.chromium.org/2639893003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/Modulator.cpp:12: return
ScriptState::forMainWorld(frame)->perContextData()->modulator();
WDYT about returning nullptr when ScriptState::forMainWorld is null? Up to you.
Depends on how you imagine this being used.

[kouhei (in TOK), 2017-01-18 07:07:10]

https://codereview.chromium.org/2639893003/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp (right):

https://codereview.chromium.org/2639893003/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/LocalWindowProxy.cpp:252:
m_scriptState->perContextData()->clearModulator();
On 2017/01/18 06:55:55, haraken wrote:
> 
> Why do you want to clear a modulator here?

Chatted offline. We shouldn't clear a modulator here.

https://codereview.chromium.org/2639893003/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/core/v8/V8PerContextData.h (right):

https://codereview.chromium.org/2639893003/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/V8PerContextData.h:98: void
setModulator(Modulator*);
On 2017/01/18 06:55:56, haraken wrote:
> 
> Instead of eagerly setting a modulator, can we instantiate the modulator
lazily?
> i.e., instantiate the modulator on the first access to modulator().

Yes. This is what we are going to do.

https://codereview.chromium.org/2639893003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/Modulator.cpp (right):

https://codereview.chromium.org/2639893003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/Modulator.cpp:12: return
ScriptState::forMainWorld(frame)->perContextData()->modulator();
On 2017/01/18 06:59:01, dominicc wrote:
> WDYT about returning nullptr when ScriptState::forMainWorld is null? Up to
you.
> Depends on how you imagine this being used.

Done.

[haraken, 2017-01-18 07:09:49]

LGTM

[kouhei (in TOK), 2017-01-18 07:18:45]

The CQ bit was checked by kouhei@chromium.org

[kouhei (in TOK), 2017-01-18 07:18:46]

The patchset sent to the CQ was uploaded after l-g-t-m from
dominicc@chromium.org
Link to the patchset: https://codereview.chromium.org/2639893003/#ps20001
(title: "review")

[commit-bot: I haz the power, 2017-01-18 07:19: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-01-18 09:15:19]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484723925666300,
"parent_rev": "73f5e80d486397d1b612966cc24562b85a603fc3", "commit_rev":
"f8f6553f21647ef987f423bb30ed260c5133a08b"}

[commit-bot: I haz the power, 2017-01-18 09:15:49]

Description was changed from

==========
[ES6 modules] Introduce Modulator "module map settings object"

This CL introduces Modulator class, which implements a HTML spec concept
"module map settings object". Modulator instance is owned by V8PerContextData.

BUG=594639
==========

to

==========
[ES6 modules] Introduce Modulator "module map settings object"

This CL introduces Modulator class, which implements a HTML spec concept
"module map settings object". Modulator instance is owned by V8PerContextData.

BUG=594639

Review-Url: https://codereview.chromium.org/2639893003
Cr-Commit-Position: refs/heads/master@{#444308}
Committed:
https://chromium.googlesource.com/chromium/src/+/f8f6553f21647ef987f423bb30ed...
==========

[commit-bot: I haz the power, 2017-01-18 09:15:50]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/f8f6553f21647ef987f423bb30ed...