New issue
Advanced search Search tips

Issue 796034 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

eval("import()") on classic inline script should resolve relative to document's current baseURL

Project Member Reported by kouhei@chromium.org, Dec 19 2017

Issue description

Project Member

Comment 1 by bugdroid1@chromium.org, Dec 20 2017

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

commit 648e383e79a76b09c880282cb771fcd568762807
Author: Kouhei Ueno <kouhei@chromium.org>
Date: Wed Dec 20 06:53:52 2017

Align to the spec: concept-script-base-url

Before this CL, "base URL" was tracked only for ModuleScript (which map
to spec concept "module script"). In the latest spec, it says that the
"base URL" is now "script"-level concept (concept-script-base-url) which
both ClassicScript and ModuleScript should track. This CL aligns Blink
implementation to the spec.

No behavioral change is expected from this CL.

Bug: 796034
Change-Id: I86b8efe1f3980e3b6ab4743a35eeefaef639f0fd
Reviewed-on: https://chromium-review.googlesource.com/833434
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525281}
[modify] https://crrev.com/648e383e79a76b09c880282cb771fcd568762807/third_party/WebKit/Source/core/dom/ClassicScript.h
[modify] https://crrev.com/648e383e79a76b09c880282cb771fcd568762807/third_party/WebKit/Source/core/dom/ModuleScript.cpp
[modify] https://crrev.com/648e383e79a76b09c880282cb771fcd568762807/third_party/WebKit/Source/core/dom/ModuleScript.h
[modify] https://crrev.com/648e383e79a76b09c880282cb771fcd568762807/third_party/WebKit/Source/core/dom/Script.h

Project Member

Comment 2 by bugdroid1@chromium.org, Dec 21 2017

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

commit 22ba80d4605c0b3f5af3bace105741e464f18d76
Author: Kouhei Ueno <kouhei@chromium.org>
Date: Thu Dec 21 05:37:36 2017

[ES6 modules] Store base URL in ReferrerScriptInfo

This CL is a part of work to align base URL handling of "script".

This CL makes ReferrerScriptInfo contain referencingScriptOrModule's "base URL"
as ReferrerScriptInfo::base_url_.
In future CLs:
- The added field will be referenced in [ResolveModuleSpecifier] Step 3, which is called from [HIMD] step 2.1.
- The "base URL" of script is plumbed to set correct value (which we currently use script's source URL in this CL).

[ResolveModuleSpecifier] https://html.spec.whatwg.org/multipage/webappapis.html#resolve-a-module-specifier
[HIMD] https://html.spec.whatwg.org/multipage/webappapis.html#hostimportmoduledynamically(referencingscriptormodule,-specifier,-promisecapability)

Bug: 796034
Change-Id: Ifbd280e84f3eea4e6a4d308793be0f8f2bcd692f
Reviewed-on: https://chromium-review.googlesource.com/838761
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525622}
[modify] https://crrev.com/22ba80d4605c0b3f5af3bace105741e464f18d76/third_party/WebKit/Source/bindings/core/v8/ReferrerScriptInfo.cpp
[modify] https://crrev.com/22ba80d4605c0b3f5af3bace105741e464f18d76/third_party/WebKit/Source/bindings/core/v8/ReferrerScriptInfo.h
[modify] https://crrev.com/22ba80d4605c0b3f5af3bace105741e464f18d76/third_party/WebKit/Source/bindings/core/v8/ReferrerScriptInfoTest.cpp
[modify] https://crrev.com/22ba80d4605c0b3f5af3bace105741e464f18d76/third_party/WebKit/Source/bindings/core/v8/ScriptController.cpp
[modify] https://crrev.com/22ba80d4605c0b3f5af3bace105741e464f18d76/third_party/WebKit/Source/bindings/core/v8/ScriptModule.cpp

Project Member

Comment 3 by bugdroid1@chromium.org, Dec 21 2017

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

commit ed8f21db26cc0be62acf8e258aa949cb59fc2686
Author: Kouhei Ueno <kouhei@chromium.org>
Date: Thu Dec 21 08:32:59 2017

[ES6 modules] Add comment about ReferrerScriptInfo::base_url_ being null

This CL is a follow-up to "[ES6 modules] Store base URL in ReferrerScriptInfo":
https://chromium-review.googlesource.com/c/chromium/src/+/838761

As suggested by yhirano@, this CL documents how base_url_.IsNull()
should be handled & why.

