Bug 1033618 - If we take the preallocated process but fail to start the app try again with a non-preallocated process. r=smaug, a=2.0+
☠☠ backed out by 394b7c158cf0 ☠ ☠
authorKyle Huey <khuey@kylehuey.com>
Tue, 08 Jul 2014 15:56:22 -0700
changeset 208852 756a66aa8ee75cfd15607119185161b793a0a6fc
parent 208851 8b04bdcbd916266f99a3b51150e4d5a62c8c176d
child 208853 f56929d87253cbdf279c623743701644c431cc77
push id494
push userraliiev@mozilla.com
push dateMon, 25 Aug 2014 18:42:16 +0000
treeherdermozilla-release@a3cc3e46b571 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug, 2
bugs1033618
milestone32.0a2
Bug 1033618 - If we take the preallocated process but fail to start the app try again with a non-preallocated process. r=smaug, a=2.0+
dom/ipc/ContentParent.cpp
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -866,28 +866,25 @@ ContentParent::CreateBrowserOrApp(const 
         // Check that the process is still alive and set its priority.
         // Hopefully the process won't die after this point, if this call
         // succeeds.
         if (!p->SetPriorityAndCheckIsAlive(initialPriority)) {
             p = nullptr;
         }
     }
 
+    bool reused = !!p;
+    bool tookPreallocated = false;
     if (!p) {
         p = MaybeTakePreallocatedAppProcess(manifestURL,
                                             initialPriority);
-        if (!p) {
-#ifdef MOZ_NUWA_PROCESS
-            if (Preferences::GetBool("dom.ipc.processPrelaunch.enabled",
-                                     false)) {
-                // Returning nullptr from here so the frame loader will retry
-                // later when we have a spare process.
-                return nullptr;
-            }
-#endif
+        tookPreallocated = !!p;
+        if (!tookPreallocated) {
+            // XXXkhuey Nuwa wants the frame loader to try again later, but the
+            // frame loader is really not set up to do that ...
             NS_WARNING("Unable to use pre-allocated app process");
             p = new ContentParent(ownApp,
                                   /* isForBrowserElement = */ false,
                                   /* isForPreallocated = */ false,
                                   initialPriority);
             p->Init();
         }
         sAppContentParents->Put(manifestURL, p);
@@ -900,16 +897,39 @@ ContentParent::CreateBrowserOrApp(const 
     PBrowserParent* browser = p->SendPBrowserConstructor(
         // DeallocPBrowserParent() releases this ref.
         nsRefPtr<TabParent>(tp).forget().take(),
         aContext.AsIPCTabContext(),
         chromeFlags,
         p->ChildID(),
         p->IsForApp(),
         p->IsForBrowser());
+    if (!browser) {
+        // We failed to actually start the PBrowser.  This can happen if the
+        // other process has already died.
+        if (!reused) {
+            // Don't leave a broken ContentParent in the hashtable.
+            p->KillHard();
+            sAppContentParents->Remove(manifestURL);
+            p = nullptr;
+        }
+
+        // If we took the preallocated process and it was already dead, try
+        // again with a non-preallocated process.  We can be sure this won't
+        // loop forever, because the next time through there will be no
+        // preallocated process to take.
+        if (tookPreallocated) {
+          return ContentParent::CreateBrowserOrApp(aContext,
+                                                   aFrameElement,
+                                                   aOpenerContentParent);
+        }
+
+        // Otherwise just give up.
+        return nullptr;
+    }
 
     p->MaybeTakeCPUWakeLock(aFrameElement);
 
     return static_cast<TabParent*>(browser);
 }
 
 void
 ContentParent::GetAll(nsTArray<ContentParent*>& aArray)