Support Inline module script

Support Inline module script

This CL
- Enables inline module scripts in ScriptLoader.
- Adds ModuleTreeLinker::FetchDescendantsForInlineScript() and
  related plumbing to ModuleTreeLinkerRegistry and Modulator.
  This is not yet spec'ed and we might need revisit once the spec is
  updated. https://github.com/whatwg/html/issues/2544
- Makes ModuleScript to store its |source_text| for CSP of inline scripts.

BUG=715369, 594639
Review-Url: https://codereview.chromium.org/2842923002
Cr-Commit-Position: refs/heads/master@{#468087}
Committed: https://chromium.googlesource.com/chromium/src/+/489fa402ff6ec3e74127929820b622407e979bb5

[hiroshige, 2017-04-26 02:50:32]

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

[commit-bot: I haz the power, 2017-04-26 02:51:03]

Dry run: CQ is trying da patch.

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

[hiroshige, 2017-04-26 02:51:24]

Description was changed from

==========
Support Inline module script

BUG=
==========

to

==========
Support Inline module script

Working Expectedly:
execorder.html
crossorigin.html

Crashing due to another issue:
imports.html

Crashing:
errorhandling.html

BUG=
==========

[commit-bot: I haz the power, 2017-04-26 03:31:59]

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

[commit-bot: I haz the power, 2017-04-26 03:31:59]

Dry run: 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_...)

[hiroshige, 2017-04-26 04:48:12]

hiroshige@chromium.org changed reviewers:
+ kouhei@chromium.org

[hiroshige, 2017-04-26 04:48:13]

I found the following issue (the only issue that causes inline-script-specific
test failure so far) while drafting this CL.

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

https://codereview.chromium.org/2842923002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/ModulatorImpl.cpp:89:
map_->SetInlineModuleScript(module_script);
SetInlineModuleScript() is currently needed to put |module_script| (== the
inline module script) to the module map because
ScriptModuleResolverImpl::Resolve() tries to look up |module_script| itself when
we try to instantiate |module_script|'s record.

Probably we shouldn't look up |module_script| in the module map during
instantiation.
(Because it's not clear what is the key for lookup if we continue to look up;
Currently I used the base URL, but of course this causes the problem because
there can be multiple different inline scripts with the same base URL.)

What does the spec say about this?

[hiroshige, 2017-04-26 04:54:59]

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

https://codereview.chromium.org/2842923002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/ScriptLoader.cpp:551:
modulator->FetchDescendantsForInlineScript(script, module_tree_client_);
[not specced]
This CL jumps to Step 5. of
https://html.spec.whatwg.org/#internal-module-script-graph-fetching-procedure
after this Step 4 here, and the script is ready when
https://html.spec.whatwg.org/#internal-module-script-graph-fetching-procedure
asynchronously returns.

Related: https://github.com/whatwg/html/issues/2544

[hiroshige, 2017-04-26 22:20:30]

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

[hiroshige, 2017-04-26 22:21:16]

Description was changed from

==========
Support Inline module script

Working Expectedly:
execorder.html
crossorigin.html

Crashing due to another issue:
imports.html

Crashing:
errorhandling.html

BUG=
==========

to

==========
Support Inline module script

Working Expectedly:
execorder.html
crossorigin.html

Crashing due to another issue:
imports.html

Crashing:
errorhandling.html

BUG=715369, 594639
==========

[commit-bot: I haz the power, 2017-04-26 22:21:25]

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-04-26 23:43:32]

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

[commit-bot: I haz the power, 2017-04-26 23:43:32]

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

[kouhei (in TOK), 2017-04-27 01:50:18]

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

https://codereview.chromium.org/2842923002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/Modulator.h:92: NOTREACHED();
move this to DummyModulator

[kouhei (in TOK), 2017-04-27 03:47:26]

https://codereview.chromium.org/2842923002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/ModulatorImpl.cpp (right):

https://codereview.chromium.org/2842923002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/ModulatorImpl.cpp:88: ModuleTreeClient*
client) {
We need to call resolver->RegisterModuleScript() somewhere before the call.
May be we can add the above call in ModuleScript::Create?
as now spec
https://html.spec.whatwg.org/multipage/webappapis.html#creating-a-module-script
Step 5 says to set it there.

https://codereview.chromium.org/2842923002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/ModuleMap.cpp (right):

https://codereview.chromium.org/2842923002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/ModuleMap.cpp:159:
entry->NotifyNewSingleModuleFinished(module_script);
This will allow other module script in the graph to refer to inline module
script, so we need to do this w/ care.

https://codereview.chromium.org/2842923002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/ModuleScript.h (right):

https://codereview.chromium.org/2842923002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/ModuleScript.h:50: parser_state,
credentials_mode, "");
note to self: "" is source_text

[hiroshige, 2017-04-27 21:26:32]

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

[commit-bot: I haz the power, 2017-04-27 21:26:59]

Dry run: CQ is trying da patch.

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

[hiroshige, 2017-04-27 21:57:20]

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

https://codereview.chromium.org/2842923002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/Modulator.h:92: NOTREACHED();
On 2017/04/27 01:50:18, kouhei wrote:
> move this to DummyModulator

Done.

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

https://codereview.chromium.org/2842923002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/ModulatorImpl.cpp:89:
map_->SetInlineModuleScript(module_script);
On 2017/04/26 04:48:13, hiroshige wrote:
> SetInlineModuleScript() is currently needed to put |module_script| (== the
> inline module script) to the module map because
> ScriptModuleResolverImpl::Resolve() tries to look up |module_script| itself
when
> we try to instantiate |module_script|'s record.
> 
> Probably we shouldn't look up |module_script| in the module map during
> instantiation.
> (Because it's not clear what is the key for lookup if we continue to look up;
> Currently I used the base URL, but of course this causes the problem because
> there can be multiple different inline scripts with the same base URL.)
> 
> What does the spec say about this?

