A big part of removing nsIDocument from SWM is replacing the stored nsIDocument references for controlled clients. Instead we will store ClientHandle references and use a new API to know when the ClientHandle is destroyed.

Not done here, but less see if the ClientHandle::OnDetach() promise causes anything to blow up: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3ff0eafb58d35e812711b81ad9abb5829293d495

Making good progress on this, but need to explore the best solution for a few problems: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f6403c304d8ea3518bee1b1631c602e1f06c95c3

Comment on attachment 8938113 [details] [diff] [review] P1 Add ClientHandle::OnDetach() which returns a MozPromise that resolves on actor destruction. r=asuth While I'm still finalizing the final patches in the queue, I'm pretty confident with the first few patches. So I'm going to flag now. This patch exposes a ClientHandle::OnDetach() method that returns a MozPromise that resolves when the handle shutdowns due to de-ref or IPC actor destruction.

Comment on attachment 8937904 [details] [diff] [review] P2 Add ServiceWorkerManager mControlledClients to track controlled ClientHandle references. r=asuth This patch adds a separate hashtable to store ClientHandle/ServiceWorkerRegistrationInfo pairs. Its keyed by the ClientInfo identifier. The goal is to ultimately replace all uses of mControlledDocuments with this new mControlledClients. Initially there will be some overlap.

Comment on attachment 8938032 [details] [diff] [review] P3 Refactor ServiceWorkerManager::GetDocumentRegistration() to GetClientRegistration(). r=asuth This patch refactors GetDocumentRegistration() to GetClientRegistration() replacing nsIDocument code with client-based code.

Comment on attachment 8938033 [details] [diff] [review] P4 Make ServiceWorkerManager::UpdateClientControllers use mControlledClients. r=asuth Refactor UpdateClientControllers to use mControlledClients instead of mControlledDocuments.

Comment on attachment 8938034 [details] [diff] [review] P5 Make ServiceWorkerManager::RemoveRegistration assert there is no controlled document. r=asuth Strengthen the assertion in RemoveScopeAndRegistration() that touches mControlledDocuments. This is in preparation of later removing it.

Comment on attachment 8938225 [details] [diff] [review] P6 Rename some service worker methods to not reference documents. r=asuth This patch just renames some methods and members that had the word "Document" in them. Instead they will now say "Client".

Comment on attachment 8938227 [details] [diff] [review] P8 Fix unregister-then-register-new-script.https.html to not race iframe.remove() and expect resurrection on failed scripts. r=asuth This fixes a bad WPT test. It was incorrectly trying to assert that registering a 404 or reject-on-install worker would not resurrect an uninstalling registration. There is no such spec language, AFAICT. See step 5.1 here: https://w3c.github.io/ServiceWorker/#register-algorithm It clears the flag before running the update algorithm with does the fetching and install event dispatch. The reason this mostly passed before was because it called frame.remove() on a controlled frame at the same time it did the register(). So it raced the register against noticing that the frame was removed and letting the old SW go away. With our new IPC-based code we begin losing this race.

I'm going to wait to flag for P7 until I can split up my final work-in-progress patch. I think some of it could be reasonably merged with it.

With the work-in-progress patch and this the only remaining uses of mControlledDocuments is for logging.

Comment on attachment 8938237 [details] [diff] [review] P9 Refactor MaybeCheckNavigationUpdate() to take a ClientInfo instead of a document. r=asuth This is a simple refactor from nsIDocument to ClientInfo using mControlledClients.

Andrew, this patch fixes a "wait for active worker" bug in test_skip_waiting.html. It was trying to use .ready in the window being opened for that, but was not waiting for an active worker before opening the window. So it never got controlled in the first place.

Comment on attachment 8938113 [details] [diff] [review] P1 Add ClientHandle::OnDetach() which returns a MozPromise that resolves on actor destruction. r=asuth Review of attachment 8938113 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/clients/manager/ClientHandle.h @@ +94,5 @@ > RefPtr<GenericPromise> > PostMessage(ipc::StructuredCloneData& aData, > const ServiceWorkerDescriptor& aSource); > > + // Return a Promise the resolves when the ClientHandle object is detached phrasing nit: s/the resolves/that resolves/

Here is another fun mochitest fix. This test_workerupdatefound.html test was doing something very weird: 1. It registered a service worker, but did not wait for activation. 2. The service worker blocked its activate event on an updatefound event and required a controlled frame to present. 3. Opened a frame that it expected in (2). In order for this to work it required some precise timing to line up. The initial updatefound event needed to fire during the activation event handler, which is not guaranteed. The activation event also needed to fire before the frame loaded, which was not guaranteed. The fix here is to move the updatefound handler out of the activate event handler. Also, we make sure the worker is activate before creating the frame.

