importScripts() should throw exception with a reason when it fails to load a script from the network.
Created attachment 337315 [details] Patch
Comment on attachment 337315 [details] Patch Attachment 337315 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/7220812 New failing tests: http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp-importScripts-redirect-cross-origin-blocked.html http/tests/security/contentSecurityPolicy/worker-csp-importScripts-redirect-cross-origin-blocked.html
Created attachment 337319 [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
Comment on attachment 337315 [details] Patch Attachment 337315 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/7220819 New failing tests: http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp-importScripts-redirect-cross-origin-blocked.html http/tests/security/contentSecurityPolicy/worker-csp-importScripts-redirect-cross-origin-blocked.html
Created attachment 337321 [details] Archive of layout-test-results from ews107 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
Comment on attachment 337315 [details] Patch Attachment 337315 [details] did not pass win-ews (win): Output: http://webkit-queues.webkit.org/results/7220926 New failing tests: http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp-importScripts-redirect-cross-origin-blocked.html http/tests/security/contentSecurityPolicy/worker-csp-importScripts-redirect-cross-origin-blocked.html
Created attachment 337324 [details] Archive of layout-test-results from ews200 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews200 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment on attachment 337315 [details] Patch Attachment 337315 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/7220869 New failing tests: http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp-importScripts-redirect-cross-origin-blocked.html http/tests/security/contentSecurityPolicy/worker-csp-importScripts-redirect-cross-origin-blocked.html
Created attachment 337325 [details] Archive of layout-test-results from ews124 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.4
Comment on attachment 337315 [details] Patch Attachment 337315 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/7220923 New failing tests: http/tests/security/contentSecurityPolicy/worker-blob-inherits-csp-importScripts-redirect-cross-origin-blocked.html http/tests/security/contentSecurityPolicy/worker-csp-importScripts-redirect-cross-origin-blocked.html
Created attachment 337329 [details] Archive of layout-test-results from ews115 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews115 Port: mac-sierra Platform: Mac OS X 10.12.6
Created attachment 337368 [details] Patch
Comment on attachment 337368 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=337368&action=review > Source/WebCore/workers/WorkerGlobalScope.cpp:292 > + return Exception { NetworkError, String { scriptLoader->error().localizedDescription() } }; I thought that the policy was that developer facing error messages were not localized. Is that incorrect? Also, why is constructing a String necessary here?
(In reply to Alexey Proskuryakov from comment #13) > Comment on attachment 337368 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=337368&action=review > > > Source/WebCore/workers/WorkerGlobalScope.cpp:292 > > + return Exception { NetworkError, String { scriptLoader->error().localizedDescription() } }; > > I thought that the policy was that developer facing error messages were not > localized. Is that incorrect? > As I wrote on IRC today (04/06): [09:40am] dydx: ap: ResourceError extends ResourceErrorBase and ResourceErrorBase owns a single string ResourceErrorBase::m_localizedDescription As far as I can tell ResourceErrorBase::m_localizedDescription is a misnomer. It always represents a non-localized string. We should consider renaming it. > Also, why is constructing a String necessary here? [09:41am] dydx: ap: Exception takes an optional message as its second argument by rvalue reference; => we need to copy the String [09:41am] dydx: ap: We should change Exception to take a String then it will support both moving and copying semantics based on how the caller passes the String
Comment on attachment 337368 [details] Patch > As far as I can tell ResourceErrorBase::m_localizedDescription is a misnomer. It always represents a non-localized string. We should consider renaming it. How did you come to this conclusion? On Mac at least, it's calling an SPI which performs localization work. However, there is a lot of precedent for using this particular ResourceError function. I think that it is unfortunate that we are mixing localized platform error strings with non-localized WebCore strings. As mentioned before, my understanding is that we want both non-localized, but I don't really know.
(In reply to Alexey Proskuryakov from comment #15) > Comment on attachment 337368 [details] > Patch > > > As far as I can tell ResourceErrorBase::m_localizedDescription is a misnomer. It always represents a non-localized string. We should consider renaming it. > > How did you come to this conclusion? On Mac at least, it's calling an SPI > which performs localization work. > I'm blind.
Committed r230345: <https://trac.webkit.org/changeset/230345>
(In reply to Daniel Bates from comment #14) > > Also, why is constructing a String necessary here? > > [09:41am] dydx: ap: Exception takes an optional message as its second > argument by rvalue reference; => we need to copy the String > [09:41am] dydx: ap: We should change Exception to take a String then it will > support both moving and copying semantics based on how the caller passes the > String For completeness, I chose to teach Exception to take a String by value in the patch for bug #184360 and updated this patch to remove the explicit call to the String constructor.
<rdar://problem/39245413>