Fixed.

https://codereview.chromium.org/2842923002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/ModulatorImpl.cpp (right):

https://codereview.chromium.org/2842923002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/ModulatorImpl.cpp:88: ModuleTreeClient*
client) {
On 2017/04/27 03:47:25, kouhei wrote:
> We need to call resolver->RegisterModuleScript() somewhere before the call.
> May be we can add the above call in ModuleScript::Create?
> as now spec
>
https://html.spec.whatwg.org/multipage/webappapis.html#creating-a-module-script
> Step 5 says to set it there.

Done in https://codereview.chromium.org/2845743003.

https://codereview.chromium.org/2842923002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/ModuleMap.cpp (right):

https://codereview.chromium.org/2842923002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/ModuleMap.cpp:159:
entry->NotifyNewSingleModuleFinished(module_script);
On 2017/04/27 03:47:25, kouhei wrote:
> This will allow other module script in the graph to refer to inline module
> script, so we need to do this w/ care.

This function is removed.

https://codereview.chromium.org/2842923002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/ModuleScript.h (right):

https://codereview.chromium.org/2842923002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/ModuleScript.h:50: parser_state,
credentials_mode, "");
On 2017/04/27 03:47:25, kouhei wrote:
> note to self: "" is source_text

Done.

[hiroshige, 2017-04-27 21:58:01]

hiroshige@chromium.org changed reviewers:
+ japhet@chromium.org

[hiroshige, 2017-04-27 21:58:01]

+module-dev

[hiroshige, 2017-04-27 22:05:16]

Description was changed from

==========
Support Inline module script

Working Expectedly:
execorder.html
crossorigin.html

Crashing due to another issue:
imports.html

Crashing:
errorhandling.html

BUG=715369, 594639
==========

to

==========
Support Inline module script

This CL
- Enables inline module scripts in ScriptLoader.
- Adds ModuleTreeLinker::FetchDescendantsForInlineScript() and
  related plumbing to ModuleTreeLinkerRegistry and Modulator.
  This is not yet spec'ed and we might need revisit once the spec is
  updated. https://github.com/whatwg/html/issues/2544
- Makes ModuleScript to store its |source_text| for CSP of inline scripts.

BUG=715369, 594639
==========

[hiroshige, 2017-04-27 22:05:28]

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

[commit-bot: I haz the power, 2017-04-27 22:30:40]

Dry run: CQ is trying da patch.

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

[kouhei (in TOK), 2017-04-28 00:20:28]

lgtm

[commit-bot: I haz the power, 2017-04-28 01:37:37]

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

[commit-bot: I haz the power, 2017-04-28 01:37:38]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[hiroshige, 2017-04-28 05:45:23]

The CQ bit was checked by hiroshige@chromium.org

[commit-bot: I haz the power, 2017-04-28 05:45:48]

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-04-28 05:49:52]

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

[commit-bot: I haz the power, 2017-04-28 05:49:54]

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[hiroshige, 2017-04-28 17:31:25]

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

[commit-bot: I haz the power, 2017-04-28 17:32:53]

Dry run: CQ is trying da patch.

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

[hiroshige, 2017-04-28 17:33:59]

The CQ bit was unchecked by hiroshige@chromium.org

[hiroshige, 2017-04-28 17:34:03]

The CQ bit was checked by hiroshige@chromium.org

[hiroshige, 2017-04-28 17:34:03]

The patchset sent to the CQ was uploaded after l-g-t-m from kouhei@chromium.org
Link to the patchset: https://codereview.chromium.org/2842923002/#ps160001
(title: "Rebase")

[commit-bot: I haz the power, 2017-04-28 17:34:23]

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-04-28 19:18:32]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1493400843641630,
"parent_rev": "cb477af46a98d9415f1491f613d43920003f4f86", "commit_rev":
"489fa402ff6ec3e74127929820b622407e979bb5"}

[commit-bot: I haz the power, 2017-04-28 19:19:49]

Description was changed from

==========
Support Inline module script

This CL
- Enables inline module scripts in ScriptLoader.
- Adds ModuleTreeLinker::FetchDescendantsForInlineScript() and
  related plumbing to ModuleTreeLinkerRegistry and Modulator.
  This is not yet spec'ed and we might need revisit once the spec is
  updated. https://github.com/whatwg/html/issues/2544
- Makes ModuleScript to store its |source_text| for CSP of inline scripts.

BUG=715369, 594639
==========

to

==========
Support Inline module script

This CL
- Enables inline module scripts in ScriptLoader.
- Adds ModuleTreeLinker::FetchDescendantsForInlineScript() and
  related plumbing to ModuleTreeLinkerRegistry and Modulator.
  This is not yet spec'ed and we might need revisit once the spec is
  updated. https://github.com/whatwg/html/issues/2544
- Makes ModuleScript to store its |source_text| for CSP of inline scripts.

BUG=715369, 594639

Review-Url: https://codereview.chromium.org/2842923002
Cr-Commit-Position: refs/heads/master@{#468087}
Committed:
https://chromium.googlesource.com/chromium/src/+/489fa402ff6ec3e74127929820b6...
==========

[commit-bot: I haz the power, 2017-04-28 19:19:50]

Committed patchset #9 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/489fa402ff6ec3e74127929820b6...