Open
Bug 1394102
Opened 7 years ago
Updated 3 days ago
FetchEvent.request.signal should be correctly aborted
Categories
(Core :: DOM: Service Workers, enhancement, P2)
Core
DOM: Service Workers
Tracking
()
NEW
People
(Reporter: baku, Unassigned)
References
(Blocks 1 open bug)
Details
Attachments
(2 files, 3 obsolete files)
|
2.52 KB,
patch
|
bkelly
:
review+
|
Details | Diff | Splinter Review |
|
11.74 KB,
patch
|
bkelly
:
review+
|
Details | Diff | Splinter Review |
This is a follow up of bug 1378342 comment 31.
|
||
Updated•7 years ago
|
Blocks: ServiceWorkers-compat
Summary: FetchEvent.signal should be correctly aborted → FetchEvent.request.signal should be correctly aborted
|
|
||
Comment 1•7 years ago
|
||
Basically we need to forward any cancellation of the outer intercepted channel to FetchEvent.request.signal.
|
|
||
Comment 2•7 years ago
|
||
Jake says he would be happy for us to add a WPT test for this and upstream through our gecko sync mechanism.
|
||
Updated•7 years ago
|
Priority: -- → P2
|
Reporter | |
Comment 3•7 years ago
|
||
Assignee: nobody → amarchesini
Attachment #8902677 -
Flags: review?(bkelly)
|
|
Reporter | |
Comment 4•7 years ago
|
||
Attachment #8902677 -
Attachment is obsolete: true
Attachment #8902677 -
Flags: review?(bkelly)
Attachment #8902680 -
Flags: review?(bkelly)
|
|
||
Comment 5•7 years ago
|
||
Comment on attachment 8902680 [details] [diff] [review] network_2.patch Review of attachment 8902680 [details] [diff] [review]: ----------------------------------------------------------------- Looks like its on the right track, but I have some questions/comments. Also, I think we should see this work in a WPT before r+. ::: dom/workers/ServiceWorkerPrivate.cpp @@ +1364,5 @@ > // Inheriting ExtendableEventWorkerRunnable so that the worker is not terminated > // while handling the fetch event, though that's very unlikely. > class FetchEventRunnable : public ExtendableFunctionalEventWorkerRunnable > + , public nsIHttpHeaderVisitor > + , public nsIObserver The FetchEventRunnable is quite large and normally is discarded after the event handler is run. It seems unfortunate to force it to stay alive until the respondWith() promise settles and the body is fully read. That could be a long time. Could we instead make a smaller nsIObserver object that does this logic? Then the larger FetchEventRunnable could be deallocated and we would only have to maintain this smaller observer. @@ +1617,5 @@ > + // Let's keep it alive until the operation is completed. > + RefPtr<FetchEventRunnable> kungFuDeathGrip = this; > + nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService(); > + if (obs) { > + obs->RemoveObserver(this, NS_HTTP_ON_STOP_REQUEST_TOPIC); Are we guaranteed this will happen if a FetchEvent respondWith() never resolves and the worker thread is terminated? I think we synthesize a failure on the channel. Right? Also, if the AbortSignal is not held alive by anything it would be nice to drop this observer sooner. I guess that would require creating a class that inherits from the AbortSignal that knows to proxy release the observer stuff on destruction. What do you think? @@ +1630,5 @@ > + > + // This will release AbortSignal on the correct thread. > + RefPtr<FetchEventAbortSignalRunnable> r = > + new FetchEventAbortSignalRunnable(mWorkerPrivate, this, canceled); > + if (NS_WARN_IF(!r->Dispatch())) { Just to make sure I understand, its ok if this fails because if the worker is shutting down then the Notify() will have released the signal already. And we don't care about signaling that a cancel occurred since the worker is going away anyway. Right?
Attachment #8902680 -
Flags: review?(bkelly) → feedback+
|
|
Reporter | |
Comment 6•7 years ago
|
||
Here a test. I'll apply the comment on the previous patch for today.
Attachment #8903478 -
Flags: review?(bkelly)
|
|
||
Updated•7 years ago
|
Attachment #8903478 -
Flags: review?(bkelly) → review+
|
|
Reporter | |
Comment 7•7 years ago
|
||
Attachment #8902680 -
Attachment is obsolete: true
Attachment #8903592 -
Flags: review?(bkelly)
|
|
||
Comment 8•7 years ago
|
||
Comment on attachment 8903592 [details] [diff] [review] network_2.patch Review of attachment 8903592 [details] [diff] [review]: ----------------------------------------------------------------- r=me with comments addressed. ::: dom/workers/ServiceWorkerPrivate.cpp @@ +1321,5 @@ > +// keeps alive AbortSignal and releases it on the correct thread. > +class AbortSignalWorkerHolder final : public WorkerHolder > +{ > +public: > + AbortSignalWorkerHolder(AbortChannelObserver* aObserver, AbortSignal* aSignal) Lets not increase the busy count with this holder. Can you initialize WorkerHolder() with AllowIdleShutdownStart? It doesn't completely matter today for service workers, but we might change how things shutdown for SW in the future. @@ +1384,5 @@ > + > + static already_AddRefed<AbortChannelObserver> > + Create(nsIChannel* aChannel) > + { > + MOZ_ASSERT(aChannel); Assert main thread please. @@ +1397,5 @@ > + return nullptr; > + } > + > + if (NS_FAILED(obs->AddObserver(observer, NS_HTTP_ON_STOP_REQUEST_TOPIC, > + false))) { I still think it would be nice to let this stuff GC if no one uses the signal, but I guess this is ok for now. If the nsIChannel does not call OnStopRequest(), though, we will definitely leak the channel forever here. Also, it doesn't make much sense to me why the necko folks want to go through the observer service here. I mean, we have the channel. We could just add ourselves as some kind of listener directly on the channel itself and not need to go through a global mechanism at all. But if this is what the recommended, ok. @@ +1445,5 @@ > + > + // This will release AbortSignal on the correct thread. > + RefPtr<AbortSignalRunnable> r = > + new AbortSignalRunnable(mWorkerHolder->GetWorkerPrivate(), this, canceled); > + if (NS_WARN_IF(!r->Dispatch())) { Can you drop the lock before calling Dispatch() since it also grabs a lock? @@ +1463,5 @@ > + > + MutexAutoLock lock(mMutex); > + > + RefPtr<AbortSignal> abortSignal = > + new AbortSignal(mState == eChannelAborted); Should this check for mWorkerHolder already existing and get its signal? Or maybe MOZ_DIAGNOSTIC_ASSERT(!mWorkerHolder)? @@ +1469,5 @@ > + if (mState == eChannelActive) { > + // This is needed to keep AbortSignal alive. > + UniquePtr<AbortSignalWorkerHolder> handler( > + new AbortSignalWorkerHolder(this, abortSignal)); > + if (NS_WARN_IF(!handler->HoldWorker(aWorkerPrivate, Terminating))) { You're Notify() method below tears down immediately in Closing state, but you allow a new holder to be registered here in Closing. It kind of doesn't matter right now because we only really Terminate service workers, but it would be nice to make these match in case service worker shutdown changes in the future. HoldWorker grabs a lock. I think we should probably do this outside of the lock. So something like: { // Lock // Check state } // HoldWorker [ // Lock // Check state again and discard new holder if its not needed any more // Otherwise save new holder and return signal } @@ +1482,5 @@ > + void > + Reset() > + { > + MutexAutoLock lock(mMutex); > + mWorkerHolder = nullptr; Even though you have a mutex here the WorkerHolder can only be destroyed on the worker thread. Can you please assert that? Also, since destroying the WorkerHolder might do some extra work in WorkerPrivate can you move mWorkerHolder to a temporary while you hold the lock and then delete it outside of the lock? @@ +1486,5 @@ > + mWorkerHolder = nullptr; > + } > + > + void > + MaybeAbortAndReleaseSignal(bool aToAbort) Please assert the worker thread here since we can only delete WorkerHolder on the worker thread. @@ +1497,5 @@ > + mWorkerHolder->Signal()->Abort(); > + } > + } > + > + mWorkerHolder = nullptr; I think you need to clear mWorkerHolder inside the lock since other threads check for its existence. But you probably want to move it to a temporary variable and do the actual deleting where you have it here.
Attachment #8903592 -
Flags: review?(bkelly) → review+
|
|
||
Comment 9•7 years ago
|
||
Note, this may work better now that bug 1391693 has landed. Also, I'm further improving some of the cancelation paths in bug 1204254 lands.
|
||
Comment 10•6 years ago
|
||
Pushed by amarchesini@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/39784ddfdbb0 FetchEvent Request.signal should be aborted when the corrisponding nsIChannel is canceled r=bkelly https://hg.mozilla.org/integration/mozilla-inbound/rev/4133f5b222de FetchEvent Request.signal should be aborted when the corrisponding nsIChannel is canceled - test, r=bkelly
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/11686 for changes under testing/web-platform/tests
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
|
||
Comment 13•6 years ago
|
||
Backed out 2 changesets (bug 1394102) for Web platform test failures. CLOSED TREE Log: https://treeherder.mozilla.org/logviewer.html#?job_id=185166383&repo=mozilla-inbound&lineNumber=15382 INFO - TEST-PASS | /fetch/api/abort/serviceworker-intercepted.https.html | Stream errors once aborted. [task 2018-06-27T14:19:44.943Z] 14:19:44 INFO - TEST-UNEXPECTED-TIMEOUT | /fetch/api/abort/serviceworker-intercepted.https.html | FetchEvent.request.signal must be aborted. - Test timed out [task 2018-06-27T14:19:44.943Z] 14:19:44 INFO - TEST-UNEXPECTED-TIMEOUT | /fetch/api/abort/serviceworker-intercepted.https.html | expected OK [task 2018-06-27T14:19:44.943Z] 14:19:44 INFO - TEST-INFO took 31079ms [task 2018-06-27T14:19:44.946Z] 14:19:44 INFO - PID 4509 | --DOCSHELL 0xd8357400 == 15 [pid = 4509] [id = {69ad5305-db72-4fa4-b343-0789440cbdf9}] [task 2018-06-27T14:19:44.946Z] 14:19:44 INFO - PID 4509 | --DOCSHELL 0xd8360400 == 14 [pid = 4509] [id = {78589776-74d8-45fb-adc0-f7e4384cfe4b}] [task 2018-06-27T14:19:44.947Z] 14:19:44 INFO - PID 4509 | --DOCSHELL 0xd835ac00 == 13 [pid = 4509] [id = {b451471e-94fe-4925-9251-4e8b22a0f389}] [task 2018-06-27T14:19:44.947Z] 14:19:44 INFO - PID 4509 | --DOCSHELL 0xdb7b5000 == 12 [pid = 4509] [id = {3e59954c-da30-44e7-9450-14de68210162}] [task 2018-06-27T14:19:44.948Z] 14:19:44 INFO - PID 4509 | --DOCSHELL 0xddd0dc00 == 11 [pid = 4509] [id = {b5aea2ae-7560-4d33-b0b5-f475686d4857}] [task 2018-06-27T14:19:44.948Z] 14:19:44 INFO - PID 4509 | --DOCSHELL 0xe345d000 == 10 [pid = 4509] [id = {8cfd26ae-6078-4ef0-882a-a097b2132bac}] [task 2018-06-27T14:19:44.949Z] 14:19:44 INFO - PID 4509 | --DOCSHELL 0xe56d1800 == 9 [pid = 4509] [id = {36329b71-f72a-4128-bb4a-4cd6985532fa}] [task 2018-06-27T14:19:44.949Z] 14:19:44 INFO - PID 4509 | --DOCSHELL 0xe56d4c00 == 8 [pid = 4509] [id = {bd851f60-394b-4095-b00b-163b8c4aa555}] [task 2018-06-27T14:19:44.998Z] 14:19:44 INFO - PID 4509 | 1530109184995 Marionette INFO Stopped listening on port 2828 [task 2018-06-27T14:19:45.902Z] 14:19:45 INFO - PID 4509 | --DOCSHELL 0xe45f1400 == 7 [pid = 4509] [id = {be4d14fb-2fd4-4b3f-9962-34f14324c8dc}] [task 2018-06-27T14:19:45.903Z] 14:19:45 INFO - PID 4509 | --DOCSHELL 0xe4649400 == 6 [pid = 4509] [id = {be8896cc-2eb2-4b60-8234-baf567dcdf57}] [task 2018-06-27T14:19:45.904Z] 14:19:45 INFO - PID 4509 | --DOCSHELL 0xdeae1000 == 5 [pid = 4509] [id = {369573e4-1025-4d2a-849f-1a4c7467ec78}] [task 2018-06-27T14:19:45.904Z] 14:19:45 INFO - PID 4509 | --DOCSHELL 0xddd11000 == 4 [pid = 4509] [id = {f1a1a55f-4137-4d00-ae30-74d74509dcb0}] [task 2018-06-27T14:19:46.021Z] 14:19:46 INFO - PID 4509 | --DOCSHELL 0xcf498c00 == 3 [pid = 4509] [id = {3fbe13cf-4958-4bc5-90ee-bd35aa03cbcd}] [task 2018-06-27T14:19:46.022Z] 14:19:46 INFO - PID 4509 | --DOCSHELL 0xceef4c00 == 2 [pid = 4509] [id = {855b82f8-d2f5-48d5-ada2-7fda18741cd7}] [task 2018-06-27T14:19:46.147Z] 14:19:46 INFO - PID 4509 | [4509, Main Thread] WARNING: NS_ENSURE_TRUE(mDB) failed: file /builds/worker/workspace/build/src/netwerk/cache/nsDiskCacheDeviceSQL.cpp, line 1421 [task 2018-06-27T14:19:46.458Z] 14:19:46 INFO - PID 4509 | [4509, IPDL Background] WARNING: 'aRv.Failed()', file /builds/worker/workspace/build/src/dom/cache/CacheOpParent.cpp, line 171 [task 2018-06-27T14:19:46.458Z] 14:19:46 INFO - PID 4509 | [4509, Main Thread] WARNING: 'aRv.Failed()', file /builds/worker/workspace/build/src/dom/cache/CacheOpChild.cpp, line 114 [task 2018-06-27T14:19:46.459Z] 14:19:46 INFO - PID 4509 | [4509, Main Thread] WARNING: NS_ENSURE_TRUE(maybeContext) failed: file /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp, line 794 [task 2018-06-27T14:19:46.459Z] 14:19:46 INFO - PID 4509 | [4509, Main Thread] WARNING: NS_ENSURE_TRUE(maybeContext) failed: file /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp, line 794 [task 2018-06-27T14:19:47.472Z] 14:19:47 INFO - PID 4509 | --DOCSHELL 0xdeae1400 == 1 [pid = 4509] [id = {64a063d9-55e9-4bda-acab-1b4d447dfb7a}] [task 2018-06-27T14:19:47.473Z] 14:19:47 INFO - PID 4509 | --DOCSHELL 0xd9ab4400 == 0 [pid = 4509] [id = {ee926a3b-d123-4402-a534-e5934e80f669}] [task 2018-06-27T14:19:48.342Z] 14:19:48 INFO - PID 4509 | --DOMWINDOW == 31 (0xe45b1200) [pid = 4509] [serial = 4] [outer = (nil)] [url = about:blank] [task 2018-06-27T14:19:48.358Z] 14:19:48 INFO - PID 4509 | --DOMWINDOW == 30 (0xf7149250) [pid = 4509] [serial = 3] [outer = (nil)] [url = chrome://browser/content/browser.xul] [task 2018-06-27T14:19:48.360Z] 14:19:48 INFO - PID 4509 | --DOMWINDOW == 29 (0xd9a67080) [pid = 4509] [serial = 12] [outer = (nil)] [url = chrome://extensions/content/dummy.xul] [task 2018-06-27T14:19:48.360Z] 14:19:48 INFO - PID 4509 | --DOMWINDOW == 28 (0xdadf6400) [pid = 4509] [serial = 15] [outer = (nil)] [url = chrome://extensions/content/dummy.xul] [task 2018-06-27T14:19:48.362Z] 14:19:48 INFO - PID 4509 | --DOMWINDOW == 27 (0xd9a671b0) [pid = 4509] [serial = 16] [outer = (nil)] [url = moz-extension://7ca2f068-3fd6-4e91-98bc-75a94bad52cb/_generated_background_page.html] [task 2018-06-27T14:19:48.363Z] 14:19:48 INFO - PID 4509 | [4509, Main Thread] WARNING: '!mMainThread', file /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 517 [task 2018-06-27T14:19:48.365Z] 14:19:48 INFO - PID 4509 | [4509, Main Thread] WARNING: '!mMainThread', file /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 517 [task 2018-06-27T14:19:48.365Z] 14:19:48 INFO - PID 4509 | [4509, Main Thread] WARNING: '!mMainThread', file /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 517 [task 2018-06-27T14:19:48.366Z] 14:19:48 INFO - PID 4509 | [4509, Main Thread] WARNING: '!mMainThread', file /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 517 [task 2018-06-27T14:19:48.367Z] 14:19:48 INFO - PID 4509 | [4509, Main Thread] WARNING: '!mMainThread', file /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 517 [task 2018-06-27T14:19:48.368Z] 14:19:48 INFO - PID 4509 | [4509, Main Thread] WARNING: '!mMainThread', file /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 517 [task 2018-06-27T14:19:48.368Z] 14:19:48 INFO - PID 4509 | [4509, Main Thread] WARNING: '!mMainThread', file /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 517 [task 2018-06-27T14:19:48.369Z] 14:19:48 INFO - PID 4509 | [4509, Main Thread] WARNING: '!mMainThread', file /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 517 [task 2018-06-27T14:19:48.371Z] 14:19:48 INFO - PID 4509 | [4509, Main Thread] WARNING: '!mMainThread', file /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 517 [task 2018-06-27T14:19:48.372Z] 14:19:48 INFO - PID 4509 | --DOMWINDOW == 26 (0xf7148c60) [pid = 4509] [serial = 1] [outer = (nil)] [url = resource://gre-resources/hiddenWindow.html] [task 2018-06-27T14:19:48.372Z] 14:19:48 INFO - PID 4509 | --DOMWINDOW == 25 (0xd9a67ec0) [pid = 4509] [serial = 24] [outer = (nil)] [url = about:newtab] [task 2018-06-27T14:19:48.374Z] 14:19:48 INFO - PID 4509 | [4509, Main Thread] WARNING: '!mMainThread', file /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 517 [task 2018-06-27T14:19:48.375Z] 14:19:48 INFO - PID 4509 | [4509, Main Thread] WARNING: '!mMainThread', file /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 517 [task 2018-06-27T14:19:48.376Z] 14:19:48 INFO - PID 4509 | [4509, Main Thread] WARNING: '!mMainThread', file /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 517 [task 2018-06-27T14:19:48.377Z] 14:19:48 INFO - PID 4509 | [4509, Main Thread] WARNING: '!mMainThread', file /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 517 [task 2018-06-27T14:19:48.378Z] 14:19:48 INFO - PID 4509 | [4509, Main Thread] WARNING: '!mMainThread', file /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 517 [task 2018-06-27T14:19:48.379Z] 14:19:48 INFO - PID 4509 | [4509, Main Thread] WARNING: '!mMainThread', file /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 517 [task 2018-06-27T14:19:48.380Z] 14:19:48 INFO - PID 4509 | [4509, Main Thread] WARNING: '!mMainThread', file /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 517 [task 2018-06-27T14:19:48.381Z] 14:19:48 INFO - PID 4509 | [4509, Main Thread] WARNING: '!mMainThread', file /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 517 [task 2018-06-27T14:19:48.382Z] 14:19:48 INFO - PID 4509 | [4509, Main Thread] WARNING: '!mMainThread', file /builds/worker/workspace/build/src/xpcom/threads/nsThreadManager.cpp, line 517 [task 2018-06-27T14:19:48.383Z] 14:19:48 INFO - PID 4509 | --DOMWINDOW == 24 (0xf7149aa0) [pid = 4509] [serial = 6] [outer = (nil)] [url = about:blank] [task 2018-06-27T14:19:48.385Z] 14:19:48 INFO - PID 4509 | --DOMWINDOW == 23 (0xdb99f600) [pid = 4509] [serial = 9] [outer = (nil)] [url = about:blank] [task 2018-06-27T14:19:48.385Z] 14:19:48 INFO - PID 4509 | --DOMWINDOW == 22 (0xe45b3000) [pid = 4509] [serial = 5] [outer = (nil)] [url = resource://gre-resources/hiddenWindow.html] [task 2018-06-27T14:19:48.386Z] 14:19:48 INFO - PID 4509 | --DOMWINDOW == 21 (0xcf4ef000) [pid = 4509] [serial = 26] [outer = (nil)] [url = about:newtab] [task 2018-06-27T14:19:48.387Z] 14:19:48 INFO - PID 4509 | --DOMWINDOW == 20 (0xdadf7800) [pid = 4509] [serial = 18] [outer = (nil)] [url = moz-extension://7ca2f068-3fd6-4e91-98bc-75a94bad52cb/_generated_background_page.html] [task 2018-06-27T14:19:48.464Z] 14:19:48 INFO - PID 4509 | WARNING: YOU ARE LEAKING THE WORLD (at least one JSRuntime and everything alive inside it, that is) AT JS_ShutDown TIME. FIX THIS! [task 2018-06-27T14:19:48.464Z] 14:19:48 INFO - PID 4509 | Hit MOZ_CRASH(NSS_Shutdown failed) at /builds/worker/workspace/build/src/xpcom/build/XPCOMInit.cpp:1048 [task 2018-06-27T14:19:48.581Z] 14:19:48 INFO - PID 4509 | #01: mozilla::ShutdownXPCOM(nsIServiceManager*) (/builds/worker/workspace/build/application/firefox/libxul.so) [task 2018-06-27T14:19:48.582Z] 14:19:48 INFO - PID 4509 | #02: ScopedXPCOMStartup::~ScopedXPCOMStartup() (/builds/worker/workspace/build/application/firefox/libxul.so) [task 2018-06-27T14:19:48.586Z] 14:19:48 INFO - PID 4509 | #03: mozilla::DefaultDelete<ScopedXPCOMStartup>::operator()(ScopedXPCOMStartup*) const [clone .isra.82] (/builds/worker/workspace/build/application/firefox/libxul.so) [task 2018-06-27T14:19:48.594Z] 14:19:48 INFO - PID 4509 | #04: XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) (/builds/worker/workspace/build/application/firefox/libxul.so) [task 2018-06-27T14:19:48.597Z] 14:19:48 INFO - PID 4509 | #05: XRE_main(int, char**, mozilla::BootstrapConfig const&) (/builds/worker/workspace/build/application/firefox/libxul.so) [task 2018-06-27T14:19:48.674Z] 14:19:48 INFO - PID 4509 | #06: do_main(int, char**, char**) (/builds/worker/workspace/build/application/firefox/firefox) [task 2018-06-27T14:19:48.675Z] 14:19:48 INFO - PID 4509 | #07: main (/builds/worker/workspace/build/application/firefox/firefox) [task 2018-06-27T14:19:48.675Z] 14:19:48 INFO - PID 4509 | ExceptionHandler::GenerateDump cloned child 4778 [task 2018-06-27T14:19:48.675Z] 14:19:48 INFO - PID 4509 | ExceptionHandler::SendContinueSignalToChild sent continue signal to child [task 2018-06-27T14:19:48.675Z] 14:19:48 INFO - PID 4509 | ExceptionHandler::WaitForContinueSignal waiting for continue signal... [task 2018-06-27T14:19:48.704Z] 14:19:48 INFO - Browser exited with return code 11 [task 2018-06-27T14:19:48.705Z] 14:19:48 WARNING - u'runner_teardown': () [task 2018-06-27T14:19:48.721Z] 14:19:48 INFO - Setting up ssl [task 2018-06-27T14:19:48.758Z] 14:19:48 INFO - certutil | [task 2018-06-27T14:19:48.800Z] 14:19:48 INFO - certutil | [task 2018-06-27T14:19:48.820Z] 14:19:48 INFO - certutil | [task 2018-06-27T14:19:48.820Z] 14:19:48 INFO - Certificate Nickname Trust Attributes [task 2018-06-27T14:19:48.821Z] 14:19:48 INFO - SSL,S/MIME,JAR/XPI [task 2018-06-27T14:19:48.821Z] 14:19:48 INFO - [task 2018-06-27T14:19:48.821Z] 14:19:48 INFO - web-platform-tests CT,, [task 2018-06-27T14:19:48.821Z] 14:19:48 INFO - [task 2018-06-27T14:19:48.845Z] 14:19:48 INFO - Application command: /builds/worker/workspace/build/application/firefox/firefox --marionette about:blank -profile /tmp/tmpnkA_Sl.mozrunner [task 2018-06-27T14:19:48.862Z] 14:19:48 INFO - Starting runner Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=4133f5b222de3cf0a7234331df4e47b1e90158a8 Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/8359b26b06efec25ec18db489fabebf7215accdf
Flags: needinfo?(amarchesini)
Upstream PR was closed without merging
|
|
Reporter | |
Updated•6 years ago
|
Flags: needinfo?(amarchesini)
|
|
||
Comment 15•6 years ago
|
||
Pushed by amarchesini@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/834072e2974e FetchEvent Request.signal should be aborted when the corrisponding nsIChannel is canceled r=bkelly https://hg.mozilla.org/integration/mozilla-inbound/rev/63b21854d800 FetchEvent Request.signal should be aborted when the corrisponding nsIChannel is canceled - test, r=bkelly
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
|
||
Comment 17•6 years ago
|
||
Backed out 2 changesets (bug 1394102) for wpt failures in /fetch/api/abort/serviceworker-intercepted.https.html Failure push: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=63b21854d8006576e83847cff297cc975ecb8b71&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable&selectedJob=185325623 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=185325623&repo=mozilla-inbound&lineNumber=15247 Backout: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=4dda7e9173675d0eb16bcdbfe0bdd8c9a89a7c19&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Flags: needinfo?(amarchesini)
Upstream PR was closed without merging
|
|
Reporter | |
Updated•6 years ago
|
Flags: needinfo?(amarchesini)
|
|
||
Comment 19•6 years ago
|
||
Pushed by amarchesini@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/afbadca69031 FetchEvent Request.signal should be aborted when the corrisponding nsIChannel is canceled r=bkelly https://hg.mozilla.org/integration/mozilla-inbound/rev/bad873c0dbbe FetchEvent Request.signal should be aborted when the corrisponding nsIChannel is canceled - test, r=bkelly
|
|
||
Comment 20•6 years ago
|
||
Backout by cbrindusan@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/9115a2685cb0 Backed out 2 changesets for wpt failures on sw.https.window.html. CLOSED TREE
|
|
||
Comment 21•6 years ago
|
||
Andrea, please do a try build before landing this again.
|
||
Comment 22•6 years ago
|
||
Backed out 2 changesets (bug 1394102) for wpt failures on sw.https.window.html. Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/9115a2685cb0ceb92a8eb5bff7b4a6a00af35bbb Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=bad873c0dbbe1c34582ba3ceefd58c7be7eecd8e&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable&selectedJob=185388193 Failure log: [task 2018-06-28T15:16:19.549Z] 15:16:19 INFO - TEST-START | /fetch/range/general.window.html [task 2018-06-28T15:16:19.630Z] 15:16:19 INFO - PID 7232 | 1530198979625 Marionette DEBUG [2147483662] Frame script loaded [task 2018-06-28T15:16:19.632Z] 15:16:19 INFO - PID 7232 | 1530198979630 Marionette DEBUG [2147483662] Frame script registered [task 2018-06-28T15:16:19.815Z] 15:16:19 INFO - .. [task 2018-06-28T15:16:19.815Z] 15:16:19 INFO - TEST-OK | /fetch/range/general.window.html | took 266ms [task 2018-06-28T15:16:19.816Z] 15:16:19 INFO - TEST-START | /fetch/range/sw.https.window.html [task 2018-06-28T15:16:19.862Z] 15:16:19 INFO - PID 7232 | 1530198979852 Marionette DEBUG [2147483649] Received DOM event beforeunload for http://web-platform.test:8000/testharness_runner.html [task 2018-06-28T15:16:19.919Z] 15:16:19 INFO - PID 7232 | 1530198979911 Marionette DEBUG [2147483649] Received DOM event pagehide for http://web-platform.test:8000/testharness_runner.html [task 2018-06-28T15:16:19.920Z] 15:16:19 INFO - PID 7232 | 1530198979912 Marionette DEBUG [2147483649] Received DOM event unload for http://web-platform.test:8000/testharness_runner.html [task 2018-06-28T15:16:19.927Z] 15:16:19 INFO - PID 7232 | 1530198979921 Marionette DEBUG [2147483649] Received DOM event DOMContentLoaded for https://web-platform.test:8443/testharness_runner.html [task 2018-06-28T15:16:19.943Z] 15:16:19 INFO - PID 7232 | 1530198979936 Marionette DEBUG [2147483649] Received DOM event pageshow for https://web-platform.test:8443/testharness_runner.html [task 2018-06-28T15:16:20.004Z] 15:16:20 INFO - PID 7232 | 1530198980001 Marionette DEBUG [2147483666] Frame script loaded [task 2018-06-28T15:16:20.012Z] 15:16:20 INFO - PID 7232 | 1530198980008 Marionette DEBUG [2147483666] Frame script registered [task 2018-06-28T15:16:20.738Z] 15:16:20 INFO - [task 2018-06-28T15:16:20.738Z] 15:16:20 INFO - TEST-PASS | /fetch/range/sw.https.window.html | Defer range header filter tests to service worker [task 2018-06-28T15:16:20.738Z] 15:16:20 INFO - TEST-PASS | /fetch/range/sw.https.window.html | Defer range header passthrough tests to service worker [task 2018-06-28T15:16:20.738Z] 15:16:20 INFO - TEST-UNEXPECTED-PASS | /fetch/range/sw.https.window.html | Ranged response not allowed following no-cors ranged request - expected FAIL
Flags: needinfo?(amarchesini)
|
|
Reporter | |
Comment 23•6 years ago
|
||
(In reply to Ben Kelly [:bkelly] from comment #21) > Andrea, please do a try build before landing this again. This was the last one before the push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3e4dc8410ddbae6106afb1143c4a7757e6a2fbca Before I did all of these: https://treeherder.mozilla.org/#/jobs?repo=try&revision=497ae6b74005c7371a086256e11a5b7b0152e6d9 https://treeherder.mozilla.org/#/jobs?repo=try&revision=03cba329bf7322a5495338d2427fa0996db32fea https://treeherder.mozilla.org/#/jobs?repo=try&revision=4502cbb4056ba09e78155493e3186290c48e7fc4 plus all of these: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e10f49f96e0af0bee0f15586da53d9b36aca5790 https://treeherder.mozilla.org/#/jobs?repo=try&revision=6b8b621e8a42abd1361b4621fdf03e4571596ca0 https://treeherder.mozilla.org/#/jobs?repo=try&revision=27ab97f40545d627765f23aca2b346e76ed59353 https://treeherder.mozilla.org/#/jobs?repo=try&revision=45951895e2c98b1f85a9d4de71919855c2203e7e https://treeherder.mozilla.org/#/jobs?repo=try&revision=497ae6b74005c7371a086256e11a5b7b0152e6d9 https://treeherder.mozilla.org/#/jobs?repo=try&revision=03cba329bf7322a5495338d2427fa0996db32fea I hope there were enough.
Flags: needinfo?(amarchesini)
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
Upstream PR was closed without merging
|
Assignee | |
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
|
||
Updated•5 years ago
|
Component: DOM: Core & HTML → DOM: Service Workers
|
|
||
Updated•5 years ago
|
Assignee: amarchesini → nobody
|
||
Updated•2 years ago
|
Severity: normal → S3
Wait, the last backout in comment #22 happened because of TEST-UNEXPECTED-PASS? That should have been easy to fix...
|
||
Comment 28•2 years ago
•
|
||
:saschanaz, you are right. As this fix is required for my bug Bug 1394102, I would like to see if this still works on latest load. I can rebase the patch containing the fix and WPT on top of central and push it on try server.
Please let me know if you had already tried this?
Flags: needinfo?(krosylight)
You mean bug 1793736. It seems the corresponding files are heavily refactored since then and thus rebasing doesn't seem straightforward. If you can see how it should be rebased then please do 👍
Blocks: 1793736
Flags: needinfo?(krosylight)
|
|
||
Comment 30•2 years ago
|
||
(In reply to Kagami :saschanaz from comment #29)
You mean bug 1793736. It seems the corresponding files are heavily refactored since then and thus rebasing doesn't seem straightforward. If you can see how it should be rebased then please do 👍
Thank you. As you have pointed out, rebasing is not straightforward. I will reach out to you in case I need help.
|
|
||
Comment 31•2 years ago
|
||
See Also: → 1568422
|
||
Updated•1 year ago
|
Attachment #9303677 -
Attachment is obsolete: true
|
||
Comment 32•3 days ago
|
||
This issue has not been updated in over 2 years, I assume work towards fixing this was stalled?
https://github.com/w3c/ServiceWorker/issues/1544#issuecomment-2099263362
When a ServiceWorker is handling the "fetch" event from a tab, and the tab is closed, the fetch request.signal is not aborted.
This makes it difficult to abort requests in the ServiceWorker once a client goes away.
As far as I can tell the only workaround is to check the clientId on the request and poll self.clients.get() to check if the client is still present until it either goes away or the request is completed.
You need to log in
before you can comment on or make changes to this bug.
Description
•