The json viewer failures are interesting. Apparently the json viewer ends up with a null principal and we shouldn't allow storage. But I think we want to be able to view json coming out of a service worker. I guess I could allow the initial fetch event, but then revoke the controller after the fact. I'll investigate that this afternoon. https://treeherder.mozilla.org/#/jobs?repo=try&revision=f23bef69da3b4903ce468d46718c117fb177ce99

Comment on attachment 8937904 [details] [diff] [review] P2 Add ServiceWorkerManager mControlledClients to track controlled ClientHandle references. r=asuth Review of attachment 8937904 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/workers/ServiceWorkerManager.cpp @@ +331,5 @@ > + > + RefPtr<ServiceWorkerManager> self(this); > + clientHandle->OnDetach()->Then( > + SystemGroup::EventTargetFor(TaskCategory::Other), __func__, > + [self = Move(self), aClientInfo] { restating for my own sanity: aClientInfo is captured by-copy, the fact that the argument aClientInfo is a reference has no bearing on this.

Comment on attachment 8938034 [details] [diff] [review] P5 Make ServiceWorkerManager::RemoveRegistration assert there is no controlled document. r=asuth Review of attachment 8938034 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/workers/ServiceWorkerManager.cpp @@ +2308,5 @@ > entry.Data()->Cancel(); > entry.Remove(); > } > > + // Verify there are no controlled documents for the purged registration. Can you add a comment explaining why we think there won't be any controlled documents for the purged registration?

Comment on attachment 8938225 [details] [diff] [review] P6 Rename some service worker methods to not reference documents. r=asuth Review of attachment 8938225 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/workers/ServiceWorkerRegistrationInfo.cpp @@ +98,1 @@ > NS_WARNING("ServiceWorkerRegistrationInfo is still controlling documents. This can be a bug or a leak in ServiceWorker API or in any other API that takes the document alive."); nit: "still controlling documents". No need to address here, but could be confusing later on when the workers thing happens. Ignore if you correct later in the stack.

Comment on attachment 8938501 [details] [diff] [review] P7 Use the mControlledClients list to drive controller start and stop logic. r=asuth Ok, I ended up folding a lot of my "hmm" patch into P7 since it made sense here. This patch moves the lifecycle management of registrations over into the mControlledClients world. This is mainly significant in that we need to know when all of the controlled clients are gone so we can move workers to redundant and activate the next worker. As part of doing this I exposed a new nsIServiceWorkerManager.StartControlling() method that takes ClientInfo and ServiceWorkerDescriptor. This is called by docshell when it controls an initial about:blank ClientSource and ensures we track these clients as ClientHandle objects. We also get ClientHandle objects into mControlledClients via ServiceWorkerManager::DispatchFetchEvent(). This is the same point where we mark the LoadInfo as controlled. In the future the docshell StartControlling() call will have to become IPC enabled. The internal DispatchFetchEvent() call to StartControllingClient() will in theory happen on the parent process already. I also fixed up the "assert no more controlled document/clients" assertions a bit. I incorrectly have an #ifdef around them that prevented the iter.Remove() from firing in release. Also, we only assert the mControlledClients case now since that is what the lifecycle is based on. Its possible documents may outlive the ClientHandle::OnDetach() promise in some race conditions.

Comment on attachment 8938502 [details] [diff] [review] P12 Don't mark an initial about:blank client as controlled if its sandboxed. r=asuth You may have noticed the P7 patch removed an assertion that we don't control documents that don't have storage access. P14 will add that assertion back in ClientSource, but first we need to fix some cases that make the new ClientSource assertions fail. The first is that we should not mark initial about:blank clients as controlled if they are sandboxed. While the nsDocShell::MaybeCreateInitialClientSource() checks for sandbox in the "no principal" case, we still need to check it again later when we have a principal and a controller. Also, this changes MaybeCreateInitialClientSource() to use the more generous check for any sandbox flags. This matches ShouldPrepareForIntercept().

Comment on attachment 8938508 [details] [diff] [review] P13 Check for a different final document principal and reset the ClientSource when it happens. r=asuth Another case that made us fail the storage access assertions in ClientSource is when the final document principal was changed to the null principal when viewing json. This happens because the streamconv for json uses nsIChannel.owner to override the principal. The ClientChannelHelper works hard to detect cross-origin network requests and redirects, but it has no way to tell when nsIChannel.owner or similar overrides are set. To address the problem this patch double-checks the final document principal against the ClientSource. If they don't match then it resets the ClientSource. In these cases its also important not to mark the new ClientSource controlled even though the nsIChannel may have been intercepted. From the service worker's perspective its like there was a client there briefly, but it quickly navigated to a new origin.

Comment on attachment 8938504 [details] [diff] [review] P14 Assert that storage is allowed when a ClientSource is both execution ready and controlled. r=asuth Finally, add assertions in ClientSource that we never have a Client that is both execution ready and controlled. The execution ready requirement is simply there to ensure we have enough state to determine our storage policy correctly.

Comment on attachment 8938377 [details] [diff] [review] P10 Fix the test_skip_waiting.html mochitest to properly wait for active worker state. r=asuth Review of attachment 8938377 [details] [diff] [review]: ----------------------------------------------------------------- Restating: waitForActivated() was actually waiting for the "skip_waiting_scope/index.html"'s ready promise which would be fulfilled when start()'s registration of worker.js reached "active", but would not claim existing clients, so a race was possible where the iframe would never be controlled if the iframe's navigation event was serviced prior to the SW reaching activation. This would manifest as a failure of checkWhetherItSkippedWaiting because the iframe could not experience a controllerchange if not controlled. So now we wait for activation. We also leave around the READY-postMessaging code, because maybe some other code uses it.

Comment on attachment 8938397 [details] [diff] [review] P11 Fix test_workerupdatefound.html not to frame loading against SW activation and updatefound events. r=asuth Review of attachment 8938397 [details] [diff] [review]: ----------------------------------------------------------------- I assume the intent was to wrap the async updatefound handling in an ExtendableEvent for maximum correctness.

Comment on attachment 8938508 [details] [diff] [review] P13 Check for a different final document principal and reset the ClientSource when it happens. r=asuth Review of attachment 8938508 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for the excellent comments! ::: dom/base/nsGlobalWindowInner.cpp @@ +1839,5 @@ > + // marked the LoadInfo controlled and it won't know about them. Its > + // also possible we are creating the client late due to the final > + // principal changing and these clients should definitely not be > + // controlled by a service worker with a different principal. > + else if (loadInfo) { style/readability expectation violation: move the preceding } down to this 'else if' line.

(In reply to Andrew Sutherland [:asuth] from comment #43) > ::: dom/base/nsGlobalWindowInner.cpp > @@ +1839,5 @@ > > + // marked the LoadInfo controlled and it won't know about them. Its > > + // also possible we are creating the client late due to the final > > + // principal changing and these clients should definitely not be > > + // controlled by a service worker with a different principal. > > + else if (loadInfo) { > > style/readability expectation violation: move the preceding } down to this > 'else if' line. Actually, I've gotten conflicting review feedback on this in the past. I used to do what you recommend, but people complained the comment was "in the last block". Putting the else on a separate line when i want to use a block comment for it seemed the best compromise to me.

(In reply to Andrew Sutherland [:asuth] from comment #42) > I assume the intent was to wrap the async updatefound handling in an > ExtendableEvent for maximum correctness. Actually, the ergonomics of `updatefound` in a service worker context are quite poor. I filed a spec issue: https://github.com/w3c/ServiceWorker/issues/1255 For now I intend to leave the test as is.

(In reply to Andrew Sutherland [:asuth] from comment #30) > ::: dom/workers/ServiceWorkerManager.cpp > @@ +2308,5 @@ > > entry.Data()->Cancel(); > > entry.Remove(); > > } > > > > + // Verify there are no controlled documents for the purged registration. > > Can you add a comment explaining why we think there won't be any controlled > documents for the purged registration? I think I added a comment somewhat describing this in P7.

(In reply to Andrew Sutherland [:asuth] from comment #31) > ::: dom/workers/ServiceWorkerRegistrationInfo.cpp > @@ +98,1 @@ > > NS_WARNING("ServiceWorkerRegistrationInfo is still controlling documents. This can be a bug or a leak in ServiceWorker API or in any other API that takes the document alive."); > > nit: "still controlling documents". No need to address here, but could be > confusing later on when the workers thing happens. Ignore if you correct > later in the stack. I'm actually just going to change this to a diagnostic assertion if I can.

Comment on attachment 8938508 [details] [diff] [review] P13 Check for a different final document principal and reset the ClientSource when it happens. r=asuth Andrew gave me verbal r+ for this over IRC. He meant to flip the flag.

One last try since I added another assertion: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c00fbb4c408b50198fb5e300db3b28f2a12077ec

I need to fix test_workerupdatefound.html on non-e10s it appears.

(In reply to Ben Kelly [:bkelly] from comment #53) > I need to fix test_workerupdatefound.html on non-e10s it appears. I can't reproduce this locally and it does not trigger frequently in my try build. I'm going to leave it as is for now and file a follow-up. See bug 1426905.

Pushed by bkelly@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/b7ae11b5bce8 P1 Add ClientHandle::OnDetach() which returns a MozPromise that resolves on actor destruction. r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/acf4d61babab P2 Add ServiceWorkerManager mControlledClients to track controlled ClientHandle references. r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/553628a56e6a P3 Refactor ServiceWorkerManager::GetDocumentRegistration() to GetClientRegistration(). r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/df13eba47970 P4 Make ServiceWorkerManager::UpdateClientControllers use mControlledClients. r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/82d39ed8c831 P5 Make ServiceWorkerManager::RemoveRegistration assert there is no controlled document. r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/f091f5e182c4 P6 Rename some service worker methods to not reference documents. r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/6f520ab76d6a P7 Use the mControlledClients list to drive controller start and stop logic. r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/4ce186c6d8f5 P8 Fix unregister-then-register-new-script.https.html to not race iframe.remove() and expect resurrection on failed scripts. r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/f2b451ce55d4 P9 Refactor MaybeCheckNavigationUpdate() to take a ClientInfo instead of a document. r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/fb89dbd922ba P10 Fix the test_skip_waiting.html mochitest to properly wait for active worker state. r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/0e50d9d1d069 P11 Fix test_workerupdatefound.html not to frame loading against SW activation and updatefound events. r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/9e1544ec814d P12 Don't mark an initial about:blank client as controlled if its sandboxed. r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/1e657fa97b71 P13 Check for a different final document principal and reset the ClientSource when it happens. r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/e6f4a2d1df9a P14 Assert that storage is allowed when a ClientSource is both execution ready and controlled. r=asuth

https://hg.mozilla.org/mozilla-central/rev/b7ae11b5bce8 https://hg.mozilla.org/mozilla-central/rev/acf4d61babab https://hg.mozilla.org/mozilla-central/rev/553628a56e6a https://hg.mozilla.org/mozilla-central/rev/df13eba47970 https://hg.mozilla.org/mozilla-central/rev/82d39ed8c831 https://hg.mozilla.org/mozilla-central/rev/f091f5e182c4 https://hg.mozilla.org/mozilla-central/rev/6f520ab76d6a https://hg.mozilla.org/mozilla-central/rev/4ce186c6d8f5 https://hg.mozilla.org/mozilla-central/rev/f2b451ce55d4 https://hg.mozilla.org/mozilla-central/rev/fb89dbd922ba https://hg.mozilla.org/mozilla-central/rev/0e50d9d1d069 https://hg.mozilla.org/mozilla-central/rev/9e1544ec814d https://hg.mozilla.org/mozilla-central/rev/1e657fa97b71 https://hg.mozilla.org/mozilla-central/rev/e6f4a2d1df9a

Backed out for M(5) permafails mochitest/test_ext_webrequest_filter.html Backout: https://hg.mozilla.org/mozilla-central/rev/2e0db1b48499fcf1598f23e04211360f9d39b7af Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=e6f4a2d1df9a4a50b1b659f87dca99d88a5ef63a (initial push https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=e6f4a2d1df9a4a50b1b659f87dca99d88a5ef63a merged to central) Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=153187458&repo=mozilla-inbound&lineNumber=29413

This was backed out, but I wanted to note it also caused these crashes: http://bit.ly/2l3PXur

Sorry for the fire drill. I think there was an intermittent for this test and it seemed unlikely to be effected by my changes so I didn't notice it perma-oranged. Looking at the test it is clearly not using service workers correctly: https://searchfox.org/mozilla-central/rev/e6fd22c37447579bd544254685c80fd0da221d93/toolkit/components/extensions/test/mochitest/test_ext_webrequest_filter.html#92 It must wait for the worker to become active, etc. I'll have to investigate the assertions some more. Some notes: 1. The stack indicates the is *not* inner window re-use. 2. Its unclear if the initial about:blank ClientSource is being taken from the docshell or not. 3. Its most likely the controller is being set via the FetchEvent interception path 4. This suggests there is a mismatch between what nsDocShell::ShouldPrepareForIntercept() does and the storage assertion 5. Most like this is due to StorageAccessForWindow() returning StorageAccess::eSessionScoped which nsDocShell does not check for. So most likely we need to fix nsDocShell::ShouldPrepareForIntercept() to use nsContentUtils::StorageAllowedForPrincipal() in addition to its third party iframe checks it does today. We can't just use StorageAllowedForWindow() since the window does not exist in the tree yet.

Of course, setting the prefs that would return eSessionScoped does not trigger the crash in that build for me. I'll have to investigate more. It also seems like many of the crashes are from profiles with ad blockers installed.

(In reply to Ben Kelly [:bkelly] from comment #60) > Of course, setting the prefs that would return eSessionScoped does not > trigger the crash in that build for me. I'll have to investigate more. > > It also seems like many of the crashes are from profiles with ad blockers > installed. I think I could not produce this in my previous tests because of bug 1426979. I made this page which has a static iframe: https://fetch-event-echo.glitch.me/index-with-frame.html If I set cookies to expire "when I close Nightly" then it triggers the assertion.

This test triggers the crash.

I've been trying to make nsDocShell use nsContentUtils::StorageAllowed*() methods. Unfortunately thats not really possible at the moment since we can't ensure that nsPermissionManager has all its data for the first docshell in a process. So for now I plan to punt on that improvement. See bug 1428130.

Comment on attachment 8939984 [details] [diff] [review] P17 Make web extension tests wait for service worker to activate to avoid races. r=kmag Review of attachment 8939984 [details] [diff] [review]: ----------------------------------------------------------------- Kris, I'm working on some service worker changes to support multi-e10s and ran into some web extension test failures when I changed the timing of things. It looks like these tests are making a couple of common service worker mistakes: 1. They are not waiting for the registered service worker to become active before proceeding with the test. 2. They are not removing the service worker controlled window when they unregister causing the service worker to stick around longer than we want. This patch fixes these two problems. Note, I do have a minor eslint issue to fix to use double quote strings. Otherwise, though, the try build is green with this patch.

Comment on attachment 8939636 [details] [diff] [review] P15 Add session lifetime policy checks to test_third_party_iframe.html. r=asuth Andrew, this patch updates our test_third_party_iframe.html service worker test to check session cookie policy as well. It triggers the crash which occurred on my first landing here. I did briefly look at removing all the extra pushPrefEnv() here, but it did not seem immediately fixable without more investigation so I punted on that again.

Comment on attachment 8939979 [details] [diff] [review] P16 Make nsDocShell check for session cookie lifetime policy before allowing service worker intercept. r=asuth This patch fixes the crash by making nsDocShell avoid intercepting navigation requests if session cookie policy is in place. I spent a day or two trying to make this code us nsContentUtils::StorageAllowed*(), but ultimately had to keep our lame docshell side implementation. It seems docshell needs to run this code before the cookie permission is in the child process. Once we move interception to the parent process we should be able to switch over to StorageAllowed*() methods. I filed bug 1428130 to follow-up there. I did factor out the checks in docshell into a helper routine, though. This lets us use the same logic in ShouldPrepareToIntercept() and MaybeCreateInitialClientSource(). Note, I did change behavior slightly. Previously we would not intercept subresource requests if the cookie policy was changed to reject after the window was controlled. I tried to do the same with this code, but that caused a test to fail which expected this kind of fetch() to work with session cookie policy. After thinking about it I decided to just make the restrictions based at navigation interception and initial control time. If the policy is changed after controlling a window then that page is still controlled. That way the page doesn't start bypassing the service worker while seeing a controller service worker value.

Comment on attachment 8939984 [details] [diff] [review] P17 Make web extension tests wait for service worker to activate to avoid races. r=kmag Review of attachment 8939984 [details] [diff] [review]: ----------------------------------------------------------------- Thanks. These tests have been pretty dodgy for a while. ::: toolkit/components/extensions/test/mochitest/head.js @@ +67,5 @@ > + if (sw.state !== state) { > + return; > + } > + sw.removeEventListener('statechange', onStateChange); > + resolve(); Nit: `if (sw.state === state) { ... }`

Comment on attachment 8939979 [details] [diff] [review] P16 Make nsDocShell check for session cookie lifetime policy before allowing service worker intercept. r=asuth Review of attachment 8939979 [details] [diff] [review]: ----------------------------------------------------------------- Nice clean-up and comments documenting the present and future! ::: docshell/base/nsDocShell.h @@ +367,5 @@ > > + // Determine if a service worker is allowed to control a window in this > + // docshell with the given URL. If there are any reasons it should not, > + // this will return false. If true is returned then the window *may* be > + // controleld. The caller must still consult either the parent controller typo not: s/controleld/controlled/

Comment on attachment 8939636 [details] [diff] [review] P15 Add session lifetime policy checks to test_third_party_iframe.html. r=asuth Review of attachment 8939636 [details] [diff] [review]: ----------------------------------------------------------------- Our mochitest framework totally needs some kind of magic template literal parser that lets us draw the testing matrix using cool emojis instead of listing them all out by hand. Like: runTestMatrix("interception allowed", matrixParser` LIFETIME_EXPIRE LIFETIME_SESSION BEHAVIOR_ACCEPT :) :( `, testShouldIntercept); Obviously, pretend I know how to make emojis.

Pushed by bkelly@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/21042078d807 P1 Add ClientHandle::OnDetach() which returns a MozPromise that resolves on actor destruction. r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/ef7cf1394d0f P2 Add ServiceWorkerManager mControlledClients to track controlled ClientHandle references. r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/818fd73be280 P3 Refactor ServiceWorkerManager::GetDocumentRegistration() to GetClientRegistration(). r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/d432082aabbd P4 Make ServiceWorkerManager::UpdateClientControllers use mControlledClients. r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/560f15a80097 P5 Make ServiceWorkerManager::RemoveRegistration assert there is no controlled document. r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/834f38d73b6f P6 Rename some service worker methods to not reference documents. r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/841e106b2810 P7 Use the mControlledClients list to drive controller start and stop logic. r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/508a6ad11141 P8 Fix unregister-then-register-new-script.https.html to not race iframe.remove() and expect resurrection on failed scripts. r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/8449669c2adc P9 Refactor MaybeCheckNavigationUpdate() to take a ClientInfo instead of a document. r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/8e4ffe879be7 P10 Fix the test_skip_waiting.html mochitest to properly wait for active worker state. r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/86acf7e4d09d P11 Fix test_workerupdatefound.html not to frame loading against SW activation and updatefound events. r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/d8406786b419 P12 Don't mark an initial about:blank client as controlled if its sandboxed. r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/42f1a8bfebf4 P13 Check for a different final document principal and reset the ClientSource when it happens. r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/e7b92985e941 P14 Assert that storage is allowed when a ClientSource is both execution ready and controlled. r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/93e07d0261cb P15 Add session lifetime policy checks to test_third_party_iframe.html. r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/5b1109ca1c1f P16 Make nsDocShell check for session cookie lifetime policy before allowing service worker intercept. r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/141238320685 P17 Make web extension tests wait for service worker to activate to avoid races. r=kmag

https://hg.mozilla.org/mozilla-central/rev/21042078d807 https://hg.mozilla.org/mozilla-central/rev/ef7cf1394d0f https://hg.mozilla.org/mozilla-central/rev/818fd73be280 https://hg.mozilla.org/mozilla-central/rev/d432082aabbd https://hg.mozilla.org/mozilla-central/rev/560f15a80097 https://hg.mozilla.org/mozilla-central/rev/834f38d73b6f https://hg.mozilla.org/mozilla-central/rev/841e106b2810 https://hg.mozilla.org/mozilla-central/rev/508a6ad11141 https://hg.mozilla.org/mozilla-central/rev/8449669c2adc https://hg.mozilla.org/mozilla-central/rev/8e4ffe879be7 https://hg.mozilla.org/mozilla-central/rev/86acf7e4d09d https://hg.mozilla.org/mozilla-central/rev/d8406786b419 https://hg.mozilla.org/mozilla-central/rev/42f1a8bfebf4 https://hg.mozilla.org/mozilla-central/rev/e7b92985e941 https://hg.mozilla.org/mozilla-central/rev/93e07d0261cb https://hg.mozilla.org/mozilla-central/rev/5b1109ca1c1f https://hg.mozilla.org/mozilla-central/rev/141238320685

Pushed by archaeopteryx@coole-files.de: https://hg.mozilla.org/mozilla-central/rev/b84fe2ad1ca2 Fix merge bustage. r=bustage-fix a=bustage-fix

Pushed by archaeopteryx@coole-files.de: https://hg.mozilla.org/mozilla-central/rev/9099a6ed993f Follow-up: Move method to correct position. r=merge-fix a=merge-fix