Bug 838616 - Eliminate a process-priority race condition around preallocated process creation. r=cjones, a=lsblakk

Previously, it was possible to return a dead preallocated process.  This
patch eliminates or at least significantly reduces the likelihood of
this race.

dom/ipc/ContentParent.cpp

@@ -234,10 +234,26 @@ ContentParent::ScheduleDelayedPreallocateAppProcess()
 }
 
 /*static*/ already_AddRefed<ContentParent>
-ContentParent::MaybeTakePreallocatedAppProcess()
+ContentParent::MaybeTakePreallocatedAppProcess(const nsAString& aAppManifestURL,
+                                               ChildPrivileges aPrivs)
 {
     nsRefPtr<ContentParent> process = sPreallocatedAppProcess.get();
     sPreallocatedAppProcess = nullptr;
+
+    if (!process) {
+        return nullptr;
+    }
+
+    if (!process->TransformPreallocatedIntoApp(aAppManifestURL, aPrivs)) {
+        NS_WARNING("Can't TransformPrealocatedIntoApp.  Maybe "
+                   "the preallocated process died?");
+
+        // Kill the process just in case it's not actually dead; we don't want
+        // to "leak" this process!
+        process->KillHard();
+        return nullptr;
+    }
+
     return process.forget();
 }
 
@@ -426,10 +442,8 @@ ContentParent::CreateBrowserOrApp(const TabContext& aContext)
     nsRefPtr<ContentParent> p = gAppContentParents->Get(manifestURL);
     if (!p) {
         ChildPrivileges privs = PrivilegesForApp(ownApp);
-        p = MaybeTakePreallocatedAppProcess();
-        if (p) {
-            p->TransformPreallocatedIntoApp(manifestURL, privs);            
-        } else {
+        p = MaybeTakePreallocatedAppProcess(manifestURL, privs);
+        if (!p) {
             NS_WARNING("Unable to use pre-allocated app process");
             p = new ContentParent(manifestURL, /* isBrowserElement = */ false,
                                   privs);
@@ -515,7 +529,7 @@ ContentParent::Init()
     NS_ASSERTION(observer, "FileUpdateDispatcher is null");
 }
 
-void
+bool
 ContentParent::TransformPreallocatedIntoApp(const nsAString& aAppManifestURL,
                                             ChildPrivileges aPrivs)
 {
@@ -534,7 +548,7 @@ ContentParent::TransformPreallocatedIntoApp(const nsAString& aAppManifestURL,
     }
 
     // If this fails, the child process died.
-    unused << SendSetProcessPrivileges(aPrivs);
+    return SendSetProcessPrivileges(aPrivs);
 }
 
 void

dom/ipc/ContentParent.h

@@ -162,7 +162,14 @@ private:
     static void PreallocateAppProcess();
     static void DelayedPreallocateAppProcess();
     static void ScheduleDelayedPreallocateAppProcess();
-    static already_AddRefed<ContentParent> MaybeTakePreallocatedAppProcess();
+
+    // Take the preallocated process and transform it into a "real" app process,
+    // for the specified manifest URL.  If there is no preallocated process (or
+    // if it's dead), this returns false.
+    static already_AddRefed<ContentParent>
+    MaybeTakePreallocatedAppProcess(const nsAString& aAppManifestURL,
+                                    ChildPrivileges aPrivs);
+
     static void FirstIdle();
 
     // Hide the raw constructor methods since we don't want client code
@@ -178,7 +185,7 @@ private:
 
     // Transform a pre-allocated app process into a "real" app
     // process, for the specified manifest URL.
-    void TransformPreallocatedIntoApp(const nsAString& aAppManifestURL,
+    bool TransformPreallocatedIntoApp(const nsAString& aAppManifestURL,
                                       ChildPrivileges aPrivs);
 
     /**