Bug 192734 – SourceProviders should use an actual URL instead of a string

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Keith Miller]

SourceProviders should use an actual URL instead of a string

[Keith Miller]

Created attachment 357375 [details]
WIP

[EWS Watchlist]

Comment on attachment 357375 [details]
WIP

Attachment 357375 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/10407010

New failing tests:
http/tests/workers/worker-importScripts.html
imported/w3c/web-platform-tests/FileAPI/url/url-format.any.worker.html
imported/w3c/web-platform-tests/workers/interfaces/WorkerGlobalScope/onerror/handled.html
imported/w3c/web-platform-tests/workers/Worker_ErrorEvent_error.htm
imported/w3c/web-platform-tests/workers/interfaces/WorkerGlobalScope/onerror/not-handled.html
imported/w3c/web-platform-tests/FileAPI/FileReader/workers.html
imported/w3c/web-platform-tests/workers/constructors/Worker/AbstractWorker.onerror.html
imported/w3c/web-platform-tests/workers/interfaces/WorkerGlobalScope/onerror/exception-in-onerror.html
http/tests/workers/worker-importScriptsOnError.html
imported/w3c/web-platform-tests/workers/baseurl/alpha/sharedworker.html
imported/w3c/web-platform-tests/workers/interfaces/WorkerGlobalScope/onerror/propagate-to-window-onerror.html
imported/w3c/web-platform-tests/workers/baseurl/alpha/worker.html

[EWS Watchlist]

Created attachment 357381 [details]
Archive of layout-test-results from ews100 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 357375 [details]
WIP

Attachment 357375 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/10407050

New failing tests:
http/tests/workers/worker-importScripts.html
imported/w3c/web-platform-tests/FileAPI/url/url-format.any.worker.html
imported/w3c/web-platform-tests/workers/interfaces/WorkerGlobalScope/onerror/handled.html
http/tests/workers/service/worker-fails-to-start.html
imported/w3c/web-platform-tests/FileAPI/FileReader/workers.html
imported/w3c/web-platform-tests/workers/interfaces/WorkerGlobalScope/onerror/not-handled.html
imported/w3c/web-platform-tests/workers/Worker_ErrorEvent_error.htm
imported/w3c/web-platform-tests/service-workers/service-worker/update-registration-with-type.https.html
imported/w3c/web-platform-tests/workers/constructors/Worker/AbstractWorker.onerror.html
imported/w3c/web-platform-tests/workers/interfaces/WorkerGlobalScope/onerror/exception-in-onerror.html
http/tests/workers/worker-importScriptsOnError.html
imported/w3c/web-platform-tests/workers/baseurl/alpha/sharedworker.html
imported/w3c/web-platform-tests/workers/interfaces/WorkerGlobalScope/onerror/propagate-to-window-onerror.html
imported/w3c/web-platform-tests/workers/baseurl/alpha/worker.html

[EWS Watchlist]

Created attachment 357382 [details]
Archive of layout-test-results from ews106 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 357375 [details]
WIP

Attachment 357375 [details] did not pass jsc-ews (mac):
Output: https://webkit-queues.webkit.org/results/10407045