Bug: 796034
Change-Id: I0aed20517c9dc6355f89b23e7ccdeb91f695d0fb
Reviewed-on: https://chromium-review.googlesource.com/838980
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525646}
[modify] https://crrev.com/ed8f21db26cc0be62acf8e258aa949cb59fc2686/third_party/WebKit/Source/bindings/core/v8/ReferrerScriptInfo.h

Project Member

Comment 4 by bugdroid1@chromium.org, Dec 21 2017

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

commit c0b78bc91021d1b83438807ad6c6f1edf748bce8
Author: Kouhei Ueno <kouhei@chromium.org>
Date: Thu Dec 21 08:43:39 2017

Plumb base URL of classic scripts to be stored in ReferrerScriptInfo

This CL is a follow-up of: "[ES6 modules] Store base URL in ReferrerScriptInfo".
https://chromium-review.googlesource.com/c/chromium/src/+/838761

Before this CL, the base URL of RSI for classic scripts were always set to its
source URL.  After this CL, the base URL of RSI for inline classic scripts are
set to "the document base URL of the containing document" [1].

No behavioral change is expected (yet), until dynamic import() implementation
is modified to respect ReferrerScriptInfo::BaseURL() in future CL.

[1] https://html.spec.whatwg.org/multipage/webappapis.html#concept-script-base-url

Bug: 796034
Change-Id: I28fc484cbf06fc8155cd668ec044c456b8bb0b9d
Reviewed-on: https://chromium-review.googlesource.com/838724
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525648}
[modify] https://crrev.com/c0b78bc91021d1b83438807ad6c6f1edf748bce8/third_party/WebKit/Source/bindings/core/v8/ScriptController.cpp
[modify] https://crrev.com/c0b78bc91021d1b83438807ad6c6f1edf748bce8/third_party/WebKit/Source/bindings/core/v8/ScriptController.h
[modify] https://crrev.com/c0b78bc91021d1b83438807ad6c6f1edf748bce8/third_party/WebKit/Source/bindings/core/v8/ScriptSourceCode.h
[modify] https://crrev.com/c0b78bc91021d1b83438807ad6c6f1edf748bce8/third_party/WebKit/Source/core/inspector/InspectorOverlayAgent.cpp
[modify] https://crrev.com/c0b78bc91021d1b83438807ad6c6f1edf748bce8/third_party/WebKit/Source/core/resize_observer/ResizeObserverTest.cpp
[modify] https://crrev.com/c0b78bc91021d1b83438807ad6c6f1edf748bce8/third_party/WebKit/Source/core/script/ClassicPendingScript.cpp
[modify] https://crrev.com/c0b78bc91021d1b83438807ad6c6f1edf748bce8/third_party/WebKit/Source/core/script/ClassicPendingScript.h
[modify] https://crrev.com/c0b78bc91021d1b83438807ad6c6f1edf748bce8/third_party/WebKit/Source/core/script/ClassicScript.cpp
[modify] https://crrev.com/c0b78bc91021d1b83438807ad6c6f1edf748bce8/third_party/WebKit/Source/core/script/ClassicScript.h

Project Member

Comment 5 by bugdroid1@chromium.org, Dec 22 2017

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

commit 0dcffd50781be10d1b1c2fc4c2f8bcd850a611b8
Author: Kouhei Ueno <kouhei@chromium.org>
Date: Fri Dec 22 05:34:01 2017

[ES6 modules] Temporarily mark base-url tests as failure instead of expected.txt

This CL will mark the tests as failure instead of using -expected.txt containing FAIL lines.
This allows us to fix the tests without flaking -expected.txt.

The tests will be marked as pass when 796034 is fixed.

Bug: 796034
Change-Id: I6d0f9343128ae8507983875821b008d16a42d0fa
Reviewed-on: https://chromium-review.googlesource.com/842142
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525940}
[modify] https://crrev.com/0dcffd50781be10d1b1c2fc4c2f8bcd850a611b8/third_party/WebKit/LayoutTests/TestExpectations
[delete] https://crrev.com/cf57dc0a5c9493256b5ba5e5f827ca393170c0be/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/string-compilation-base-url-external-classic-expected.txt
[delete] https://crrev.com/cf57dc0a5c9493256b5ba5e5f827ca393170c0be/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/string-compilation-base-url-external-module-expected.txt
[delete] https://crrev.com/cf57dc0a5c9493256b5ba5e5f827ca393170c0be/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/string-compilation-base-url-inline-classic-expected.txt
[delete] https://crrev.com/cf57dc0a5c9493256b5ba5e5f827ca393170c0be/third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/string-compilation-base-url-inline-module-expected.txt

