Bug 1373660 - Block the preallocated process manager while a content process is being launched.

We should not let the ppm to do work before the first paint in a new cp. This patch
makes sure that we only let the ppm spawn a new process after the last process reached
an idle state AND the main process becomes idle too. r=mrbkap

dom/ipc/ContentParent.cpp

@@ -603,6 +603,8 @@ ContentParent::PreallocateProcess()
     new ContentParent(/* aOpener = */ nullptr,
                       NS_LITERAL_STRING(DEFAULT_REMOTE_TYPE));
 
+  PreallocatedProcessManager::AddBlocker(process);
+
   if (!process->LaunchSubprocess(PROCESS_PRIORITY_PREALLOC)) {
     return nullptr;
   }
@@ -880,6 +882,9 @@ ContentParent::GetNewOrUsedBrowserProcess(const nsAString& aRemoteType,
   // Create a new process from scratch.
   RefPtr<ContentParent> p = new ContentParent(aOpener, aRemoteType);
 
+  // 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;
   }
@@ -2727,10 +2732,9 @@ mozilla::ipc::IPCResult
 ContentParent::RecvFirstIdle()
 {
   // When the ContentChild goes idle, it sends us a FirstIdle message
-  // which we use as a good time to prelaunch another process. If we
-  // prelaunch any sooner than this, then we'll be competing with the
-  // child process and slowing it down.
-  PreallocatedProcessManager::AllocateAfterDelay();
+  // which we use as a good time to signal the PreallocatedProcessManager
+  // that it can start allocating processes from now on.
+  PreallocatedProcessManager::RemoveBlocker(this);
   return IPC_OK();
 }
 

dom/ipc/PreallocatedProcessManager.cpp

@@ -39,9 +39,8 @@ public:
   NS_DECL_NSIOBSERVER
 
   // See comments on PreallocatedProcessManager for these methods.
-  void AllocateAfterDelay();
-  void AllocateOnIdle();
-  void AllocateNow();
+  void AddBlocker(ContentParent* aParent);
+  void RemoveBlocker(ContentParent* aParent);
   already_AddRefed<ContentParent> Take();
   bool Provide(ContentParent* aParent);
 
@@ -54,6 +53,11 @@ private:
 
   void Init();
 
+  bool CanAllocate();
+  void AllocateAfterDelay();
+  void AllocateOnIdle();
+  void AllocateNow();
+
   void RereadPrefs();
   void Enable();
   void Disable();
@@ -64,6 +68,7 @@ private:
   bool mEnabled;
   bool mShutdown;
   RefPtr<ContentParent> mPreallocatedProcess;
+  nsTHashtable<nsUint64HashKey> mBlockers;
 };
 
 /* static */ StaticRefPtr<PreallocatedProcessManagerImpl>
@@ -79,15 +84,6 @@ PreallocatedProcessManagerImpl::Singleton()
     ClearOnShutdown(&sSingleton);
   }
 
-  // First time when we init sSingleton, the pref database might not be in a
-  // reliable state (we are too early), so despite dom.ipc.processPrelaunch.enabled
-  // is set to true Preferences::GetBool will return false (it cannot find the pref).
-  // Since Init() above will be called only once, and the pref will not be changed,
-  // the manger will stay disabled. To prevent that let's re-read the pref each time
-  // someone accessing the manager singleton. This is a hack but this is not a hot code
-  // so it should be fine.
-  sSingleton->RereadPrefs();
-
   return sSingleton;
 }
 
@@ -168,6 +164,11 @@ PreallocatedProcessManagerImpl::Take()
     return nullptr;
   }
 
+  if (mPreallocatedProcess) {
+    // The preallocated process is taken. Let's try to start up a new one soon.
+    AllocateOnIdle();
+  }
+
   return mPreallocatedProcess.forget();
 }
 
@@ -195,20 +196,46 @@ PreallocatedProcessManagerImpl::Enable()
   AllocateAfterDelay();
 }
 