New failing tests:
ChakraCore.yaml/ChakraCore/test/fieldopts/fieldhoist_nullfieldhoist.js.default
ChakraCore.yaml/ChakraCore/test/strict/bug212755.js.default
ChakraCore.yaml/ChakraCore/test/fieldopts/fieldhoist6b.js.default
ChakraCore.yaml/ChakraCore/test/es6/supersyntax05.js.default
ChakraCore.yaml/ChakraCore/test/es6/globalCatchNewTargetSyntaxError.js.default
ChakraCore.yaml/ChakraCore/test/strict/stricteval2-deferred.js.default
ChakraCore.yaml/ChakraCore/test/strict/nonSimpleParameterList.js.default
ChakraCore.yaml/ChakraCore/test/es6/supersyntax02.js.default
ChakraCore.yaml/ChakraCore/test/es6/globalLambdaNewTargetSyntaxError.js.default
ChakraCore.yaml/ChakraCore/test/strict/delete.js.default
ChakraCore.yaml/ChakraCore/test/LetConst/defer2.js.default
ChakraCore.yaml/ChakraCore/test/fieldopts/fieldhoist_unreachable.js.default
ChakraCore.yaml/ChakraCore/test/StackTrace/ErrorPrototype.js.default
ChakraCore.yaml/ChakraCore/test/fieldopts/fieldhoist6.js.default
ChakraCore.yaml/ChakraCore/test/fieldopts/fieldhoist_negzero.js.default
ChakraCore.yaml/ChakraCore/test/strict/stricteval-deferred.js.default
ChakraCore.yaml/ChakraCore/test/strict/strictargs2-deferred.js.default
ChakraCore.yaml/ChakraCore/test/Object/forIn.error.js.default
ChakraCore.yaml/ChakraCore/test/strict/strictargs3-deferred.js.default
ChakraCore.yaml/ChakraCore/test/LetConst/defer5.js.default
ChakraCore.yaml/ChakraCore/test/LetConst/defer1.js.default
ChakraCore.yaml/ChakraCore/test/Error/validate_line_column.js.default
ChakraCore.yaml/ChakraCore/test/Error/NativeErrors.js.default
ChakraCore.yaml/ChakraCore/test/es6/globalParamCatchNewTargetSyntaxError.js.default
ChakraCore.yaml/ChakraCore/test/strict/formal_samename1.js.default
ChakraCore.yaml/ChakraCore/test/es6/unicode_6_identifier_Blue511452.js.default
ChakraCore.yaml/ChakraCore/test/Function/deferredBadContinue.js.default
ChakraCore.yaml/ChakraCore/test/strict/multiunit.js.default
ChakraCore.yaml/ChakraCore/test/Basics/bug650104.js.default
ChakraCore.yaml/ChakraCore/test/Error/inlineSameFunc.js.default
ChakraCore.yaml/ChakraCore/test/es6/supersyntax06.js.default
ChakraCore.yaml/ChakraCore/test/StackTrace/FunctionName.js.default
ChakraCore.yaml/ChakraCore/test/fieldopts/fieldhoist7.js.default
ChakraCore.yaml/ChakraCore/test/es6/unicode_6_identifier_Blue524737.js.default
ChakraCore.yaml/ChakraCore/test/es6/globalNewTargetSyntaxError.js.default
ChakraCore.yaml/ChakraCore/test/strict/formal_samename2.js.default
ChakraCore.yaml/ChakraCore/test/strict/strictargs-deferred.js.default
ChakraCore.yaml/ChakraCore/test/fieldopts/fieldhoist_undefined_global.js.default
ChakraCore.yaml/ChakraCore/test/es6/unicode_blue_533163_utf8.js.default
apiTests

[EWS Watchlist]

Comment on attachment 357375 [details]
WIP

Attachment 357375 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10407200

New failing tests:
http/tests/workers/worker-importScripts.html
imported/w3c/web-platform-tests/FileAPI/url/url-format.any.worker.html
imported/w3c/web-platform-tests/workers/interfaces/WorkerGlobalScope/onerror/handled.html
http/tests/workers/service/worker-fails-to-start.html
imported/w3c/web-platform-tests/FileAPI/FileReader/workers.html
imported/w3c/web-platform-tests/workers/interfaces/WorkerGlobalScope/onerror/not-handled.html
imported/w3c/web-platform-tests/workers/Worker_ErrorEvent_error.htm
imported/w3c/web-platform-tests/service-workers/service-worker/update-registration-with-type.https.html
imported/w3c/web-platform-tests/workers/constructors/Worker/AbstractWorker.onerror.html
imported/w3c/web-platform-tests/workers/interfaces/WorkerGlobalScope/onerror/exception-in-onerror.html
http/tests/workers/worker-importScriptsOnError.html
imported/w3c/web-platform-tests/workers/baseurl/alpha/sharedworker.html
imported/w3c/web-platform-tests/workers/interfaces/WorkerGlobalScope/onerror/propagate-to-window-onerror.html
imported/w3c/web-platform-tests/workers/baseurl/alpha/worker.html

[EWS Watchlist]

Created attachment 357385 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 357375 [details]
WIP

Attachment 357375 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/10407221

New failing tests:
http/tests/workers/worker-importScripts.html
imported/w3c/web-platform-tests/FileAPI/url/url-format.any.worker.html
imported/w3c/web-platform-tests/workers/interfaces/WorkerGlobalScope/onerror/handled.html
imported/w3c/web-platform-tests/workers/Worker_ErrorEvent_error.htm
imported/w3c/web-platform-tests/workers/interfaces/WorkerGlobalScope/onerror/not-handled.html
imported/w3c/web-platform-tests/FileAPI/FileReader/workers.html
imported/w3c/web-platform-tests/workers/constructors/Worker/AbstractWorker.onerror.html
imported/w3c/web-platform-tests/workers/interfaces/WorkerGlobalScope/onerror/exception-in-onerror.html
http/tests/workers/worker-importScriptsOnError.html
imported/w3c/web-platform-tests/workers/baseurl/alpha/sharedworker.html
imported/w3c/web-platform-tests/workers/interfaces/WorkerGlobalScope/onerror/propagate-to-window-onerror.html
imported/w3c/web-platform-tests/workers/baseurl/alpha/worker.html