Project Member

Comment 6 by bugdroid1@chromium.org, Dec 22 2017

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

commit 282b0eeda63f32bb2ca2dbaa5ed42a4ca87378da
Author: Kouhei Ueno <kouhei@chromium.org>
Date: Fri Dec 22 06:51:23 2017

[ES6 modules] Modulator API should distinguish {source,base} URL

Before this CL, ES6 modules internal API signatures have assumed that
source URL (URL of script source code resource) is always equal to
script's base URL (#concept-script-base-url [1]).
However, this is incorrect, since base URL of inline script is base URL of the
containing document, which can be modified through <base> element.

This CL changes various function signatures to distinguish {source,base} URL,
to prepare for future CLs which actually corrects the behavior (see crbug).

No behavior change from this CL is expected.

[1] https://html.spec.whatwg.org/multipage/webappapis.html#concept-script-base-url

Bug: 796034
Change-Id: I8f79a97a350a7c46bf6f0c4e67b0063fc24b340d
Reviewed-on: https://chromium-review.googlesource.com/838723
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525949}
[modify] https://crrev.com/282b0eeda63f32bb2ca2dbaa5ed42a4ca87378da/third_party/WebKit/LayoutTests/http/tests/devtools/tracing/timeline-script-module-expected.txt
[modify] https://crrev.com/282b0eeda63f32bb2ca2dbaa5ed42a4ca87378da/third_party/WebKit/Source/bindings/core/v8/ScriptModule.cpp
[modify] https://crrev.com/282b0eeda63f32bb2ca2dbaa5ed42a4ca87378da/third_party/WebKit/Source/bindings/core/v8/ScriptModule.h
[modify] https://crrev.com/282b0eeda63f32bb2ca2dbaa5ed42a4ca87378da/third_party/WebKit/Source/bindings/core/v8/ScriptModuleTest.cpp
[modify] https://crrev.com/282b0eeda63f32bb2ca2dbaa5ed42a4ca87378da/third_party/WebKit/Source/core/loader/modulescript/ModuleScriptLoader.cpp
[modify] https://crrev.com/282b0eeda63f32bb2ca2dbaa5ed42a4ca87378da/third_party/WebKit/Source/core/loader/modulescript/ModuleScriptLoaderTest.cpp
[modify] https://crrev.com/282b0eeda63f32bb2ca2dbaa5ed42a4ca87378da/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinkerTest.cpp
[modify] https://crrev.com/282b0eeda63f32bb2ca2dbaa5ed42a4ca87378da/third_party/WebKit/Source/core/script/DynamicModuleResolverTest.cpp
[modify] https://crrev.com/282b0eeda63f32bb2ca2dbaa5ed42a4ca87378da/third_party/WebKit/Source/core/script/Modulator.h
[modify] https://crrev.com/282b0eeda63f32bb2ca2dbaa5ed42a4ca87378da/third_party/WebKit/Source/core/script/ModulatorImplBase.cpp
[modify] https://crrev.com/282b0eeda63f32bb2ca2dbaa5ed42a4ca87378da/third_party/WebKit/Source/core/script/ModulatorImplBase.h
[modify] https://crrev.com/282b0eeda63f32bb2ca2dbaa5ed42a4ca87378da/third_party/WebKit/Source/core/script/ModuleScript.cpp
[modify] https://crrev.com/282b0eeda63f32bb2ca2dbaa5ed42a4ca87378da/third_party/WebKit/Source/core/script/ModuleScript.h
[modify] https://crrev.com/282b0eeda63f32bb2ca2dbaa5ed42a4ca87378da/third_party/WebKit/Source/core/script/ScriptLoader.cpp
[modify] https://crrev.com/282b0eeda63f32bb2ca2dbaa5ed42a4ca87378da/third_party/WebKit/Source/core/script/ScriptModuleResolverImplTest.cpp
[modify] https://crrev.com/282b0eeda63f32bb2ca2dbaa5ed42a4ca87378da/third_party/WebKit/Source/core/testing/DummyModulator.cpp
[modify] https://crrev.com/282b0eeda63f32bb2ca2dbaa5ed42a4ca87378da/third_party/WebKit/Source/core/testing/DummyModulator.h
[modify] https://crrev.com/282b0eeda63f32bb2ca2dbaa5ed42a4ca87378da/third_party/WebKit/Source/modules/animationworklet/AnimationWorkletGlobalScopeTest.cpp
[modify] https://crrev.com/282b0eeda63f32bb2ca2dbaa5ed42a4ca87378da/third_party/WebKit/Source/modules/animationworklet/AnimationWorkletThreadTest.cpp
[modify] https://crrev.com/282b0eeda63f32bb2ca2dbaa5ed42a4ca87378da/third_party/WebKit/Source/modules/webaudio/AudioWorkletGlobalScopeTest.cpp
[modify] https://crrev.com/282b0eeda63f32bb2ca2dbaa5ed42a4ca87378da/third_party/WebKit/Source/modules/webaudio/AudioWorkletThreadTest.cpp

