Closed
Bug 666748
Opened 13 years ago
Closed 13 years ago
Replace ContentParent::GetSingleton to allow for multiple content processes
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla9
People
(Reporter: benjamin, Assigned: benjamin)
References
Details
Attachments
(1 file, 2 obsolete files)
We want multiple content processes in desktop Firefox (and probably mobile as well, if the memory tradeoffs are good). This means we need to replace ContentParent::GetSingleton with a function which can hand out content processes as necessary. We'll obviously want to experiment with a pool, process-per-tab, and other policies, but for now let's start with something simple like a pref to govern how many content processes are created and then hand out tabs randomly to processes. Fennec will of course default to one process for now.
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → benjamin
Assignee | ||
Comment 1•13 years ago
|
||
This patch needs message-manager love.
Comment 2•13 years ago
|
||
How much is there still to do? Could we land even the partial patch to e10s repo, but so that only one process is actually used?
Assignee | ||
Comment 3•13 years ago
|
||
Smaug dealt with the message manager bits on top of this patch, so this should be ready for review.
Attachment #544235 -
Attachment is obsolete: true
Attachment #548257 -
Flags: review?(josh)
Comment 4•13 years ago
|
||
Comment on attachment 548257 [details] [diff] [review] Remove ContentParent::GetSingleton ready for review, rev. 2 Review of attachment 548257 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me, modulo the minor comments and one major concern. ::: content/base/src/nsFrameMessageManager.cpp @@ -779,4 +779,5 @@ > > bool SendAsyncMessageToChildProcess(void* aCallbackData, > > const nsAString& aMessage, > > const nsAString& aJSON) > > { > > +#if 0 // Need code for multiple content processes I can't find the bug with smaug's work in it right now. Does it subsume this block entirely, and has it already landed? This will break mobile as-is. ::: dom/ipc/ContentParent.cpp @@ -161,7 +163,9 @@ > > - if (gSingleton && !gSingleton->IsAlive()) > > + if (!gContentParents) > > - gSingleton = nsnull; > > + gContentParents = new nsTArray<ContentParent*>(); NaN more ... > > + PRInt32 maxContentProcesses = Preferences::GetInt("dom.ipc.processCount", 1); > > - nsRefPtr<ContentParent> parent = new ContentParent(); > > + if (maxContentProcesses < 1) > > - gSingleton = parent; NaN more ... PK11_GenerateRandom, please. rand() is not cryptogaphically sound. (kidding!) @@ -367,3 +396,3 @@ > > NS_ASSERTION(NS_IsMainThread(), "Wrong thread!"); > > - //If the previous content process has died, a new one could have > > + if (gContentParents) { > > - //been started since. > > + gContentParents->RemoveElement(this); This shouldn't be necessary, since we remove ourselves in ActorDestroy. @@ -654,4 +682,4 @@ > > return NS_ERROR_NOT_AVAILABLE; > > } > > else if (!strcmp(aTopic, "child-memory-reporter-request")) { > > - SendPMemoryReportRequestConstructor(); > > + unused << SendPMemoryReportRequestConstructor(); There's a bug filed about fixing this the right way (ie. we actually want to take some action if it fails). Just revert this hunk for now, please.
Attachment #548257 -
Flags: review?(josh) → review+
Comment 5•13 years ago
|
||
Comment on attachment 548257 [details] [diff] [review] Remove ContentParent::GetSingleton ready for review, rev. 2 >diff --git a/widget/src/android/nsWindow.cpp b/widget/src/android/nsWindow.cpp >--- a/widget/src/android/nsWindow.cpp >+++ b/widget/src/android/nsWindow.cpp >+ nsTArray<ContentParent*> cplist; >+ ContentParent::GetAll(); Er, I do think you want to pass something to GetAll.
Assignee | ||
Comment 6•13 years ago
|
||
Attachment #548257 -
Attachment is obsolete: true
Assignee | ||
Comment 7•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/b764f55dd37d
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•