[EWS Watchlist]

Created attachment 357386 [details]
Archive of layout-test-results from ews116 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 357375 [details]
WIP

Attachment 357375 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/10408540

New failing tests:
http/tests/workers/worker-importScripts.html
http/tests/workers/worker-importScriptsOnError.html

[EWS Watchlist]

Created attachment 357393 [details]
Archive of layout-test-results from ews205 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews205  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[EWS Watchlist]

Comment on attachment 357375 [details]
WIP

Attachment 357375 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/10409833

New failing tests:
http/tests/workers/worker-importScripts.html
http/tests/workers/worker-importScriptsOnError.html

[EWS Watchlist]

Created attachment 357396 [details]
Archive of layout-test-results from ews201 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews201  Port: win-future  Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit

[Keith Miller]

Created attachment 358029 [details]
Patch

[Keith Miller]

Created attachment 358045 [details]
Patch

[EWS Watchlist]

Comment on attachment 358045 [details]
Patch

Attachment 358045 [details] did not pass jsc-ews (mac):
Output: https://webkit-queues.webkit.org/results/10537049

New failing tests:
ChakraCore.yaml/ChakraCore/test/fieldopts/fieldhoist_nullfieldhoist.js.default
ChakraCore.yaml/ChakraCore/test/strict/bug212755.js.default
ChakraCore.yaml/ChakraCore/test/fieldopts/fieldhoist6b.js.default
ChakraCore.yaml/ChakraCore/test/es6/supersyntax05.js.default
ChakraCore.yaml/ChakraCore/test/es6/globalCatchNewTargetSyntaxError.js.default
ChakraCore.yaml/ChakraCore/test/strict/stricteval2-deferred.js.default
ChakraCore.yaml/ChakraCore/test/strict/nonSimpleParameterList.js.default
ChakraCore.yaml/ChakraCore/test/es6/supersyntax02.js.default
ChakraCore.yaml/ChakraCore/test/es6/globalLambdaNewTargetSyntaxError.js.default
ChakraCore.yaml/ChakraCore/test/strict/delete.js.default
ChakraCore.yaml/ChakraCore/test/LetConst/defer2.js.default
ChakraCore.yaml/ChakraCore/test/fieldopts/fieldhoist_unreachable.js.default
ChakraCore.yaml/ChakraCore/test/StackTrace/ErrorPrototype.js.default
ChakraCore.yaml/ChakraCore/test/fieldopts/fieldhoist6.js.default
ChakraCore.yaml/ChakraCore/test/fieldopts/fieldhoist_negzero.js.default
ChakraCore.yaml/ChakraCore/test/strict/stricteval-deferred.js.default
ChakraCore.yaml/ChakraCore/test/strict/strictargs2-deferred.js.default
ChakraCore.yaml/ChakraCore/test/Object/forIn.error.js.default
ChakraCore.yaml/ChakraCore/test/strict/strictargs3-deferred.js.default
ChakraCore.yaml/ChakraCore/test/LetConst/defer5.js.default
ChakraCore.yaml/ChakraCore/test/LetConst/defer1.js.default
ChakraCore.yaml/ChakraCore/test/Error/validate_line_column.js.default
ChakraCore.yaml/ChakraCore/test/Error/NativeErrors.js.default
ChakraCore.yaml/ChakraCore/test/es6/globalParamCatchNewTargetSyntaxError.js.default
ChakraCore.yaml/ChakraCore/test/strict/formal_samename1.js.default
ChakraCore.yaml/ChakraCore/test/es6/unicode_6_identifier_Blue511452.js.default
ChakraCore.yaml/ChakraCore/test/Function/deferredBadContinue.js.default
ChakraCore.yaml/ChakraCore/test/strict/multiunit.js.default
ChakraCore.yaml/ChakraCore/test/Basics/bug650104.js.default
ChakraCore.yaml/ChakraCore/test/Error/inlineSameFunc.js.default
ChakraCore.yaml/ChakraCore/test/es6/supersyntax06.js.default
ChakraCore.yaml/ChakraCore/test/StackTrace/FunctionName.js.default
ChakraCore.yaml/ChakraCore/test/fieldopts/fieldhoist7.js.default
ChakraCore.yaml/ChakraCore/test/es6/unicode_6_identifier_Blue524737.js.default
ChakraCore.yaml/ChakraCore/test/es6/globalNewTargetSyntaxError.js.default
ChakraCore.yaml/ChakraCore/test/strict/formal_samename2.js.default
ChakraCore.yaml/ChakraCore/test/strict/strictargs-deferred.js.default
ChakraCore.yaml/ChakraCore/test/fieldopts/fieldhoist_undefined_global.js.default
ChakraCore.yaml/ChakraCore/test/es6/unicode_blue_533163_utf8.js.default
apiTests

