Here's my current patch queue:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d6795a5b3c1344f0344186d5d40411bbbffe7d73

In the last patch, I was finally able to get rid of the main thread in LSRequestBase/LSSimpleRequestBase.

However, there's still one spot in localstorage/ActorsParent.cpp which uses main thread.
It's the creation of old style originKey for which we use GenerateOriginKey which currently needs nsIPrincipal.
If we want to use MozURL instead of nsIPrincipal, we need to enhance MozURL a little bit.

Also, quota manager uses main thread in a couple of places, most of them are "easily" fixable with current infrastructure, but there's one spot where we need to get origin for a spec without the option to fall back to principal.originNoSuffix() for special schemes (for example when we are restoring the .metadata-v2 file).
The problem is that some schemes are currently not supported by MozURL's Origin method and since we don't have principal.originNoSuffix(), we have to teach MozURL to support them.

:asuth, I think I need your initial feedback for the current patch queue, especially regarding how to handle special schemes.
One option is to enhance MozURL::Origin to support them.

Actually, I think the current patch queue is enough for this bug, I'll file separate bugs for main thread usage described in comment 13.

(In reply to Andrew Sutherland [:asuth] from comment #10)

Comment on attachment 9034133 [details] [diff] [review]
Part 3: Move observer registration to InitializeLocalStorage

Review of attachment 9034133 [details] [diff] [review]:

Reviewer note: sInstance goes away in the next patch.

::: dom/localstorage/ActorsParent.cpp
@@ +7919,5 @@

• return NS_ERROR_FAILURE;
• }
• MOZ_ALWAYS_SUCCEEDS(obs->RemoveObserver(this, kPrivateBrowsingObserverTopic));
• MOZ_ALWAYS_SUCCEEDS(obs->RemoveObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID));

Please add a comment that calls out that the instance will have died after
this removal call and that it's not safe to do anything after this point.
It's not always super obvious later on when touching code like this.

Ok, I added:
In general, the instance will have died after the latter removal call, so
it's not safe to do anything after that point.
However, Shutdown is currently called from Observe which is called by the
Observer Service which holds a strong reference to the observer while the
Observe method is being called.

(In reply to Andrew Sutherland [:asuth] from comment #11)

Comment on attachment 9034134 [details] [diff] [review]
Part 4: Send an async IPC message instead of dispatching a runnable to the
PBackground thread when clearing private browsing

Review of attachment 9034134 [details] [diff] [review]:

::: dom/localstorage/ActorsParent.h
@@ +70,5 @@

bool DeallocPBackgroundLSSimpleRequestParent(
PBackgroundLSSimpleRequestParent* aActor);

+bool RecvClearPrivateBrowsing();

(And this name will want to be updated when the ipdl method name gets
namespaced too.)

Ok.

::: ipc/glue/BackgroundParentImpl.cpp
@@ +397,5 @@

+mozilla::ipc::IPCResult BackgroundParentImpl::RecvClearPrivateBrowsing() {

• AssertIsInMainProcess();
• AssertIsOnBackgroundThread();
• if (!mozilla::dom::RecvClearPrivateBrowsing()) {

Arguably we should be doing a safety check that
!BackgroundParent::IsOtherProcessActor(this) so that only the parent can
tell us this.

Oh, good catch, fixed.

::: ipc/glue/PBackground.ipdl
@@ +150,5 @@

* see `PBackgroundLSRequest.ipdl` for details.)
*/

async PBackgroundLSSimpleRequest(LSSimpleRequestParams params);

• async ClearPrivateBrowsing();

Please namespace this method somehow, as it's very generic. Maybe something
like LSClearPrivateBrowsing().

Ok, changed.

Also please add a comment here that explains what's going on. Something
like:

Asynchronously propagates the "last-pb-context-exited" observer notification
to LocalStorage NextGen implementation so it can clear retained
private-browsing in-memory Datastores. Using a (same-process) IPC message
avoids the need for the main-thread nsIObserver to have a reference to the
PBackground thread and directly dispatch a runnable to it.

Added.

(In reply to Andrew Sutherland [:asuth] from comment #12)

Comment on attachment 9034136 [details] [diff] [review]
Part 6: Release content parent using NS_ProxyRelease

Review of attachment 9034136 [details] [diff] [review]:

r=asuth with the proposed change made.

::: dom/localstorage/ActorsParent.cpp
@@ +3131,2 @@

   RefPtr<ContentParent> contentParent =
       BackgroundParent::GetContentParent(aBackgroundActor);

You can just call BackgroundParent::GetChildID(aBackgroundActor) and avoid
getting the ContentParent reference. If you look at where it gets it at
https://searchfox.org/mozilla-central/rev/
ecf61f8f3904549f5d65a8a511dbd7ea4fd1a51d/ipc/glue/BackgroundImpl.cpp#744
it's calling it on the same actor->mContent that GetContentParent() returns
but without the refcount shenanigans.

Cool, that's even better.

I'm going to land first 6 patches and then request reviews for the rest in phabricator.

https://hg.mozilla.org/mozilla-central/rev/9b51d5398126
https://hg.mozilla.org/mozilla-central/rev/09ac1f780e56
https://hg.mozilla.org/mozilla-central/rev/534dc9cee9ae
https://hg.mozilla.org/mozilla-central/rev/2c48efbd5486
https://hg.mozilla.org/mozilla-central/rev/6e9e487f5d16
https://hg.mozilla.org/mozilla-central/rev/08d6d7646f18

The special case is now handled in GetSpecialBaseDomain in ContentPrincipal.cpp

https://hg.mozilla.org/mozilla-central/rev/b5b99eeb0467
https://hg.mozilla.org/mozilla-central/rev/6dab164824a8
https://hg.mozilla.org/mozilla-central/rev/5adb364e4ddf
https://hg.mozilla.org/mozilla-central/rev/1169146b0409
https://hg.mozilla.org/mozilla-central/rev/5b51bc2a466c
https://hg.mozilla.org/mozilla-central/rev/a87a6189d20b
https://hg.mozilla.org/mozilla-central/rev/dc30924bd97b
https://hg.mozilla.org/mozilla-central/rev/d430843fe847
https://hg.mozilla.org/mozilla-central/rev/e8acfb77b2f1
https://hg.mozilla.org/mozilla-central/rev/696bd0721718
https://hg.mozilla.org/mozilla-central/rev/eb1b8fcb03cb

(the comment 13 discussion about URIs moved elsewhere, clearing needinfo)