Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(24)

Issue 2704323002: [ES6 modules] Introduce ModuleMap (Closed)

Created:
3 years, 10 months ago by kouhei (in TOK)
Modified:
3 years, 9 months ago
CC:
chromium-reviews, sof, eae+blinkwatch, blink-reviews-dom_chromium.org, dglazkov+blink, blink-reviews, rwlbuis
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[ES6 modules] Introduce ModuleMap This CL introduces ModuleMap, which corresponds to "module map" spec concept. ModuleMap keeps track of every ModuleScripts loaded inside a single Modulator. When a ModuleScript instance is needed, we query ModuleMap to find if there is a existing instance of the ModuleScript. If there isn't an existing instance, ModuleMap is responsible for fetching it. See below diagram for where it stands in the stack: https://docs.google.com/document/d/1vjiWxwhg9D0GNNOYgw3AxMG0iKOC9I3jlID4GTgZsac/edit#heading=h.47x0qrpzjbj4 BUG=594639 Review-Url: https://codereview.chromium.org/2704323002 Cr-Commit-Position: refs/heads/master@{#453141} Committed: https://chromium.googlesource.com/chromium/src/+/95773d4b1d9191b58e36e2c3b8329abf07c80a8a

Patch Set 1 #

Total comments: 28

Patch Set 2 : hiroshige review / rebased #

Total comments: 15

Patch Set 3 : yhirano #

Patch Set 4 : unlined #

Patch Set 5 : no core expor #

Unified diffs Side-by-side diffs Delta from patch set Stats (+608 lines, -0 lines) Patch
M third_party/WebKit/Source/core/BUILD.gn View 1 2 3 2 chunks +3 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/BUILD.gn View 1 2 chunks +3 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/Modulator.h View 1 2 chunks +31 lines, -0 lines 0 comments Download
A third_party/WebKit/Source/core/dom/ModuleMap.h View 1 2 1 chunk +60 lines, -0 lines 0 comments Download
A third_party/WebKit/Source/core/dom/ModuleMap.cpp View 1 2 1 chunk +146 lines, -0 lines 0 comments Download
A third_party/WebKit/Source/core/dom/ModuleMapTest.cpp View 1 2 3 1 chunk +248 lines, -0 lines 0 comments Download
A third_party/WebKit/Source/core/dom/ScriptModuleResolver.h View 1 chunk +43 lines, -0 lines 0 comments Download
A third_party/WebKit/Source/core/testing/DummyModulator.h View 1 2 3 4 1 chunk +43 lines, -0 lines 0 comments Download
A third_party/WebKit/Source/core/testing/DummyModulator.cpp View 1 2 3 1 chunk +31 lines, -0 lines 0 comments Download

Depends on Patchset:

Messages

Total messages: 31 (21 generated)
kouhei (in TOK)
3 years, 10 months ago (2017-02-21 06:49:10 UTC) #4
kouhei (in TOK)
win link failure expected. will fix later https://codereview.chromium.org/2704323002/diff/1/third_party/WebKit/Source/core/testing/DummyModulator.h File third_party/WebKit/Source/core/testing/DummyModulator.h (right): https://codereview.chromium.org/2704323002/diff/1/third_party/WebKit/Source/core/testing/DummyModulator.h#newcode33 third_party/WebKit/Source/core/testing/DummyModulator.h:33: virtual ~DummyModulator() ...
3 years, 10 months ago (2017-02-21 06:50:58 UTC) #5
hiroshige
https://codereview.chromium.org/2704323002/diff/1/third_party/WebKit/Source/core/dom/Modulator.h File third_party/WebKit/Source/core/dom/Modulator.h (right): https://codereview.chromium.org/2704323002/diff/1/third_party/WebKit/Source/core/dom/Modulator.h#newcode29 third_party/WebKit/Source/core/dom/Modulator.h:29: // spec: "top-level module fetch flag" +link? https://codereview.chromium.org/2704323002/diff/1/third_party/WebKit/Source/core/dom/Modulator.h#newcode51 third_party/WebKit/Source/core/dom/Modulator.h:51: ...
3 years, 10 months ago (2017-02-22 01:21:31 UTC) #8
kouhei (in TOK)
https://codereview.chromium.org/2704323002/diff/1/third_party/WebKit/Source/core/dom/Modulator.h File third_party/WebKit/Source/core/dom/Modulator.h (right): https://codereview.chromium.org/2704323002/diff/1/third_party/WebKit/Source/core/dom/Modulator.h#newcode29 third_party/WebKit/Source/core/dom/Modulator.h:29: // spec: "top-level module fetch flag" On 2017/02/22 01:21:30, ...
3 years, 10 months ago (2017-02-22 07:08:04 UTC) #11
yhirano
https://codereview.chromium.org/2704323002/diff/20001/third_party/WebKit/Source/core/dom/ModuleMap.cpp File third_party/WebKit/Source/core/dom/ModuleMap.cpp (right): https://codereview.chromium.org/2704323002/diff/20001/third_party/WebKit/Source/core/dom/ModuleMap.cpp#newcode35 third_party/WebKit/Source/core/dom/ModuleMap.cpp:35: Entry(ModuleMap*); +explicit https://codereview.chromium.org/2704323002/diff/20001/third_party/WebKit/Source/core/dom/ModuleMap.h File third_party/WebKit/Source/core/dom/ModuleMap.h (right): https://codereview.chromium.org/2704323002/diff/20001/third_party/WebKit/Source/core/dom/ModuleMap.h#newcode39 third_party/WebKit/Source/core/dom/ModuleMap.h:39: ModuleScript* ...
3 years, 10 months ago (2017-02-22 09:10:42 UTC) #14
kouhei (in TOK)
https://codereview.chromium.org/2704323002/diff/20001/third_party/WebKit/Source/core/dom/ModuleMap.cpp File third_party/WebKit/Source/core/dom/ModuleMap.cpp (right): https://codereview.chromium.org/2704323002/diff/20001/third_party/WebKit/Source/core/dom/ModuleMap.cpp#newcode35 third_party/WebKit/Source/core/dom/ModuleMap.cpp:35: Entry(ModuleMap*); On 2017/02/22 09:10:42, yhirano wrote: > +explicit Done. ...
3 years, 10 months ago (2017-02-24 02:10:31 UTC) #15
yhirano
lgtm
3 years, 10 months ago (2017-02-24 03:39:57 UTC) #18
kouhei (in TOK)
On 2017/02/24 03:39:57, yhirano wrote: > lgtm Thanks. I'll commit this once DummyModulator is uninlined ...
3 years, 10 months ago (2017-02-24 03:45:07 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2704323002/80001
3 years, 9 months ago (2017-02-27 02:07:12 UTC) #28
commit-bot: I haz the power
3 years, 9 months ago (2017-02-27 03:52:47 UTC) #31
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/95773d4b1d9191b58e36e2c3b832...

Powered by Google App Engine
This is Rietveld 408576698