[EWS Watchlist]

Comment on attachment 358045 [details]
Patch

Attachment 358045 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/10537109

New failing tests:
fast/css-custom-paint/arguments.html
fast/css-custom-paint/animate.html
fast/css-custom-paint/constructor.html
fast/css-custom-paint/simple-hidpi.html
fast/css-custom-paint/multiple-worklets-isolation.html
fast/css-custom-paint/leaks.html
fast/css-custom-paint/properties.html
fast/css-custom-paint/image.html
fast/css-custom-paint/registerPaintBindings.html
fast/css-custom-paint/worklet.html
fast/css-custom-paint/delay.html
fast/css-custom-paint/multiple-worklets.html
fast/css-custom-paint/delay-repaint.html
fast/css-custom-paint/animate-repaint.html

[EWS Watchlist]

Created attachment 358046 [details]
Archive of layout-test-results from ews106 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 358045 [details]
Patch

Attachment 358045 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10537174

New failing tests:
fast/css-custom-paint/arguments.html
fast/css-custom-paint/animate.html
fast/css-custom-paint/constructor.html
fast/css-custom-paint/simple-hidpi.html
fast/css-custom-paint/multiple-worklets-isolation.html
fast/css-custom-paint/leaks.html
fast/css-custom-paint/properties.html
fast/css-custom-paint/image.html
fast/css-custom-paint/registerPaintBindings.html
fast/css-custom-paint/worklet.html
fast/css-custom-paint/delay.html
fast/css-custom-paint/multiple-worklets.html
fast/css-custom-paint/delay-repaint.html
fast/css-custom-paint/animate-repaint.html

[EWS Watchlist]

Created attachment 358047 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[Keith Miller]

Created attachment 358122 [details]
Patch

[EWS Watchlist]

Comment on attachment 358122 [details]
Patch

Attachment 358122 [details] did not pass jsc-ews (mac):
Output: https://webkit-queues.webkit.org/results/10570626

New failing tests:
ChakraCore.yaml/ChakraCore/test/fieldopts/fieldhoist_nullfieldhoist.js.default
ChakraCore.yaml/ChakraCore/test/strict/bug212755.js.default
ChakraCore.yaml/ChakraCore/test/fieldopts/fieldhoist6b.js.default
ChakraCore.yaml/ChakraCore/test/es6/supersyntax05.js.default
ChakraCore.yaml/ChakraCore/test/es6/globalCatchNewTargetSyntaxError.js.default
ChakraCore.yaml/ChakraCore/test/strict/stricteval2-deferred.js.default
ChakraCore.yaml/ChakraCore/test/strict/nonSimpleParameterList.js.default
ChakraCore.yaml/ChakraCore/test/es6/supersyntax02.js.default
ChakraCore.yaml/ChakraCore/test/es6/globalLambdaNewTargetSyntaxError.js.default
ChakraCore.yaml/ChakraCore/test/strict/delete.js.default
ChakraCore.yaml/ChakraCore/test/LetConst/defer2.js.default
ChakraCore.yaml/ChakraCore/test/fieldopts/fieldhoist_unreachable.js.default
ChakraCore.yaml/ChakraCore/test/StackTrace/ErrorPrototype.js.default
ChakraCore.yaml/ChakraCore/test/fieldopts/fieldhoist6.js.default
ChakraCore.yaml/ChakraCore/test/fieldopts/fieldhoist_negzero.js.default
ChakraCore.yaml/ChakraCore/test/strict/stricteval-deferred.js.default
ChakraCore.yaml/ChakraCore/test/strict/strictargs2-deferred.js.default
ChakraCore.yaml/ChakraCore/test/Object/forIn.error.js.default
ChakraCore.yaml/ChakraCore/test/strict/strictargs3-deferred.js.default
ChakraCore.yaml/ChakraCore/test/LetConst/defer5.js.default
ChakraCore.yaml/ChakraCore/test/LetConst/defer1.js.default
ChakraCore.yaml/ChakraCore/test/Error/validate_line_column.js.default
ChakraCore.yaml/ChakraCore/test/Error/NativeErrors.js.default
ChakraCore.yaml/ChakraCore/test/es6/globalParamCatchNewTargetSyntaxError.js.default
ChakraCore.yaml/ChakraCore/test/strict/formal_samename1.js.default
ChakraCore.yaml/ChakraCore/test/es6/unicode_6_identifier_Blue511452.js.default
ChakraCore.yaml/ChakraCore/test/Function/deferredBadContinue.js.default
ChakraCore.yaml/ChakraCore/test/strict/multiunit.js.default
ChakraCore.yaml/ChakraCore/test/Basics/bug650104.js.default
ChakraCore.yaml/ChakraCore/test/Error/inlineSameFunc.js.default
ChakraCore.yaml/ChakraCore/test/es6/supersyntax06.js.default
ChakraCore.yaml/ChakraCore/test/StackTrace/FunctionName.js.default
ChakraCore.yaml/ChakraCore/test/fieldopts/fieldhoist7.js.default
ChakraCore.yaml/ChakraCore/test/es6/unicode_6_identifier_Blue524737.js.default
ChakraCore.yaml/ChakraCore/test/es6/globalNewTargetSyntaxError.js.default
ChakraCore.yaml/ChakraCore/test/strict/formal_samename2.js.default
ChakraCore.yaml/ChakraCore/test/strict/strictargs-deferred.js.default
ChakraCore.yaml/ChakraCore/test/fieldopts/fieldhoist_undefined_global.js.default
ChakraCore.yaml/ChakraCore/test/es6/unicode_blue_533163_utf8.js.default
apiTests

