New issue
Advanced search Search tips

Issue 715369 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocking:
issue 594639



Sign in to add a comment

Support inline module scripts

Project Member Reported by hirosh...@chromium.org, Apr 26 2017

Issue description

Spec: step 22, the "module" case of "prepare a script":
https://html.spec.whatwg.org/#prepare-a-script
 
Assigning to me as I'll a brief one-day investigation for estimating what are needed.
Status: Started (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/498b00e062fb73064e19187b6b174f0d6ff62968

commit 498b00e062fb73064e19187b6b174f0d6ff62968
Author: hiroshige <hiroshige@chromium.org>
Date: Fri Apr 28 03:34:33 2017

Move "create a module script" from ModuleScriptLoader to ModuleScript

Because it will be used from outside ModuleScriptLoader when we support
inline scripts.

This CL
- Renames existing ModuleScript::Create() (which is a simple wrapper of
  the constructor) to CreateForTest(),
- Moves ModuleScriptLoader::CreateModuleScript() to new
  ModuleScript::Create(), and
- Reorders the parameters to match with the spec.

BUG= 594639 ,  715369 

Review-Url: https://codereview.chromium.org/2845743003
Cr-Commit-Position: refs/heads/master@{#467873}

[modify] https://crrev.com/498b00e062fb73064e19187b6b174f0d6ff62968/third_party/WebKit/Source/core/dom/ModuleMapTest.cpp
[modify] https://crrev.com/498b00e062fb73064e19187b6b174f0d6ff62968/third_party/WebKit/Source/core/dom/ModuleScript.cpp
[modify] https://crrev.com/498b00e062fb73064e19187b6b174f0d6ff62968/third_party/WebKit/Source/core/dom/ModuleScript.h
[modify] https://crrev.com/498b00e062fb73064e19187b6b174f0d6ff62968/third_party/WebKit/Source/core/dom/ScriptModuleResolverImplTest.cpp
[modify] https://crrev.com/498b00e062fb73064e19187b6b174f0d6ff62968/third_party/WebKit/Source/core/loader/modulescript/ModuleScriptLoader.cpp
[modify] https://crrev.com/498b00e062fb73064e19187b6b174f0d6ff62968/third_party/WebKit/Source/core/loader/modulescript/ModuleScriptLoader.h
[modify] https://crrev.com/498b00e062fb73064e19187b6b174f0d6ff62968/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinkerTest.cpp

Project Member

Comment 4 by bugdroid1@chromium.org, Apr 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d97cc73459881aeedfb39ed40ff5e0f9aefea9aa

commit d97cc73459881aeedfb39ed40ff5e0f9aefea9aa
Author: hiroshige <hiroshige@chromium.org>
Date: Fri Apr 28 05:16:51 2017

Introduce ModuleScript::CreateInternal()

This is preparation for https://codereview.chromium.org/2850673002
to create a place to insert RegisterModuleScript() both for
Create() and CreateForTest().

This doesn't change the behavior.

BUG= 594639 ,  715369 

Review-Url: https://codereview.chromium.org/2844413003
Cr-Commit-Position: refs/heads/master@{#467891}

[modify] https://crrev.com/d97cc73459881aeedfb39ed40ff5e0f9aefea9aa/third_party/WebKit/Source/core/dom/ModuleScript.cpp
[modify] https://crrev.com/d97cc73459881aeedfb39ed40ff5e0f9aefea9aa/third_party/WebKit/Source/core/dom/ModuleScript.h

Project Member

Comment 5 by bugdroid1@chromium.org, Apr 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fca892fe54b316243806c3b1cfc6cad5e23ae098

commit fca892fe54b316243806c3b1cfc6cad5e23ae098
Author: hiroshige <hiroshige@chromium.org>
Date: Fri Apr 28 05:42:01 2017

Move a RegisterModuleScript() call to ModuleScript::CreateInternal()

To align with the spec, and also this is needed for inline module
scripts because NotifyNewSingleModuleFinished() is not called for
inline scripts.

This CL also makes DummyModulator to ignore RegisterModuleScript()
calls where it is not tested explcitly, because this CL makes
an additional RegisterModuleScript() call for CreateForTest() which is
not coupled with NotifyNewSingleModuleFinished().

This CL doesn't change the non-test behavior, because in the non-test
code ModuleScript::Create() is always coupled with
NotifyNewSingleModuleFinished() and thus this CL just makes
RegisterModuleScript() a little earlier.

BUG= 594639 ,  715369 

Review-Url: https://codereview.chromium.org/2850673002
Cr-Commit-Position: refs/heads/master@{#467907}

[modify] https://crrev.com/fca892fe54b316243806c3b1cfc6cad5e23ae098/third_party/WebKit/Source/core/dom/ModuleMap.cpp
[modify] https://crrev.com/fca892fe54b316243806c3b1cfc6cad5e23ae098/third_party/WebKit/Source/core/dom/ModuleScript.cpp
[modify] https://crrev.com/fca892fe54b316243806c3b1cfc6cad5e23ae098/third_party/WebKit/Source/core/testing/DummyModulator.cpp
[modify] https://crrev.com/fca892fe54b316243806c3b1cfc6cad5e23ae098/third_party/WebKit/Source/core/testing/DummyModulator.h

Project Member

Comment 6 by bugdroid1@chromium.org, Apr 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/489fa402ff6ec3e74127929820b622407e979bb5

commit 489fa402ff6ec3e74127929820b622407e979bb5
Author: hiroshige <hiroshige@chromium.org>
Date: Fri Apr 28 19:18:29 2017

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}

[modify] https://crrev.com/489fa402ff6ec3e74127929820b622407e979bb5/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/489fa402ff6ec3e74127929820b622407e979bb5/third_party/WebKit/Source/core/dom/Modulator.h
[modify] https://crrev.com/489fa402ff6ec3e74127929820b622407e979bb5/third_party/WebKit/Source/core/dom/ModulatorImpl.cpp
[modify] https://crrev.com/489fa402ff6ec3e74127929820b622407e979bb5/third_party/WebKit/Source/core/dom/ModulatorImpl.h
[modify] https://crrev.com/489fa402ff6ec3e74127929820b622407e979bb5/third_party/WebKit/Source/core/dom/ModuleScript.cpp
[modify] https://crrev.com/489fa402ff6ec3e74127929820b622407e979bb5/third_party/WebKit/Source/core/dom/ModuleScript.h
[modify] https://crrev.com/489fa402ff6ec3e74127929820b622407e979bb5/third_party/WebKit/Source/core/dom/ScriptLoader.cpp
[modify] https://crrev.com/489fa402ff6ec3e74127929820b622407e979bb5/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.cpp
[modify] https://crrev.com/489fa402ff6ec3e74127929820b622407e979bb5/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.h
[modify] https://crrev.com/489fa402ff6ec3e74127929820b622407e979bb5/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinkerRegistry.cpp
[modify] https://crrev.com/489fa402ff6ec3e74127929820b622407e979bb5/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinkerRegistry.h
[modify] https://crrev.com/489fa402ff6ec3e74127929820b622407e979bb5/third_party/WebKit/Source/core/testing/DummyModulator.cpp
[modify] https://crrev.com/489fa402ff6ec3e74127929820b622407e979bb5/third_party/WebKit/Source/core/testing/DummyModulator.h

Status: Fixed (was: Started)
Closing as done.
Project Member

Comment 8 by bugdroid1@chromium.org, Jul 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8a589e1f43f36535e10667213f4ad19e33ad17e3

commit 8a589e1f43f36535e10667213f4ad19e33ad17e3
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Fri Jul 07 02:27:18 2017

Add comprehensive wpt tests for script load/error events

These tests cover
1. all clauses of Step 23 of "prepare a script" in the spec
   https://html.spec.whatwg.org/#prepare-a-script, and
2. all branches around ScriptLoader/HTMLParserScriptRunner
   (but not XMLDocument) that correspond to 1.

Bug: 686281,  739514 ,  715369 
Change-Id: Ib5a7077864ee213f93272c76b3cf38e2d280cc99
Reviewed-on: https://chromium-review.googlesource.com/560636
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#484800}
[modify] https://crrev.com/8a589e1f43f36535e10667213f4ad19e33ad17e3/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/8a589e1f43f36535e10667213f4ad19e33ad17e3/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/load-error-events-1.html
[add] https://crrev.com/8a589e1f43f36535e10667213f4ad19e33ad17e3/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/load-error-events-2.html
[add] https://crrev.com/8a589e1f43f36535e10667213f4ad19e33ad17e3/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/load-error-events-3.html
[delete] https://crrev.com/470ac8ad6279aeecc13ea5349dbc842ab0eb39f6/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/load-event.html
[add] https://crrev.com/8a589e1f43f36535e10667213f4ad19e33ad17e3/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/load-error-events-inline.html
[add] https://crrev.com/8a589e1f43f36535e10667213f4ad19e33ad17e3/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/load-error-events.html
[add] https://crrev.com/8a589e1f43f36535e10667213f4ad19e33ad17e3/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/resources/load-error-events-helpers.js
[add] https://crrev.com/8a589e1f43f36535e10667213f4ad19e33ad17e3/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/resources/load-error-events.py
[add] https://crrev.com/8a589e1f43f36535e10667213f4ad19e33ad17e3/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/resources/slow.py

Project Member

Comment 9 by bugdroid1@chromium.org, Jul 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5cd334f5bb536f421a3321921b5543b02ce34534

commit 5cd334f5bb536f421a3321921b5543b02ce34534
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Fri Jul 07 04:39:21 2017

Set ModulePendingScript::IsExternal() to false for inline module scripts

Previously, ModulePendingScript::IsExternal() always returns true,
According to the spec, "from an external file"
https://html.spec.whatwg.org/#concept-script-external
is false for inline module scripts.
This CL adjusts some DCHECK()s to also allow inline module scripts.

This CL doesn't change any behavior, because currently IsExternal()
is used only in DCHECK()s.

Bug: 686281,  715369 
Change-Id: I27b1d79269519ae6f8fdacd44929da82b9446e0c
Reviewed-on: https://chromium-review.googlesource.com/558506
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#484834}
[modify] https://crrev.com/5cd334f5bb536f421a3321921b5543b02ce34534/third_party/WebKit/Source/core/dom/ModulePendingScript.cpp
[modify] https://crrev.com/5cd334f5bb536f421a3321921b5543b02ce34534/third_party/WebKit/Source/core/dom/ModulePendingScript.h
[modify] https://crrev.com/5cd334f5bb536f421a3321921b5543b02ce34534/third_party/WebKit/Source/core/dom/PendingScript.cpp
[modify] https://crrev.com/5cd334f5bb536f421a3321921b5543b02ce34534/third_party/WebKit/Source/core/dom/PendingScript.h
[modify] https://crrev.com/5cd334f5bb536f421a3321921b5543b02ce34534/third_party/WebKit/Source/core/dom/ScriptLoader.cpp
[modify] https://crrev.com/5cd334f5bb536f421a3321921b5543b02ce34534/third_party/WebKit/Source/core/html/parser/HTMLParserScriptRunner.cpp

Project Member

Comment 10 by bugdroid1@chromium.org, Jul 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ed0d20f7c347cc2ec55621918941556e141d8341

commit ed0d20f7c347cc2ec55621918941556e141d8341
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Fri Jul 07 21:46:24 2017

Do not fire load events for async/dynamic inline module scripts

This CL stops firing load events for inline module scripts controlled
by ScriptRunner, by checking IsExternal() in Execute().

This CL doesn't affect classic scripts, because all classic scripts
controlled by ScriptRunner are external scripts.

Bug:  715369 
Test: external/wpt/html/semantics/scripting-1/the-script-element/module/load-error-events-inline.html
Change-Id: I1157b7e2d1f4f6e64077fde901cc6a6b95c102ae
Reviewed-on: https://chromium-review.googlesource.com/557978
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485059}
[modify] https://crrev.com/ed0d20f7c347cc2ec55621918941556e141d8341/third_party/WebKit/Source/core/dom/ScriptLoader.cpp

Project Member

Comment 11 by bugdroid1@chromium.org, Jul 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/72f9a4aac2ccfdf18e0618c7fcdc25f1a88cec28

commit 72f9a4aac2ccfdf18e0618c7fcdc25f1a88cec28
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Fri Jul 07 23:29:38 2017

Do not fire load event for inline scripts in HTMLParserScriptRunner

According to the spec: Step 8 of "execute a script block":
https://html.spec.whatwg.org/#execute-the-script-block

This CL stops firing load events for
- parser-inserted inline module scripts without async in HTML,
  i.e. those fall into 1st Clause of Step 23 of "prepare a script"
  ( Issue 715369 ; This CL and [1] stop firing load events of all
  inline module scripts)
- parser-inserted inline classic scripts that fall into
  5th Clause of Step 23 of "prepare a script" ( Issue 739514 )

[1] https://chromium-review.googlesource.com/c/557978/

Bug: 686281,  739514 ,  715369 
Change-Id: Ia1a2e9653bccb99ff47faa1d8b8186c043e83770
Reviewed-on: https://chromium-review.googlesource.com/557972
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485100}
[modify] https://crrev.com/72f9a4aac2ccfdf18e0618c7fcdc25f1a88cec28/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/72f9a4aac2ccfdf18e0618c7fcdc25f1a88cec28/third_party/WebKit/Source/core/html/parser/HTMLParserScriptRunner.cpp

Sign in to add a comment