+void
+PreallocatedProcessManagerImpl::AddBlocker(ContentParent* aParent)
+{
+  uint64_t childID = aParent->ChildID();
+  MOZ_ASSERT(!mBlockers.Contains(childID));
+  mBlockers.PutEntry(childID);
+}
+
+void
+PreallocatedProcessManagerImpl::RemoveBlocker(ContentParent* aParent)
+{
+  uint64_t childID = aParent->ChildID();
+  MOZ_ASSERT(mBlockers.Contains(childID));
+  mBlockers.RemoveEntry(childID);
+  if (!mPreallocatedProcess && mBlockers.IsEmpty()) {
+    AllocateAfterDelay();
+  }
+}
+
+bool
+PreallocatedProcessManagerImpl::CanAllocate()
+{
+  return mEnabled &&
+         mBlockers.IsEmpty() &&
+         !mPreallocatedProcess &&
+         !mShutdown &&
+         !ContentParent::IsMaxProcessCountReached(NS_LITERAL_STRING(DEFAULT_REMOTE_TYPE));
+}
+
 void
 PreallocatedProcessManagerImpl::AllocateAfterDelay()
 {
-  if (!mEnabled || mPreallocatedProcess || mShutdown) {
+  if (!mEnabled) {
     return;
   }
 
-  // Originally AllocateOnIdle() was post here, but since the gecko parent
-  // message loop in practice never goes idle, that didn't work out well.
-  // Let's just launch the process after the delay.
   NS_DelayedDispatchToCurrentThread(
-    NewRunnableMethod("PreallocatedProcessManagerImpl::AllocateNow",
+    NewRunnableMethod("PreallocatedProcessManagerImpl::AllocateOnIdle",
                       this,
-                      &PreallocatedProcessManagerImpl::AllocateNow),
+                      &PreallocatedProcessManagerImpl::AllocateOnIdle),
     Preferences::GetUint("dom.ipc.processPrelaunch.delayMs",
                          DEFAULT_ALLOCATE_DELAY));
 }
@@ -216,7 +243,7 @@ PreallocatedProcessManagerImpl::AllocateAfterDelay()
 void
 PreallocatedProcessManagerImpl::AllocateOnIdle()
 {
-  if (!mEnabled || mPreallocatedProcess || mShutdown) {
+  if (!mEnabled) {
     return;
   }
 
@@ -229,8 +256,11 @@ PreallocatedProcessManagerImpl::AllocateOnIdle()
 void
 PreallocatedProcessManagerImpl::AllocateNow()
 {
-  if (!mEnabled || mPreallocatedProcess || mShutdown ||
-      ContentParent::IsMaxProcessCountReached(NS_LITERAL_STRING(DEFAULT_REMOTE_TYPE))) {
+  if (!CanAllocate()) {
+    if (mEnabled && !mPreallocatedProcess && !mBlockers.IsEmpty()) {
+      // If it's too early to allocate a process let's retry later.
+      AllocateAfterDelay();
+    }
     return;
   }
 
@@ -260,10 +290,6 @@ PreallocatedProcessManagerImpl::CloseProcess()
 void
 PreallocatedProcessManagerImpl::ObserveProcessShutdown(nsISupports* aSubject)
 {
-  if (!mPreallocatedProcess) {
-    return;
-  }
-
   nsCOMPtr<nsIPropertyBag2> props = do_QueryInterface(aSubject);
   NS_ENSURE_TRUE_VOID(props);
 
@@ -271,9 +297,11 @@ PreallocatedProcessManagerImpl::ObserveProcessShutdown(nsISupports* aSubject)
   props->GetPropertyAsUint64(NS_LITERAL_STRING("childID"), &childID);
   NS_ENSURE_TRUE_VOID(childID != CONTENT_PROCESS_ID_UNKNOWN);
 
-  if (childID == mPreallocatedProcess->ChildID()) {
+  if (mPreallocatedProcess && childID == mPreallocatedProcess->ChildID()) {
     mPreallocatedProcess = nullptr;
   }
+
+  mBlockers.RemoveEntry(childID);
 }
 
 inline PreallocatedProcessManagerImpl* GetPPMImpl()
@@ -282,21 +310,15 @@ inline PreallocatedProcessManagerImpl* GetPPMImpl()
 }
 
 /* static */ void
-PreallocatedProcessManager::AllocateAfterDelay()
+PreallocatedProcessManager::AddBlocker(ContentParent* aParent)
 {
-  GetPPMImpl()->AllocateAfterDelay();
+  GetPPMImpl()->AddBlocker(aParent);
 }
 
 /* static */ void
-PreallocatedProcessManager::AllocateOnIdle()
+PreallocatedProcessManager::RemoveBlocker(ContentParent* aParent)
 {
-  GetPPMImpl()->AllocateOnIdle();
-}
-
-/* static */ void
-PreallocatedProcessManager::AllocateNow()
-{
-  GetPPMImpl()->AllocateNow();
+  GetPPMImpl()->RemoveBlocker(aParent);
 }
 
 /* static */ already_AddRefed<ContentParent>

dom/ipc/PreallocatedProcessManager.h

@@ -33,31 +33,14 @@ class PreallocatedProcessManager final
   typedef mozilla::dom::ContentParent ContentParent;
 
 public:
-  /**
-   * Create a process after a delay.  We wait for a period of time (specified
-   * by the dom.ipc.processPrelaunch.delayMs pref), then wait for this process
-   * to go idle, then allocate the new process.
-   *
-   * If the dom.ipc.processPrelaunch.enabled pref is false, or if we already
-   * have a preallocated process, this function does nothing.
-   */
-  static void AllocateAfterDelay();
 
   /**
-   * Create a process once this process goes idle.
-   *
-   * If the dom.ipc.processPrelaunch.enabled pref is false, or if we already
-   * have a preallocated process, this function does nothing.
+   * Before first paint we don't want to allocate any processes in the background.
+   * To avoid that, the PreallocatedProcessManager won't start up any processes while
+   * there is a blocker active.
    */
-  static void AllocateOnIdle();
-
-  /**
-   * Create a process right now.
-   *
-   * If the dom.ipc.processPrelaunch.enabled pref is false, or if we already
-   * have a preallocated process, this function does nothing.
-   */
-  static void AllocateNow();
+  static void AddBlocker(ContentParent* aParent);
+  static void RemoveBlocker(ContentParent* aParent);
 
   /**
    * Take the preallocated process, if we have one.  If we don't have one, this