[Keith Miller]

Created attachment 358155 [details]
Patch

[Keith Miller]

Created attachment 358156 [details]
Patch

[Yusuke Suzuki]

Comment on attachment 358156 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=358156&action=review

r=me, nice direction.

> Source/JavaScriptCore/ChangeLog:20
> +        * jsc.cpp:

In this patch, the provider's sourceURL actually becomes URL. But in jsc shell, we previously did not use URL. We used file path instead.
This patch does not use file URL for the jsc shell. Instead we create invalid URL which holds a file path. Is my understanding correct?
If it is correct, it would be nice if we have this description in the ChangeLog.

> Source/WebCore/ChangeLog:14
> +        WTFMoving it.

Nice.

> Source/JavaScriptCore/jsc.cpp:2447
> +                URL fileNameURL = URL({ }, (fileName));

Unnecessary parentheses around `fileName`.

> Source/JavaScriptCore/jsc.cpp:2448
> +                promise = loadAndEvaluateModule(globalObject->globalExec(), makeSource(stringFromUTF(scriptBuffer), SourceOrigin { absolutePath(fileName) }, WTFMove(fileNameURL), TextPosition(), SourceProviderSourceType::Module), jsUndefined());

Do we need this `fileNameURL` variable? Passing `URL({ }, fileName)` directly to loadAndEvaluateModule seems fine.

[Keith Miller]

Comment on attachment 358156 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=358156&action=review

>> Source/JavaScriptCore/ChangeLog:20
>> +        * jsc.cpp:
> 
> In this patch, the provider's sourceURL actually becomes URL. But in jsc shell, we previously did not use URL. We used file path instead.
> This patch does not use file URL for the jsc shell. Instead we create invalid URL which holds a file path. Is my understanding correct?
> If it is correct, it would be nice if we have this description in the ChangeLog.

Yeah, I had to preserve the shell's behavior because the full url is used in the stack dump. Since our Chakra tests rely exact diffs absolute file paths was hard to get to work correctly.

>> Source/JavaScriptCore/jsc.cpp:2447
>> +                URL fileNameURL = URL({ }, (fileName));
> 
> Unnecessary parentheses around `fileName`.

fixed.

>> Source/JavaScriptCore/jsc.cpp:2448
>> +                promise = loadAndEvaluateModule(globalObject->globalExec(), makeSource(stringFromUTF(scriptBuffer), SourceOrigin { absolutePath(fileName) }, WTFMove(fileNameURL), TextPosition(), SourceProviderSourceType::Module), jsUndefined());
> 
> Do we need this `fileNameURL` variable? Passing `URL({ }, fileName)` directly to loadAndEvaluateModule seems fine.

Yeah, fixed.

[Keith Miller]

Committed r239569: <https://trac.webkit.org/changeset/239569>

[Radar WebKit Bug Importer]

<rdar://problem/47006032>

[Radar WebKit Bug Importer]

<rdar://problem/47006035>