Open Bug 1394102 Opened 7 years ago Updated 9 months ago

FetchEvent.request.signal should be correctly aborted

Categories

(Core :: DOM: Service Workers, enhancement, P2)

Product:
Core
Component:
DOM: Service Workers
Type:
enhancement

Tracking

()

People

(Reporter: baku, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 3 obsolete files)

test
2.52 KB, patch
bkelly
: review+
Details | Diff | Splinter Review
network_2.patch
11.74 KB, patch
bkelly
: review+
Details | Diff | Splinter Review
This is a follow up of bug 1378342 comment 31.
Blocks: ServiceWorkers-compat
Summary: FetchEvent.signal should be correctly aborted → FetchEvent.request.signal should be correctly aborted
Basically we need to forward any cancellation of the outer intercepted channel to FetchEvent.request.signal.
Jake says he would be happy for us to add a WPT test for this and upstream through our gecko sync mechanism.
Priority: -- → P2
Depends on: 1395140
Attached patch network_2.patch (obsolete) — — Splinter Review
Assignee: nobody → amarchesini
Attachment #8902677 - Flags: review?(bkelly)
Attached patch network_2.patch (obsolete) — — Splinter Review
Attachment #8902677 - Attachment is obsolete: true
Attachment #8902677 - Flags: review?(bkelly)
Attachment #8902680 - Flags: review?(bkelly)
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+
Depends on: 1395837
Attached patch test — — Splinter Review
Here a test. I'll apply the comment on the previous patch for today.
Attachment #8903478 - Flags: review?(bkelly)
Attachment #8903478 - Flags: review?(bkelly) → review+
Attached patch network_2.patch — — Splinter Review
Attachment #8902680 - Attachment is obsolete: true
Attachment #8903592 - Flags: review?(bkelly)
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+
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.
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.
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
Flags: needinfo?(amarchesini)
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.
Upstream PR was closed without merging
Flags: needinfo?(amarchesini)
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
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
Andrea, please do a try build before landing this again.
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)
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
Upstream PR was closed without merging
Component: DOM → DOM: Core & HTML
Component: DOM: Core & HTML → DOM: Service Workers
Assignee: amarchesini → nobody
Severity: normal → S3

Wait, the last backout in comment #22 happened because of TEST-UNEXPECTED-PASS? That should have been easy to fix...

: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)

(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.

Attached file WIP: Port fix for Bug 1394102 (obsolete) —
Attachment #9303677 - Attachment is obsolete: true

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.

Attachment

General

Creator:
Created:
Updated:
Size: