[not-for-commit] kitsune changes + pending kouhei changes (issue 2838933003 by kouhei@chromium.org)

0 views
Skip to first unread message

kou...@chromium.org

unread,
Apr 26, 2017, 9:00:08 AM4/26/17
to hiro...@chromium.org, chromium...@chromium.org, blink-revie...@chromium.org, blink-rev...@chromium.org, tfa...@chromium.org, sigb...@opera.com, eae+bli...@chromium.org, loading-rev...@chromium.org, blink-re...@chromium.org, dglazko...@chromium.org, dominicc+...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, rob....@samsung.com
Reviewers: hiroshige
CL: https://codereview.chromium.org/2838933003/

Message:
not for review, but incase if you got stuck...

I plan to upload these changes as proper CLs this week

Description:
kitsune changes + pending kouhei changes

Affected files (+1421, -600 lines):
A dep.js
A depdep.js
A module.html
A module.js
A synerr.js
M third_party/WebKit/LayoutTests/TestExpectations
A third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/crossorigin-import-different.noinline.sub.html
A third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/crossorigin-import-different.noinline.sub.js
A third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/crossorigin-import-missingheader.noinline.sub.html
A third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/crossorigin-import-missingheader.noinline.sub.js
A third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/crossorigin-import-same.noinline.sub.html
A third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/crossorigin-import-same.noinline.sub.js
A third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/crossorigin-import-wrongheader.noinline.sub.html
A third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/crossorigin-import-wrongheader.noinline.sub.js
A third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/crossorigin.noinline.html
A third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/crossorigin.noinline-expected.txt
A third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/empty.js
A third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/errored.html
A third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/errored-root.js
M third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/errorhandling-parseerror-common.js
A third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/errorhandling-parseerror-dependent.noinline.html
A third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/errorhandling-parseerror-dependent.noinline.js
A third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/errorhandling-parseerror-dependentmultiple.noinline.html
A third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/errorhandling-parseerror-dependentmultiple.noinline-a.js
A third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/errorhandling-parseerror-dependentmultiple.noinline-b.js
A third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/errorhandling-parseerror-root.noinline.html
A third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/errorhandling-parseerror-root.noinline.js
A third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/errorhandling.noinline.html
A third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/errorhandling.noinline-expected.txt
M third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/execorder.html
A + third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/execorder.noinline.html
A third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/execorder.noinline-a.js
A third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/imports.noinline.html
A third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/imports.noinline-a.js
A third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/imports.noinline-b.js
A third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/imports.noinline-c.js
A third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/imports.noinline-expected.txt
A third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/test1.html
A third_party/WebKit/LayoutTests/external/wpt/html/semantics/scripting-1/the-script-element/module/throw.js
M third_party/WebKit/Source/bindings/core/v8/ScriptModule.h
M third_party/WebKit/Source/bindings/core/v8/ScriptModule.cpp
M third_party/WebKit/Source/bindings/core/v8/ScriptStreamer.h
M third_party/WebKit/Source/bindings/core/v8/ScriptStreamer.cpp
M third_party/WebKit/Source/bindings/core/v8/ScriptStreamerTest.cpp
M third_party/WebKit/Source/bindings/core/v8/V8ScriptRunner.h
M third_party/WebKit/Source/bindings/core/v8/V8ScriptRunner.cpp
M third_party/WebKit/Source/core/dom/BUILD.gn
A third_party/WebKit/Source/core/dom/ClassicPendingScript.h
A + third_party/WebKit/Source/core/dom/ClassicPendingScript.cpp
M third_party/WebKit/Source/core/dom/Modulator.h
M third_party/WebKit/Source/core/dom/ModulatorImpl.h
M third_party/WebKit/Source/core/dom/ModulatorImpl.cpp
M third_party/WebKit/Source/core/dom/ModuleMap.cpp
A third_party/WebKit/Source/core/dom/ModulePendingScript.h
A third_party/WebKit/Source/core/dom/ModulePendingScript.cpp
M third_party/WebKit/Source/core/dom/ModuleScript.h
M third_party/WebKit/Source/core/dom/ModuleScript.cpp
M third_party/WebKit/Source/core/dom/PendingScript.h
M third_party/WebKit/Source/core/dom/PendingScript.cpp
M third_party/WebKit/Source/core/dom/ScriptLoader.h
M third_party/WebKit/Source/core/dom/ScriptLoader.cpp
M third_party/WebKit/Source/core/dom/ScriptModuleResolverImpl.cpp
M third_party/WebKit/Source/core/dom/ScriptModuleResolverImplTest.cpp
M third_party/WebKit/Source/core/html/parser/HTMLParserScriptRunner.cpp
M third_party/WebKit/Source/core/loader/modulescript/ModuleScriptLoader.cpp
M third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.cpp
M third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinkerTest.cpp
M third_party/WebKit/Source/core/testing/DummyModulator.h
M third_party/WebKit/Source/core/testing/DummyModulator.cpp
M third_party/WebKit/Source/core/xml/parser/XMLDocumentParser.cpp
A threejs-bootstrap.html


