Description
youenn fablet
2016-04-27 06:08:10 PDT
Currently worker-imported scripts, as tested by <https://github.com/w3c/web-platform-tests/blob/master/fetch/nosniff/importscripts.html>, do not respect X-Content-Type-Options: nosniff. We will implement such support in bug #171248. I am assuming you mean that running the test represented by <https://github.com/w3c/web-platform-tests/blob/master/fetch/nosniff/importscripts.html> causes a crash as opposed to accessing <https://github.com/w3c/web-platform-tests/blob/master/fetch/nosniff/importscripts.html>. For completeness, the test <https://github.com/w3c/web-platform-tests/blob/master/fetch/nosniff/importscripts.html> has since been imported into LayoutTests/imported/w3c/web-platform-tests/fetch/nosniff/importscripts.html. This now hits ASSERTION FAILED: !m_isolatedWorld->isNormal() || m_wrapper || !m_jsFunction I can reproduce quite easily when running in a loop. I'll attach a patch that reduces the test case somewhat. Created attachment 329570 [details]
Patch to reduce test
This assertion generally means that an event listener wasn't protected from GC, and got collected. That would be a pretty bad bug for Fetch if so. Where is the full stack trace? I don't know that it's a bug in fetch code, particularly. It's the onmessage handler for the Worker that disappears. STDERR: ../../Source/WebCore/bindings/js/JSEventListener.h(128) : JSC::JSObject* WebCore::JSEventListener::jsFunction(WebCore::ScriptExecutionContext&) const STDERR: 1 0x7f717617d147 /WebKit/GTK/WebKit/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(WTFCrash+0x1e) [0x7f717617d147] STDERR: 2 0x7f718333e1c3 /WebKit/GTK/WebKit/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebCore::JSEventListener::jsFunction(WebCore::ScriptExecutionContext&) const+0x1a5) [0x7f718333e1c3] STDERR: 3 0x7f718333c5a8 /WebKit/GTK/WebKit/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext&, WebCore::Event&)+0xf6) [0x7f718333c5a8] STDERR: 4 0x7f7183705dac /WebKit/GTK/WebKit/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebCore::EventTarget::fireEventListeners(WebCore::Event&, WTF::Vector<WTF::RefPtr<WebCore::RegisteredEventListener, WTF::DumbPtrTraits<WebCore::RegisteredEventListener> >, 1ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>)+0x3bc) [0x7f7183705dac] STDERR: 5 0x7f71837058bb /WebKit/GTK/WebKit/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebCore::EventTarget::fireEventListeners(WebCore::Event&)+0x11f) [0x7f71837058bb] STDERR: 6 0x7f718370564b /WebKit/GTK/WebKit/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebCore::EventTarget::dispatchEvent(WebCore::Event&)+0xf3) [0x7f718370564b] STDERR: 7 0x7f718471db8f /WebKit/GTK/WebKit/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(+0xa6bdb8f) [0x7f718471db8f] STDERR: 8 0x7f718472aa96 /WebKit/GTK/WebKit/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(+0xa6caa96) [0x7f718472aa96] STDERR: 9 0x7f71831f9848 /WebKit/GTK/WebKit/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WTF::Function<void (WebCore::ScriptExecutionContext&)>::operator()(WebCore::ScriptExecutionContext&) const+0x78) [0x7f71831f9848] STDERR: 10 0x7f71831f83a7 /WebKit/GTK/WebKit/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebCore::ScriptExecutionContext::Task::performTask(WebCore::ScriptExecutionContext&)+0x23) [0x7f71831f83a7] STDERR: 11 0x7f718366c6e4 /WebKit/GTK/WebKit/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(+0x960c6e4) [0x7f718366c6e4] STDERR: 12 0x7f718367e548 /WebKit/GTK/WebKit/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(+0x961e548) [0x7f718367e548] STDERR: 13 0x7f7181aa0398 /WebKit/GTK/WebKit/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WTF::Function<void ()>::operator()() const+0x5e) [0x7f7181aa0398] STDERR: 14 0x7f71761980ef /WebKit/GTK/WebKit/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(WTF::dispatchFunctionsFromMainThread()+0x105) [0x7f71761980ef] STDERR: 15 0x7f71761fc8df /WebKit/GTK/WebKit/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(WTF::MainThreadDispatcher::fired()+0x11) [0x7f71761fc8df] STDERR: 16 0x7f71761fca44 /WebKit/GTK/WebKit/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(WTF::RunLoop::Timer<WTF::MainThreadDispatcher>::fired()+0x66) [0x7f71761fca44] STDERR: 17 0x7f71761ffb81 /WebKit/GTK/WebKit/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(+0x2d73b81) [0x7f71761ffb81] STDERR: 18 0x7f71761ffbbd /WebKit/GTK/WebKit/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(+0x2d73bbd) [0x7f71761ffbbd] STDERR: 19 0x7f71761ff242 /WebKit/GTK/WebKit/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(+0x2d73242) [0x7f71761ff242] STDERR: 20 0x7f71761ff271 /WebKit/GTK/WebKit/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(+0x2d73271) [0x7f71761ff271] STDERR: 21 0x7f7177426a4a /WebKit/GTK/WebKit/WebKitBuild/DependenciesGTK/Root/lib/libglib-2.0.so.0(g_main_context_dispatch+0x15a) [0x7f7177426a4a] STDERR: 22 0x7f7177426df8 /WebKit/GTK/WebKit/WebKitBuild/DependenciesGTK/Root/lib/libglib-2.0.so.0(+0x4adf8) [0x7f7177426df8] STDERR: 23 0x7f7177427122 /WebKit/GTK/WebKit/WebKitBuild/DependenciesGTK/Root/lib/libglib-2.0.so.0(g_main_loop_run+0xc2) [0x7f7177427122] STDERR: 24 0x7f71761ff7c8 /WebKit/GTK/WebKit/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(WTF::RunLoop::run()+0xba) [0x7f71761ff7c8] STDERR: 25 0x7f71823fb0e6 /WebKit/GTK/WebKit/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(int WebKit::ChildProcessMain<WebKit::WebProcess, WebKit::WebProcessMain>(int, char**)+0x91) [0x7f71823fb0e6] STDERR: 26 0x7f71823faf8a /WebKit/GTK/WebKit/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebProcessMainUnix+0x20) [0x7f71823faf8a] STDERR: 27 0x4009b1 /WebKit/GTK/WebKit/WebKitBuild/Debug/bin/WebKitWebProcess(main+0x3d) [0x4009b1] STDERR: 28 0x7f71799a7830 /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf0) [0x7f71799a7830] STDERR: 29 0x400869 /WebKit/GTK/WebKit/WebKitBuild/Debug/bin/WebKitWebProcess(_start+0x29) [0x400869] (In reply to Ms2ger from comment #7) > I don't know that it's a bug in fetch code, particularly. It's the onmessage > handler for the Worker that disappears. Looking at the code, JSWorker is not collectable when loading the script. After that point, it is again collectable. I guess we could extend the fact that it is not collectable until being terminated. Or if it has an on message event handler. Created attachment 330374 [details]
Patch
Comment on attachment 330374 [details] Patch Attachment 330374 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/5909433 New failing tests: fast/workers/dedicated-worker-lifecycle.html fast/workers/worker-exception-during-navigation.html Created attachment 330379 [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.12.6
Comment on attachment 330374 [details] Patch Attachment 330374 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/5915689 New failing tests: inspector/worker/worker-create-and-terminate.html fast/workers/termination-with-port-messages.html fast/workers/worker-document-leak.html fast/workers/worker-terminate.html Created attachment 330399 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 330374 [details] Patch Attachment 330374 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/5915958 New failing tests: fast/workers/termination-with-port-messages.html html5lib/generated/run-comments01-data.html fast/workers/dedicated-worker-lifecycle.html inspector/worker/worker-create-and-terminate.html fast/workers/worker-terminate.html inspector/unit-tests/test-harness-expect-functions-async.html Created attachment 330402 [details]
Archive of layout-test-results from ews101 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 330403 [details]
Patch
Comment on attachment 330403 [details] Patch Attachment 330403 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/5916834 New failing tests: fast/workers/worker-document-leak.html http/wpt/resource-timing/rt-initiatorType-other.html Created attachment 330406 [details]
Archive of layout-test-results from ews103 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 330403 [details] Patch Attachment 330403 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/5917209 New failing tests: fast/workers/worker-document-leak.html Created attachment 330410 [details]
Archive of layout-test-results from ews115 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 330403 [details] Patch Attachment 330403 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/5917717 New failing tests: fast/mediastream/MediaStream-MediaElement-setObject-null.html Created attachment 330412 [details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Created attachment 340181 [details]
Patch
Comment on attachment 340181 [details] Patch Attachment 340181 [details] did not pass win-ews (win): Output: http://webkit-queues.webkit.org/results/7650044 New failing tests: http/tests/security/contentSecurityPolicy/userAgentShadowDOM/allow-audio.html Created attachment 340188 [details]
Archive of layout-test-results from ews204 for win-future
The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews204 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment on attachment 340181 [details]
Patch
win error is unrelated
*** Bug 173432 has been marked as a duplicate of this bug. *** Comment on attachment 340181 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=340181&action=review > Source/WebCore/ChangeLog:8 > + Make Worker uncollectable if it has an onmessage event handler. Doesn't this mean that it's essentially never collectable? If onmessage is set, we cannot let the worker be collected since we do not know if a message will have to be dispatched in the future. A web page can always stop the worker or unset onmessage. The fact that it's possible for a page to do something explicit to avoid performance problems is not enough for us. What if it doesn't? Worker lifetime is directly addressed in the spec, <https://w3c.github.io/workers/#worker-lifetime>. Having an onmessage handler on Worker is not the end of the story. Comment on attachment 340181 [details]
Patch
OK, will try to come up with something more aggressive.
Created attachment 340699 [details]
Patch
Comment on attachment 340699 [details] Patch Attachment 340699 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/7722599 New failing tests: transitions/created-while-suspended.html Created attachment 340701 [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
> New failing tests:
> transitions/created-while-suspended.html
Failure seems unrelated
*** Bug 186978 has been marked as a duplicate of this bug. *** Comment on attachment 340699 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=340699&action=review > Source/WebCore/workers/DedicatedWorkerGlobalScope.cpp:87 > + thread().runLoop().postTaskForMode([](auto& context) { Can some assertions or comments be added to clarify why m_importScriptCount isn't being accessed from multiple threads concurrently? I'm assuming that's true, but I don't know how to confirm that. Other that that, leaving for someone else to review. Created attachment 344632 [details]
Patch
(In reply to Alexey Proskuryakov from comment #38) > Comment on attachment 340699 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=340699&action=review > > > Source/WebCore/workers/DedicatedWorkerGlobalScope.cpp:87 > > + thread().runLoop().postTaskForMode([](auto& context) { > > Can some assertions or comments be added to clarify why m_importScriptCount > isn't being accessed from multiple threads concurrently? I'm assuming that's > true, but I don't know how to confirm that. Added a comment. importScripts is called in the worker thread and the lambda as well so m_importScriptCount is only accessed from the worker thread. Comment on attachment 344632 [details] Patch Attachment 344632 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/8491450 New failing tests: http/tests/preload/onload_event.html Created attachment 344676 [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
Comment on attachment 344632 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=344632&action=review > Source/WebCore/ChangeLog:10 > + Decrement the counter after an even loop iteration to handle the case of importScripts being called one after the other. s/even/event/ > Source/WebCore/workers/DedicatedWorkerGlobalScope.cpp:-83 > - thread().workerObjectProxy().reportPendingActivity(hasPendingActivity()); So hasPendingActivity() has to do with ActiveDOMObjects, not loads. Could you explain to me why moving the call to thread().workerObjectProxy().reportPendingActivity(hasPendingActivity()); before the call to Base::importScripts(urls) work? > Source/WebCore/workers/DedicatedWorkerGlobalScope.cpp:87 > + // We decrement m_importScriptCount asycnhronously in case another importScripts is made synchronously. m_importScriptCount is modified only in Worker thread. Typo: asycnhronously > Source/WebCore/workers/DedicatedWorkerGlobalScope.h:73 > + static bool isType(const WebCore::ScriptExecutionContext& context) We should add an overload that takes a WorkerGlobalScope& in parameter. Then the overload that takes a ScriptExecutionContext& could do the is<WebCore::WorkerGlobalScope>(context) and call this new one. Comment on attachment 344632 [details]
Patch
This has been requesting review for more than one year. If this is still needed, please rebase and re-request review.
|