Link rel=modulepreload
Reported by
guybedf...@gmail.com,
Jul 11 2017
|
|||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/59.0.3071.115 Safari/537.36 Steps to reproduce the problem: 1. Clone https://github.com/guybedford/modules-link-preload 2. Run a local server 3. Open test.html What is the expected behavior? module999.js seems like it is preloaded ok, but the preloaded source doesn't seem to be used by the loading pipeline. In addition there is a warning that an unnecessary preload request was made. What went wrong? The preloaded <link rel="preload" as="script" src="lib/module999.js" /> should be used by the module pipeline. Did this work before? N/A Chrome version: 61.0.3154.0 Channel: canary OS Version: OS X 10.12.5 Flash Version:
,
Jul 20 2017
,
Jul 24 2017
Is there still a spec issue that needs to be resolved?
,
Jul 24 2017
,
Jul 24 2017
> Is there still a spec issue that needs to be resolved? It depends. At least theoretically, I think we can do some work here without further spec work. That is, we should be able to hook up the preloaded source so that it is used by the module pipeline, even if as dumb text. Maybe that is a good target for this bug? The extra spec work comes about in figuring out if we can also parse and compile the source text, and even store it in the the module map. Currently as="script" does not give enough information to do that. That is why we had a spec thread at https://github.com/whatwg/fetch/issues/486, with a summary at https://github.com/whatwg/fetch/issues/486#issuecomment-282044172 and a proposal at https://github.com/whatwg/html/pull/2383 . That proposal currently does not have any implementer interest, so if Chrome is interested in it (or has opinions on how it should be changed---e.g. maybe crawling the graph is not worthwhile), please comment there.
,
Jul 24 2017
> The extra spec work comes about in figuring out if we can also parse and compile the source text, and even store it in the the module map. You could actually instantiate as well, in theory.
,
Jul 24 2017
Short term solution/question:
Why are we not reusing ScriptResource preloaded? Check why it doesn't hit -> make it hit.
After this work, the module script doesn't benefit from ahead of time compile/instantiate, but should still benefit from having the raw bytes on blink MemoryCache.
Long term solution:
Step 1: Distinguish blink::{Module,Classic}ScriptResource
Step 2: if as="modulescript", LinkPreloader preloads ModuleScriptResource which CompileModule() on Resource::NotifyFinished()
Step 3: Also call InstantiateModule() somehow?
,
Jul 25 2017
I'm aware this isn't necessarily the best place to discuss this, so feel free to ignore. But I worry a lot about module loading performance - for example in https://bugs.chromium.org/p/chromium/issues/detail?id=740874. Now imagine I wanted to make the slow tree in that issue load without the latency delay in browsers. The fix I'd like to add for this is to tell the browser all the deep dependencies ahead of time, using a link preload. The goal being that it should be possible to parse and execute the modules then in a single latency (the modules here have very low bandwidth). If my only option is to instantiate preload, then I'm having to deeply instantiate every module in the tree, which would lead to exponential traversal of the tree, in addition to breaking circular reference instantiations as they now become top-level instantiate calls. This is why I'd simply like to see a simple fetch-based preloader without instantiation. Yes, instantiation may still be a useful preloading technique, but as a separate use case to trying to speed up deep prefetching.
,
Jul 25 2017
Correction to the above - circular references are unaffected by top-level instantiating every module in the tree. But the argument over performance stands. A tree of N nodes being traversed M ~ O(N) times just sounds like a perf issue waiting to happen.
,
Jul 25 2017
>c#8 The bug was already fixed (and made it to M61 branch). Sorry I forgot to update the bug.
,
Jul 25 2017
Amazing! Thanks so much for the update, that is great to hear.
,
Jul 25 2017
re: partial instantiate, as long as the subgraph is complete, we can mark all the subgraph records as "instantiated" and not need to visit the nodes in the subgraph again, which would avoid exponential time. There is still an open question about how we would know that the subgraph is complete w/o actually trying though. (If we always try, we will still have a worst case scn of exponential time)
,
Jul 25 2017
Actually it sounds like the performance profile from https://bugs.chromium.org/p/chromium/issues/detail?id=740874 would be exactly what to expect with this preload mechanism though - ~20s load time for 170 modules preloaded.
,
Jul 25 2017
If preloading is done upfront, subgraphs with circular references can easily overlap causing deadlock. The result being one still ends up traversing every subgraph for this scenario. This is why I say an instantiation preload is a separate use case - it's not for dealing with the graph latency problem, its for dealing with preloading a dynamic load event to get new functionality of another section of the app, as opposed to boot-time loading.
,
Jul 25 2017
>c#13 I don't think it is not necessarily the case. We have many ways that we can address c#12 w/o exponential worst case. One top of my head is "memorizing" the incomplete edges for each module graph node. That said, as I made a note in c#7, we will start from caching raw bytes in Memory Cache, then caching parsed module script, and finally explore eager instantiating.
,
Jul 25 2017
Thanks, as long as performance is upheld as the primary requirement, that is my only main concern. Let the numbers decide then certainly. And I will continue to test performance here too.
,
Aug 21 2017
ksakamoto: Would you own this?
,
Aug 22 2017
Did some initial investigation. Re #c7: > Why are we not reusing ScriptResource preloaded? Check why it doesn't hit -> make it hit. To make it hit, requests' credentials mode must match. In the current implementation, it hits memory cached ScriptResource if both <link rel=preload> and <script type=module> have the same crossorigin= attribute (anonymous or use-credentials). If <script type=module> has no crossorigin= attribute, the module is fetched with "omit" credentials mode, but there's no way to use this mode in <link rel=preload>.
,
Aug 23 2017
,
Sep 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/73482ef199fa82d96601f4221dcfe1debd0d7ede commit 73482ef199fa82d96601f4221dcfe1debd0d7ede Author: Kunihiko Sakamoto <ksakamoto@chromium.org> Date: Wed Sep 13 06:41:50 2017 Add a layout test for module script preloading As a follow-up to https://chromium-review.googlesource.com/658016, this adds a layout test that makes sure module scripts are preloaded. Bug: 740886 Change-Id: Iffa21862444d940750d03bf7cb744812cd87f4a1 Reviewed-on: https://chromium-review.googlesource.com/664498 Commit-Queue: Kouhei Ueno <kouhei@chromium.org> Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Cr-Commit-Position: refs/heads/master@{#501557} [add] https://crrev.com/73482ef199fa82d96601f4221dcfe1debd0d7ede/third_party/WebKit/LayoutTests/http/tests/preload/module-script.html [add] https://crrev.com/73482ef199fa82d96601f4221dcfe1debd0d7ede/third_party/WebKit/LayoutTests/http/tests/preload/resources/empty.js
,
Sep 27 2017
I tried the Canary release 63.0.3223.8 which includes the fix, but I am still seeing messages like: The resource xxxx was preloaded using link preload but not used within a few seconds from the window's load event. Please make sure it wasn't preloaded for nothing. I've made sure to match the crossorigins and everything. I even tried the repo project in this original issue and get the same error.
,
Sep 27 2017
Sorry for confusion, the above patch is for Preload Scanner, another kind of preload. Link preload for modules still doesn't work, we're working on it.
,
Sep 30 2017
,
Sep 30 2017
,
Oct 11 2017
,
Nov 8 2017
,
Nov 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/80b8282d97f105ac5f59a50640d732e62374e4e2 commit 80b8282d97f105ac5f59a50640d732e62374e4e2 Author: Kunihiko Sakamoto <ksakamoto@chromium.org> Date: Fri Nov 10 06:52:28 2017 Implement link rel=modulepreload This implements link rel=modulepreload without submodules fetch, behind the Experimental Web Platform feature flag. LinkLoader fetches module script using Modulator::FetchSingle, so it populates the module map but does not fetch descendant modules. Unlike the preload fetches by link rel=preload, this doesn't set FetchParameters::SetLinkPreload flag, so it works like a regular script fetch. HTMLPreloadScanner fetches module scripts in a similar way to speculative preload of <script type=module>, i.e. just fetch a ScriptResource and not populate the module map. Spec PR: https://github.com/whatwg/html/pull/2383 Intent to implement: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/ynkrM70KDD4 Bug: 740886 Change-Id: I5f4420e305f4962b3fbe3f703ce95a5a43e4f7a9 Reviewed-on: https://chromium-review.googlesource.com/662697 Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org> Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Cr-Commit-Position: refs/heads/master@{#515490} [add] https://crrev.com/80b8282d97f105ac5f59a50640d732e62374e4e2/third_party/WebKit/LayoutTests/external/wpt/preload/modulepreload.html [add] https://crrev.com/80b8282d97f105ac5f59a50640d732e62374e4e2/third_party/WebKit/LayoutTests/external/wpt/preload/resources/module1.js [add] https://crrev.com/80b8282d97f105ac5f59a50640d732e62374e4e2/third_party/WebKit/LayoutTests/external/wpt/preload/resources/module2.js [add] https://crrev.com/80b8282d97f105ac5f59a50640d732e62374e4e2/third_party/WebKit/LayoutTests/external/wpt/preload/resources/syntax-error.js [modify] https://crrev.com/80b8282d97f105ac5f59a50640d732e62374e4e2/third_party/WebKit/Source/core/dom/DynamicModuleResolver.cpp [modify] https://crrev.com/80b8282d97f105ac5f59a50640d732e62374e4e2/third_party/WebKit/Source/core/dom/DynamicModuleResolverTest.cpp [modify] https://crrev.com/80b8282d97f105ac5f59a50640d732e62374e4e2/third_party/WebKit/Source/core/dom/ModuleMap.cpp [modify] https://crrev.com/80b8282d97f105ac5f59a50640d732e62374e4e2/third_party/WebKit/Source/core/dom/ModuleMapTest.cpp [modify] https://crrev.com/80b8282d97f105ac5f59a50640d732e62374e4e2/third_party/WebKit/Source/core/dom/ScriptLoader.cpp [modify] https://crrev.com/80b8282d97f105ac5f59a50640d732e62374e4e2/third_party/WebKit/Source/core/dom/ScriptLoader.h [modify] https://crrev.com/80b8282d97f105ac5f59a50640d732e62374e4e2/third_party/WebKit/Source/core/html/LinkRelAttribute.cpp [modify] https://crrev.com/80b8282d97f105ac5f59a50640d732e62374e4e2/third_party/WebKit/Source/core/html/LinkRelAttribute.h [modify] https://crrev.com/80b8282d97f105ac5f59a50640d732e62374e4e2/third_party/WebKit/Source/core/html/RelList.cpp [modify] https://crrev.com/80b8282d97f105ac5f59a50640d732e62374e4e2/third_party/WebKit/Source/core/html/parser/HTMLPreloadScanner.cpp [modify] https://crrev.com/80b8282d97f105ac5f59a50640d732e62374e4e2/third_party/WebKit/Source/core/html/parser/PreloadRequest.cpp [modify] https://crrev.com/80b8282d97f105ac5f59a50640d732e62374e4e2/third_party/WebKit/Source/core/loader/LinkLoader.cpp [modify] https://crrev.com/80b8282d97f105ac5f59a50640d732e62374e4e2/third_party/WebKit/Source/core/loader/LinkLoader.h [modify] https://crrev.com/80b8282d97f105ac5f59a50640d732e62374e4e2/third_party/WebKit/Source/core/loader/LinkLoaderTest.cpp [modify] https://crrev.com/80b8282d97f105ac5f59a50640d732e62374e4e2/third_party/WebKit/Source/core/loader/modulescript/ModuleScriptFetchRequest.h [modify] https://crrev.com/80b8282d97f105ac5f59a50640d732e62374e4e2/third_party/WebKit/Source/core/loader/modulescript/ModuleScriptLoader.cpp [modify] https://crrev.com/80b8282d97f105ac5f59a50640d732e62374e4e2/third_party/WebKit/Source/core/loader/modulescript/ModuleScriptLoaderTest.cpp [modify] https://crrev.com/80b8282d97f105ac5f59a50640d732e62374e4e2/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.cpp [modify] https://crrev.com/80b8282d97f105ac5f59a50640d732e62374e4e2/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinkerTest.cpp [modify] https://crrev.com/80b8282d97f105ac5f59a50640d732e62374e4e2/third_party/WebKit/Source/core/workers/WorkletGlobalScope.cpp [modify] https://crrev.com/80b8282d97f105ac5f59a50640d732e62374e4e2/third_party/WebKit/Source/platform/runtime_enabled_features.json5
,
Nov 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/74c7dbf73733dbb4791e699851345f3789687498 commit 74c7dbf73733dbb4791e699851345f3789687498 Author: Kunihiko Sakamoto <ksakamoto@chromium.org> Date: Thu Nov 16 03:47:04 2017 Link rel=modulepreload: Add comments that map code to the spec This patch also fixes the followings: - Reject modulepreloads with invalid as="" value - ParserState should be "not parser inserted" Bug: 740886 Change-Id: I9f3c8e87da31a0b09bad208d681e200b6a06d69e Reviewed-on: https://chromium-review.googlesource.com/768773 Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org> Cr-Commit-Position: refs/heads/master@{#516974} [modify] https://crrev.com/74c7dbf73733dbb4791e699851345f3789687498/third_party/WebKit/LayoutTests/external/wpt/preload/modulepreload.html [modify] https://crrev.com/74c7dbf73733dbb4791e699851345f3789687498/third_party/WebKit/Source/core/loader/LinkLoader.cpp [modify] https://crrev.com/74c7dbf73733dbb4791e699851345f3789687498/third_party/WebKit/Source/core/loader/LinkLoader.h [modify] https://crrev.com/74c7dbf73733dbb4791e699851345f3789687498/third_party/WebKit/Source/core/loader/LinkLoaderTest.cpp
,
Nov 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2b7f6b1a4550dca068814989ae772a747185520c commit 2b7f6b1a4550dca068814989ae772a747185520c Author: Kunihiko Sakamoto <ksakamoto@chromium.org> Date: Thu Nov 16 07:58:55 2017 Link rel=modulepreload: Add UseCounter Bug: 740886 Change-Id: I946c3b33aef3a1bb49ffa3e73bf1c040cbc309c0 Reviewed-on: https://chromium-review.googlesource.com/771373 Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org> Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Cr-Commit-Position: refs/heads/master@{#517020} [modify] https://crrev.com/2b7f6b1a4550dca068814989ae772a747185520c/third_party/WebKit/Source/core/loader/LinkLoader.cpp [modify] https://crrev.com/2b7f6b1a4550dca068814989ae772a747185520c/third_party/WebKit/public/platform/web_feature.mojom [modify] https://crrev.com/2b7f6b1a4550dca068814989ae772a747185520c/tools/metrics/histograms/enums.xml
,
Nov 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0c7f97a60d5b54d507f9f4494f002934124b9d74 commit 0c7f97a60d5b54d507f9f4494f002934124b9d74 Author: Kunihiko Sakamoto <ksakamoto@chromium.org> Date: Tue Nov 21 01:34:58 2017 Link rel=modulepreload: Support integrity attribute This CL enables subresource integrity check for link rel=modulepreload, by passing integrity value to ScriptFetchOptions. Note that this is only for modulepreload; integrity is still not supported for link rel=preload (crbug.com/677022). Bug: 740886 Change-Id: I09159ed802e49b402e71ff464dee56a380a73b07 Reviewed-on: https://chromium-review.googlesource.com/773719 Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org> Cr-Commit-Position: refs/heads/master@{#518037} [modify] https://crrev.com/0c7f97a60d5b54d507f9f4494f002934124b9d74/third_party/WebKit/LayoutTests/external/wpt/preload/modulepreload.html [modify] https://crrev.com/0c7f97a60d5b54d507f9f4494f002934124b9d74/third_party/WebKit/Source/core/html/HTMLLinkElement.cpp [modify] https://crrev.com/0c7f97a60d5b54d507f9f4494f002934124b9d74/third_party/WebKit/Source/core/html/HTMLLinkElement.h [modify] https://crrev.com/0c7f97a60d5b54d507f9f4494f002934124b9d74/third_party/WebKit/Source/core/html/LinkStyle.cpp [modify] https://crrev.com/0c7f97a60d5b54d507f9f4494f002934124b9d74/third_party/WebKit/Source/core/loader/LinkLoader.cpp [modify] https://crrev.com/0c7f97a60d5b54d507f9f4494f002934124b9d74/third_party/WebKit/Source/core/loader/LinkLoader.h [modify] https://crrev.com/0c7f97a60d5b54d507f9f4494f002934124b9d74/third_party/WebKit/Source/core/loader/LinkLoaderTest.cpp [modify] https://crrev.com/0c7f97a60d5b54d507f9f4494f002934124b9d74/third_party/WebKit/Source/platform/loader/LinkHeader.cpp [modify] https://crrev.com/0c7f97a60d5b54d507f9f4494f002934124b9d74/third_party/WebKit/Source/platform/loader/LinkHeader.h
,
Dec 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/960460d86dcd0d20f9c382ec82924ce1a2e7940f commit 960460d86dcd0d20f9c382ec82924ce1a2e7940f Author: Hiroshige Hayashizaki <hiroshige@chromium.org> Date: Tue Dec 05 17:13:11 2017 Fire load event for link modulepreload with non-fetch errors According to the spec comment, load events (instead of error events) should be fired for modules with non-fetch errors such as parse errors. Bug: 740886 , 763597 Change-Id: I6074b36631e1299b90d52bc01e42c5ddeb957b31 Reviewed-on: https://chromium-review.googlesource.com/802251 Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org> Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org> Cr-Commit-Position: refs/heads/master@{#521726} [modify] https://crrev.com/960460d86dcd0d20f9c382ec82924ce1a2e7940f/third_party/WebKit/LayoutTests/external/wpt/preload/modulepreload.html [modify] https://crrev.com/960460d86dcd0d20f9c382ec82924ce1a2e7940f/third_party/WebKit/Source/core/loader/LinkLoader.cpp
,
Dec 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b60066879b0a160ff9111dcf92d1c1aced5648aa commit b60066879b0a160ff9111dcf92d1c1aced5648aa Author: Kunihiko Sakamoto <ksakamoto@chromium.org> Date: Mon Dec 18 03:18:16 2017 Update comment in modulepreload web platform test Bug: 740886 Change-Id: I7cdfd6099a24bd36ccb1f962317f83b5e5299f20 Reviewed-on: https://chromium-review.googlesource.com/828342 Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org> Cr-Commit-Position: refs/heads/master@{#524645} [modify] https://crrev.com/b60066879b0a160ff9111dcf92d1c1aced5648aa/third_party/WebKit/LayoutTests/external/wpt/preload/modulepreload.html
,
Feb 16 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8c9b82a67c6d7b0560df77f0e29272ae3f34d2bc commit 8c9b82a67c6d7b0560df77f0e29272ae3f34d2bc Author: Kunihiko Sakamoto <ksakamoto@chromium.org> Date: Fri Feb 16 04:01:12 2018 Ship link rel=modulepreload Intent to Ship: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/T-OkMDASYfk Bug: 740886 Change-Id: I494bc350b5a7eaa2d3f46e4fa948ef1c3b014d37 Reviewed-on: https://chromium-review.googlesource.com/923221 Reviewed-by: Kent Tamura <tkent@chromium.org> Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org> Cr-Commit-Position: refs/heads/master@{#537187} [modify] https://crrev.com/8c9b82a67c6d7b0560df77f0e29272ae3f34d2bc/third_party/WebKit/Source/platform/runtime_enabled_features.json5
,
Feb 19 2018
Tested the issue on Win-10 and mac 10.12.6 using latest chrome version #66.0.3350.0 as per the merged issue #770205 in comment #25. Attached a screen cast and screenshot for reference. Following are the steps followed to reproduce the issue. ------------ 1. Ran http server command and opened the html file in issue id: 770205 2. Opened dev tools->network tab 3. Observed that index.js file loaded twice as in the reporter version #61.0.3163.100 ksakamoto@ - Could you please check the attached screen cast and screenshot and please let us know the expected behaviour and please confirm the fix. Thanks...!!
,
Mar 7 2018
krajshree@: The processing model of <link rel="preload"> does not work well for module scripts, so we introduced a new primitive <link rel="modulepreload">. Changing Summary and Type to reflect that.
,
Nov 8
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ae4199c2bc2be11442e4e072cf30233441007c22 commit ae4199c2bc2be11442e4e072cf30233441007c22 Author: Dave Tapuska <dtapuska@chromium.org> Date: Thu Nov 08 03:29:30 2018 Remove ModulePreload runtime enabled flag. The feature shipped in M66. BUG= 740886 Change-Id: I8a0c0927e80100ba652018c4d476e8797ad0349a Reviewed-on: https://chromium-review.googlesource.com/c/1323895 Commit-Queue: Dave Tapuska <dtapuska@chromium.org> Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org> Cr-Commit-Position: refs/heads/master@{#606317} [modify] https://crrev.com/ae4199c2bc2be11442e4e072cf30233441007c22/third_party/blink/renderer/core/html/link_rel_attribute.cc [modify] https://crrev.com/ae4199c2bc2be11442e4e072cf30233441007c22/third_party/blink/renderer/core/html/rel_list.cc [modify] https://crrev.com/ae4199c2bc2be11442e4e072cf30233441007c22/third_party/blink/renderer/platform/runtime_enabled_features.json5 |
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by rtoy@chromium.org
, Jul 11 2017Status: Untriaged (was: Unconfirmed)