Backed out changeset 55f1f62f7477 (bug 836654)
authorEd Morley <emorley@mozilla.com>
Thu, 14 Feb 2013 10:02:27 +0000
changeset 131749 bbfd203a9019fe868a3559687985dfce7cdae262
parent 131748 20af8035e0f2081e570ec004acf6cc1b64c04d21
child 131750 3150005010d9b6686cae272486f0728cb9def828
push id2323
push userbbajaj@mozilla.com
push dateMon, 01 Apr 2013 19:47:02 +0000
treeherdermozilla-beta@7712be144d91 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs836654
milestone21.0a1
backs out55f1f62f747754db259732d557937de29d0511fa
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out changeset 55f1f62f7477 (bug 836654)
content/base/src/nsFrameLoader.cpp
dom/ipc/ContentParent.cpp
dom/ipc/ContentParent.h
--- a/content/base/src/nsFrameLoader.cpp
+++ b/content/base/src/nsFrameLoader.cpp
@@ -2059,16 +2059,24 @@ nsFrameLoader::TryRemoteBrowser()
   } else if (OwnerIsBrowserFrame()) {
     // The |else| above is unnecessary; OwnerIsBrowserFrame() implies !ownApp.
     context.SetTabContextForBrowserFrame(containingApp, scrollingBehavior);
   }
 
   nsCOMPtr<nsIDOMElement> ownerElement = do_QueryInterface(mOwnerContent);
   mRemoteBrowser = ContentParent::CreateBrowserOrApp(context, ownerElement);
   if (mRemoteBrowser) {
+    // If we're an app, send the frame element's mozapptype down to the child
+    // process.  This ends up in TabChild::GetAppType().
+    if (ownApp) {
+      nsAutoString appType;
+      mOwnerContent->GetAttr(kNameSpaceID_None, nsGkAtoms::mozapptype, appType);
+      mRemoteBrowser->SendSetAppType(appType);
+    }
+
     nsCOMPtr<nsIDocShellTreeItem> rootItem;
     parentAsItem->GetRootTreeItem(getter_AddRefs(rootItem));
     nsCOMPtr<nsIDOMWindow> rootWin = do_GetInterface(rootItem);
     nsCOMPtr<nsIDOMChromeWindow> rootChromeWin = do_QueryInterface(rootWin);
     NS_ABORT_IF_FALSE(rootChromeWin, "How did we not get a chrome window here?");
 
     nsCOMPtr<nsIBrowserDOMWindow> browserDOMWin;
     rootChromeWin->GetBrowserDOMWindow(getter_AddRefs(browserDOMWin));
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -262,18 +262,18 @@ ContentParent::MaybeTakePreallocatedAppP
 {
     nsRefPtr<ContentParent> process = sPreallocatedAppProcess.get();
     sPreallocatedAppProcess = nullptr;
 
     if (!process) {
         return nullptr;
     }
 
-    if (!process->SetPriorityAndCheckIsAlive(aInitialPriority) ||
-        !process->TransformPreallocatedIntoApp(aAppManifestURL, aPrivs)) {
+    if (!process->TransformPreallocatedIntoApp(aAppManifestURL, aPrivs,
+                                               aInitialPriority)) {
         // 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();
 }
@@ -487,62 +487,40 @@ ContentParent::CreateBrowserOrApp(const 
 
     // Each app gets its own ContentParent instance.
     nsAutoString manifestURL;
     if (NS_FAILED(ownApp->GetManifestURL(manifestURL))) {
         NS_ERROR("Failed to get manifest URL");
         return nullptr;
     }
 
-    ProcessPriority initialPriority = GetInitialProcessPriority(aFrameElement);
+    nsRefPtr<ContentParent> p = gAppContentParents->Get(manifestURL);
+    if (!p) {
+        ProcessPriority initialPriority = GetInitialProcessPriority(aFrameElement);
 
-    nsRefPtr<ContentParent> p = gAppContentParents->Get(manifestURL);
-    if (p) {
-        // 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;
-        }
-    }
-
-    if (!p) {
         ChildPrivileges privs = PrivilegesForApp(ownApp);
         p = MaybeTakePreallocatedAppProcess(manifestURL, privs,
                                             initialPriority);
         if (!p) {
             NS_WARNING("Unable to use pre-allocated app process");
             p = new ContentParent(manifestURL, /* isBrowserElement = */ false,
                                   privs, initialPriority);
             p->Init();
         }
         gAppContentParents->Put(manifestURL, p);
     }
 
+    p->MaybeTakeCPUWakeLock(aFrameElement);
+
     nsRefPtr<TabParent> tp = new TabParent(aContext);
     tp->SetOwnerElement(aFrameElement);
     PBrowserParent* browser = p->SendPBrowserConstructor(
         tp.forget().get(), // DeallocPBrowserParent() releases this ref.
         aContext.AsIPCTabContext(),
         /* chromeFlags */ 0);
-
-    // Send the frame element's mozapptype down to the child process.  This ends
-    // up in TabChild::GetAppType().  We have to do this /before/ we acquire the
-    // CPU wake lock for this process, because if the child sees that it has a
-    // CPU wake lock but its TabChild doesn't have the right mozapptype, it
-    // might downgrade its process priority.
-    nsCOMPtr<Element> frameElement = do_QueryInterface(aFrameElement);
-    if (frameElement) {
-      nsAutoString appType;
-      frameElement->GetAttr(kNameSpaceID_None, nsGkAtoms::mozapptype, appType);
-      browser->SendSetAppType(appType);
-    }
-
-    p->MaybeTakeCPUWakeLock(aFrameElement);
-
     return static_cast<TabParent*>(browser);
 }
 
 static PLDHashOperator
 AppendToTArray(const nsAString& aKey, ContentParent* aValue, void* aArray)
 {
     nsTArray<ContentParent*> *array =
         static_cast<nsTArray<ContentParent*>*>(aArray);
@@ -690,24 +668,24 @@ StaticAutoPtr<LinkedList<SystemMessageHa
     SystemMessageHandledListener::sListeners;
 
 NS_IMPL_ISUPPORTS1(SystemMessageHandledListener,
                    nsITimerCallback)
 
 } // anonymous namespace
 
 void
-ContentParent::SetProcessPriority(ProcessPriority aPriority)
+ContentParent::SetProcessInitialPriority(ProcessPriority aInitialPriority)
 {
     if (!Preferences::GetBool("dom.ipc.processPriorityManager.enabled")) {
         return;
     }
 
-    hal::SetProcessPriority(base::GetProcId(mSubprocess->GetChildProcessHandle()),
-                            aPriority);
+    SetProcessPriority(base::GetProcId(mSubprocess->GetChildProcessHandle()),
+                       aInitialPriority);
 }
 
 void
 ContentParent::MaybeTakeCPUWakeLock(nsIDOMElement* aFrameElement)
 {
     // Take the CPU wake lock on behalf of this processs if it's expecting a
     // system message.  We'll release the CPU lock once the message is
     // delivered, or after some period of time, which ever comes first.
@@ -725,46 +703,43 @@ ContentParent::MaybeTakeCPUWakeLock(nsID
 
     // This object's Init() function keeps it alive.
     nsRefPtr<SystemMessageHandledListener> listener =
         new SystemMessageHandledListener();
     listener->Init(lock);
 }
 
 bool
-ContentParent::SetPriorityAndCheckIsAlive(ProcessPriority aPriority)
+ContentParent::TransformPreallocatedIntoApp(const nsAString& aAppManifestURL,
+                                            ChildPrivileges aPrivs,
+                                            ProcessPriority aInitialPriority)
 {
-    SetProcessPriority(aPriority);
+    MOZ_ASSERT(mAppManifestURL == MAGIC_PREALLOCATED_APP_MANIFEST_URL);
+    // Clients should think of mAppManifestURL as const ... we're
+    // bending the rules here just for the preallocation hack.
+    const_cast<nsString&>(mAppManifestURL) = aAppManifestURL;
 
-    // Now that we've set this process's priority, check whether the process is
-    // still alive.  Hopefully we've set the priority to FOREGROUND*, so the
-    // process won't unexpectedly crash after this point!
+    SetProcessInitialPriority(aInitialPriority);
+
+    // Now that we've increased the process's priority from BACKGROUND (where
+    // the preallocated app sits) to something higher, check whether the process
+    // is still alive.  Hopefully the process won't unexpectedly crash after
+    // this point!
     //
     // It's not legal to call DidProcessCrash on Windows if the process has not
-    // terminated yet, so we have to skip this check here.
+    // terminated yet, so we have to skip this check there.
+
 #ifndef XP_WIN
     bool exited = false;
     base::DidProcessCrash(&exited, mSubprocess->GetChildProcessHandle());
     if (exited) {
         return false;
     }
 #endif
 
-    return true;
-}
-
-bool
-ContentParent::TransformPreallocatedIntoApp(const nsAString& aAppManifestURL,
-                                            ChildPrivileges aPrivs)
-{
-    MOZ_ASSERT(mAppManifestURL == MAGIC_PREALLOCATED_APP_MANIFEST_URL);
-    // Clients should think of mAppManifestURL as const ... we're
-    // bending the rules here just for the preallocation hack.
-    const_cast<nsString&>(mAppManifestURL) = aAppManifestURL;
-
     return SendSetProcessPrivileges(aPrivs);
 }
 
 void
 ContentParent::ShutDownProcess()
 {
   if (!mIsDestroyed) {
     const InfallibleTArray<PIndexedDBParent*>& idbParents =
@@ -1091,17 +1066,17 @@ ContentParent::ContentParent(const nsASt
     mSubprocess = new GeckoChildProcessHost(GeckoProcessType_Content,
                                             aOSPrivileges);
 
     mSubprocess->LaunchAndWaitForProcessHandle();
 
     // Set the subprocess's priority.  We do this first because we're likely
     // /lowering/ its CPU and memory priority, which it has inherited from this
     // process.
-    SetProcessPriority(aInitialPriority);
+    SetProcessInitialPriority(aInitialPriority);
 
     Open(mSubprocess->GetChannel(), mSubprocess->GetChildProcessHandle());
 
     // NB: internally, this will send an IPC message to the child
     // process to get it to create the CompositorChild.  This
     // message goes through the regular IPC queue for this
     // channel, so delivery will happen-before any other messages
     // we send.  The CompositorChild must be created before any
--- a/dom/ipc/ContentParent.h
+++ b/dom/ipc/ContentParent.h
@@ -182,35 +182,30 @@ private:
                   ChildPrivileges aOSPrivileges = base::PRIVILEGES_DEFAULT,
                   hal::ProcessPriority aInitialPriority = hal::PROCESS_PRIORITY_FOREGROUND);
     virtual ~ContentParent();
 
     void Init();
 
     // Set the child process's priority.  Once the child starts up, it will
     // manage its own priority via the ProcessPriorityManager.
-    void SetProcessPriority(hal::ProcessPriority aInitialPriority);
+    void SetProcessInitialPriority(hal::ProcessPriority aInitialPriority);
 
     // If the frame element indicates that the child process is "critical" and
     // has a pending system message, this function acquires the CPU wake lock on
     // behalf of the child.  We'll release the lock when the system message is
     // handled or after a timeout, whichever comes first.
     void MaybeTakeCPUWakeLock(nsIDOMElement* aFrameElement);
 
-    // Set the child process's priority and then check whether the child is
-    // still alive.  Returns true if the process is still alive, and false
-    // otherwise.  If you pass a FOREGROUND* priority here, it's (hopefully)
-    // unlikely that the process will be killed after this point.
-    bool SetPriorityAndCheckIsAlive(hal::ProcessPriority aPriority);
-
     // Transform a pre-allocated app process into a "real" app
     // process, for the specified manifest URL.  If this returns false, the
     // child process has died.
     bool TransformPreallocatedIntoApp(const nsAString& aAppManifestURL,
-                                      ChildPrivileges aPrivs);
+                                      ChildPrivileges aPrivs,
+                                      hal::ProcessPriority aInitialPriority);
 
     /**
      * Mark this ContentParent as dead for the purposes of Get*().
      * This method is idempotent.
      */
     void MarkAsDead();
 
     /**