Bug 836654 - Part 9: During app-frame creation, take the CPU wake lock only after sending down the mozapptype. r=cjones

This prevents the child process from downgrading its CPU priority when
it notices that it has the CPU wake lock but no "critical" frames.

In this patch we also reduce the scope of a race condition which occurs
when we don't launch a new process for an app.  In this case, we still
need to set the child process's priority and then check that the process
is still alive.

Because this isn't a brand-new process, it's possible that the process
will downgrade its priority (e.g. due to a timer) after we check that
it's still alive and before it notices that we've acquired the CPU wake
lock on its behalf.  This patch does not resolve that race.
This commit is contained in:
Justin Lebar 2013-02-14 15:41:30 -05:00
parent 4ce90cdcbc
commit 093600fa79
3 changed files with 59 additions and 37 deletions

View File

@ -2054,14 +2054,6 @@ nsFrameLoader::TryRemoteBrowser()
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);

View File

@ -267,8 +267,8 @@ ContentParent::MaybeTakePreallocatedAppProcess(const nsAString& aAppManifestURL,
return nullptr;
}
if (!process->TransformPreallocatedIntoApp(aAppManifestURL, aPrivs,
aInitialPriority)) {
if (!process->SetPriorityAndCheckIsAlive(aInitialPriority) ||
!process->TransformPreallocatedIntoApp(aAppManifestURL, aPrivs)) {
// Kill the process just in case it's not actually dead; we don't want
// to "leak" this process!
process->KillHard();
@ -492,10 +492,19 @@ ContentParent::CreateBrowserOrApp(const TabContext& aContext,
return nullptr;
}
nsRefPtr<ContentParent> p = gAppContentParents->Get(manifestURL);
if (!p) {
ProcessPriority initialPriority = GetInitialProcessPriority(aFrameElement);
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);
@ -508,14 +517,27 @@ ContentParent::CreateBrowserOrApp(const TabContext& aContext,
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);
unused << browser->SendSetAppType(appType);
}
p->MaybeTakeCPUWakeLock(aFrameElement);
return static_cast<TabParent*>(browser);
}
@ -673,14 +695,14 @@ NS_IMPL_ISUPPORTS1(SystemMessageHandledListener,
} // anonymous namespace
void
ContentParent::SetProcessInitialPriority(ProcessPriority aInitialPriority)
ContentParent::SetProcessPriority(ProcessPriority aPriority)
{
if (!Preferences::GetBool("dom.ipc.processPriorityManager.enabled")) {
return;
}
SetProcessPriority(base::GetProcId(mSubprocess->GetChildProcessHandle()),
aInitialPriority);
hal::SetProcessPriority(base::GetProcId(mSubprocess->GetChildProcessHandle()),
aPriority);
}
void
@ -708,25 +730,16 @@ ContentParent::MaybeTakeCPUWakeLock(nsIDOMElement* aFrameElement)
}
bool
ContentParent::TransformPreallocatedIntoApp(const nsAString& aAppManifestURL,
ChildPrivileges aPrivs,
ProcessPriority aInitialPriority)
ContentParent::SetPriorityAndCheckIsAlive(ProcessPriority 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;
SetProcessPriority(aPriority);
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!
// 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!
//
// It's not legal to call DidProcessCrash on Windows if the process has not
// terminated yet, so we have to skip this check there.
// terminated yet, so we have to skip this check here.
#ifndef XP_WIN
bool exited = false;
base::DidProcessCrash(&exited, mSubprocess->GetChildProcessHandle());
@ -735,6 +748,18 @@ ContentParent::TransformPreallocatedIntoApp(const nsAString& aAppManifestURL,
}
#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);
}
@ -1071,7 +1096,7 @@ ContentParent::ContentParent(const nsAString& aAppManifestURL,
// 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.
SetProcessInitialPriority(aInitialPriority);
SetProcessPriority(aInitialPriority);
Open(mSubprocess->GetChannel(), mSubprocess->GetChildProcessHandle());

View File

@ -187,7 +187,7 @@ private:
// Set the child process's priority. Once the child starts up, it will
// manage its own priority via the ProcessPriorityManager.
void SetProcessInitialPriority(hal::ProcessPriority aInitialPriority);
void SetProcessPriority(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
@ -195,12 +195,17 @@ private:
// 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,
hal::ProcessPriority aInitialPriority);
ChildPrivileges aPrivs);
/**
* Mark this ContentParent as dead for the purposes of Get*().