kou...@chromium.org

unread,
Apr 26, 2017, 9:03:44 AM4/26/17
to hiro...@chromium.org, chromium...@chromium.org, blink-revie...@chromium.org, blink-rev...@chromium.org, tfa...@chromium.org, sigb...@opera.com, eae+bli...@chromium.org, loading-rev...@chromium.org, blink-re...@chromium.org, dglazko...@chromium.org, dominicc+...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, rob....@samsung.com
I've only changed places w/ comments so no need to read entire cl


https://codereview.chromium.org/2838933003/diff/1/third_party/WebKit/Source/core/dom/ModuleMap.cpp
File third_party/WebKit/Source/core/dom/ModuleMap.cpp (left):

https://codereview.chromium.org/2838933003/diff/1/third_party/WebKit/Source/core/dom/ModuleMap.cpp#oldcode105
third_party/WebKit/Source/core/dom/ModuleMap.cpp:105:
DCHECK(!is_fetching_);
The module script maybe during fetch if called from
MTL::UninstantiatedDescendants() and that's fine.

https://codereview.chromium.org/2838933003/diff/1/third_party/WebKit/Source/core/dom/ModuleMap.cpp
File third_party/WebKit/Source/core/dom/ModuleMap.cpp (right):

https://codereview.chromium.org/2838933003/diff/1/third_party/WebKit/Source/core/dom/ModuleMap.cpp#newcode155
third_party/WebKit/Source/core/dom/ModuleMap.cpp:155: return nullptr;
I forgot why, but we need this.

https://codereview.chromium.org/2838933003/diff/1/third_party/WebKit/Source/core/dom/ScriptModuleResolverImpl.cpp
File third_party/WebKit/Source/core/dom/ScriptModuleResolverImpl.cpp
(right):

https://codereview.chromium.org/2838933003/diff/1/third_party/WebKit/Source/core/dom/ScriptModuleResolverImpl.cpp#newcode80
third_party/WebKit/Source/core/dom/ScriptModuleResolverImpl.cpp:80:
ModuleInstantiationState::kInstantiated) << "maybe cycles: " <<
module_script->BaseURL().GetString() << "referrer module " <<
referrer_module->BaseURL().GetString();
As discussed in hangout, this assert doesn't hold when we have cycles.

https://codereview.chromium.org/2838933003/diff/1/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.cpp
File
third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.cpp
(right):

https://codereview.chromium.org/2838933003/diff/1/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.cpp#newcode254
third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.cpp:254:
if (urls.IsEmpty()) {
DCHECK doesn't always hold. The requests may be all in ancestor_list

https://codereview.chromium.org/2838933003/

hiro...@chromium.org

unread,
Apr 27, 2017, 9:15:36 PM4/27/17
to kouhei...@chromium.org, chromium...@chromium.org, blink-revie...@chromium.org, blink-rev...@chromium.org, tfa...@chromium.org, sigb...@opera.com, eae+bli...@chromium.org, loading-rev...@chromium.org, blink-re...@chromium.org, dglazko...@chromium.org, dominicc+...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, rob....@samsung.com

https://codereview.chromium.org/2838933003/diff/1/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.cpp
File
third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.cpp
(right):

https://codereview.chromium.org/2838933003/diff/1/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.cpp#newcode254
third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.cpp:254:
if (urls.IsEmpty()) {
On 2017/04/26 13:03:44, kouhei wrote:
> DCHECK doesn't always hold. The requests may be all in ancestor_list

Memo: This causes crash in imports.html wpt test.

https://codereview.chromium.org/2838933003/
Reply all
Reply to author
Forward
0 new messages