Bug 1488993 - Fix PreallocatedProcessManager blocker management (v2). r=mconley,smaug

This fixes/adjusts two things about how content process preallocation is blocked:

1. Processes aren't registered as blockers until after they launch
successfully.  The main goal is to not leak a blocker if launch fails,
but we don't need to block *while* launching synchronously, because this
all happens on the main thread.

2. Preallocated processes themselves aren't blockers.  The main goal
here is so that async preallocation doesn't need extra complexity to
avoid being blocked by itself when launch completes.  This mostly
doesn't affect actual behavior, because we currently support at most
one preallocated process.  The difference is the window from when the
process is sent its first PBrowserConstructor until when it's next idle,
where there is now no longer a blocker, but this seems to be relatively
short (~100ms) and we don't even try to launch a new process until at
least 1s + an idle runnable.

This patch does not explicitly RemoveBlocker in ActorDestroy like the
first attempt did, because it's unnecessary: this is handled in the
ipc:content-shutdown observer.

Differential Revision: https://phabricator.services.mozilla.com/D8939

--HG--
extra : moz-landing-system : lando

dom/ipc/ContentParent.cpp

@@ -631,8 +631,6 @@ ContentParent::PreallocateProcess()
                       eNotRecordingOrReplaying,
                       /* aRecordingFile = */ EmptyString());
 
-  PreallocatedProcessManager::AddBlocker(process);
-
   if (!process->LaunchSubprocess(PROCESS_PRIORITY_PREALLOC)) {
     return nullptr;
   }
@@ -902,13 +900,13 @@ ContentParent::GetNewOrUsedBrowserProcess(Element* aFrameElement,
   // Create a new process from scratch.
   RefPtr<ContentParent> p = new ContentParent(aOpener, aRemoteType, recordReplayState, recordingFile);
 
-  // Until the new process is ready let's not allow to start up any preallocated processes.
-  PreallocatedProcessManager::AddBlocker(p);
-
   if (!p->LaunchSubprocess(aPriority)) {
     return nullptr;
   }
 
+  // Until the new process is ready let's not allow to start up any preallocated processes.
+  PreallocatedProcessManager::AddBlocker(p);
+
   if (recordReplayState == eNotRecordingOrReplaying) {
     contentParents.AppendElement(p);
   }

dom/ipc/PreallocatedProcessManager.cpp

@@ -212,7 +212,13 @@ void
 PreallocatedProcessManagerImpl::RemoveBlocker(ContentParent* aParent)
 {
   uint64_t childID = aParent->ChildID();
-  MOZ_ASSERT(mBlockers.Contains(childID));
+  // This used to assert that the blocker existed, but preallocated
+  // processes aren't blockers anymore because it's not useful and
+  // interferes with async launch, and it's simpler if content
+  // processes don't need to remember whether they were preallocated.
+  // (And preallocated processes can't AddBlocker when taken, because
+  // it's possible for a short-lived process to be recycled through
+  // Provide() and Take() before reaching RecvFirstIdle.)
   mBlockers.RemoveEntry(childID);
   if (!mPreallocatedProcess && mBlockers.IsEmpty()) {
     AllocateAfterDelay();