Project Member

Comment 7 by bugdroid1@chromium.org, Dec 22 2017

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

commit 2c53cb695bda73713c9addbbb678261d870dadf9
Author: Kouhei Ueno <kouhei@chromium.org>
Date: Fri Dec 22 11:10:29 2017

[ES6 modules] The base url for external classic script should be resource URL

This CL fixes a bug introduced in:
https://chromium.googlesource.com/chromium/src.git/+/c0b78bc91021d1b83438807ad6c6f1edf748bce8

In the previous CL, the base URL of both {inline,external} classic scripts were
set to "the document base URL of the containing document".
However, the base URL of external classic scripts should be
"the URL from which the script was obtained".  This CL fixes the issue.

Test: external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/string-compilation-base-url-external-classic.html
Bug: 796034
Change-Id: I5674bf4139164606030b48cf66b48962c11d35f3
Reviewed-on: https://chromium-review.googlesource.com/842282
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525973}
[modify] https://crrev.com/2c53cb695bda73713c9addbbb678261d870dadf9/third_party/WebKit/Source/core/script/ClassicPendingScript.cpp
[modify] https://crrev.com/2c53cb695bda73713c9addbbb678261d870dadf9/third_party/WebKit/Source/core/script/ClassicPendingScript.h

Project Member

Comment 8 by bugdroid1@chromium.org, Jan 5 2018

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

commit 9bfb1928143a56d1263aa62047dbf7ca52cf7d92
Author: Kouhei Ueno <kouhei@chromium.org>
Date: Fri Jan 05 12:36:35 2018

[ES6 modules] DynamicModuleResolver should use referrer script's baseURL, not referrer script's URL

Before this CL, the dynamic import() implementation used script's source code URL as the base URL.
After this CL, it will use "referrer script's base URL" as specified in [ResolveModuleSpecifier] Step 3, which is called from [HIMD] step 2.1.

[ResolveModuleSpecifier] https://html.spec.whatwg.org/multipage/webappapis.html#resolve-a-module-specifier
[HIMD] https://html.spec.whatwg.org/multipage/webappapis.html#hostimportmoduledynamically(referencingscriptormodule,-specifier,-promisecapability)

Test: DynamicModuleResolverTest.ResolveWithReferrerScriptInfoBaseURL
Test: external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/string-compilation-base-url-*
Bug: 796034
Change-Id: I0a744998ade7e82ad3ca904e60a194a768cd18dc
Reviewed-on: https://chromium-review.googlesource.com/842362
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527261}
[modify] https://crrev.com/9bfb1928143a56d1263aa62047dbf7ca52cf7d92/third_party/WebKit/Source/bindings/core/v8/ReferrerScriptInfo.cpp
[modify] https://crrev.com/9bfb1928143a56d1263aa62047dbf7ca52cf7d92/third_party/WebKit/Source/bindings/core/v8/V8Initializer.cpp
[modify] https://crrev.com/9bfb1928143a56d1263aa62047dbf7ca52cf7d92/third_party/WebKit/Source/core/script/DynamicModuleResolver.cpp
[modify] https://crrev.com/9bfb1928143a56d1263aa62047dbf7ca52cf7d92/third_party/WebKit/Source/core/script/DynamicModuleResolver.h
[modify] https://crrev.com/9bfb1928143a56d1263aa62047dbf7ca52cf7d92/third_party/WebKit/Source/core/script/DynamicModuleResolverTest.cpp

Sign in to add a comment