For now, let's assume we have 1 content process allocated to "browser tabs", and ∞ allocated to "web apps". Then the way we should allocate processes is - if it's "mozapp", launch a new process - else, if it's "mozbrowser", launch the "browser tabs" process if it doesn't exist, and put the frame in the "browser tabs" process (We have to check in this order because current "mozapp" iframes are also "mozbrowser", as an implementation detail.) We want one OS process per mozapp process for a variety of reasons around security and responsiveness. In Gonk, we can get away with this willy-nilly allocation because we can rely on the kernel lowmemkiller to gun down background processes if memory gets tight. We can allocate "browser tabs" however we want, but we don't have much reason to do something different yet.

> In Gonk, we can get away with this willy-nilly allocation because we can rely on the kernel > lowmemkiller to gun down background processes if memory gets tight. Do we have any evidence that the oomkiller will kill the process we want in this case? AIUI if we launch a new process which eats up a lot of RAM, the oomkiller will shoot *it*. That's precisely not the behavior we want. http://linux-mm.org/OOM_Killer Maybe if we immunized important processes from being killed (/proc/<pid>/oomadj set to OOM_DISABLE), we could get away with this as described here.

You're thinking of the linux OOM killer. I'm talking about the android lowmemkiller driver, which we can program to suit our needs.

Before you ask, https://bugzilla.mozilla.org/show_bug.cgi?id=744515#c4 ;)

how to control lowmemkiller is different between GB and ICS. ICS code's comment could help to understand how to use it. - until GB: lowmemkiller setting code is with in init.rc. [1] - ICS : the setting code is within ProcessList.java [2] [1] http://androidxref.com/2.3.6/xref/system/core/rootdir/init.rc#212 [2] http://androidxref.com/4.0.4/xref/frameworks/base/services/java/com/android/server/am/ProcessList.java

jlebar, you're not working on this, right?

(In reply to Chris Jones [:cjones] [:warhammer] from comment #5) > jlebar, you're not working on this, right? I haven't looked at this bug, but I can. It's simple to do, but it seems that there's actually some heuristics involved here. In particular, we /could/ have more than one process for our browser tabs, so long as all tabs that can touch each other are in the same process. That's how things work at the moment (in theory!). Anyway, it's not clear to me that the correct way to tune this to put all browser tabs in the same process, so we could wait to tune this until we have more things running OOP and we have a better idea of the trade-offs involved in having lots of content processes.

This isn't strictly a browser-api issue, but marking it as blocking that bug so I don't lose track of it. Anyway the main change will be for specifically browser tabs.

The critically important part of this work is process-per-mozapp. To a first approximation, at the moment, I don't care how we allocate processes to mozbrowser-!mozapp. One process for all of them is fine by me.

> The critically important part of this work is process-per-mozapp. I haven't been working on that, modulo what's already done. Is there more work to do there? I guess right now we have some arbitrary process limit and allocate randomly within it.

Yes, in nsFrameLoader, when we ask for a process, we need to know whether the process is going to be used for a mozapp or !mozapp frame. If mozapp, we need to force ContentParent to give us a freshly allocated process. We can keep the current arbitrary limit alive for !mozapp frames, but like I said I don't care about that too much right now.

With the work being done for the security model, it should be quite easy to get all those information (DocShell and Principal should carry enough data for that). Maybe we could wait for that? Instead of hurrying up a temporary solution that will need to be fixed?

To the best of my knowledge, we're not missing any information we need to finish the work here.

Indeed, we might be able to handle that currently but we might have to re-write that later to use other ways. The same way nsIDOMWindowUtils.setApp() is very likely going to be removed in favor of using Principal. I would prefer not to add to many things that will need to be rewritten.

What's happening is, the information of "is this docshell/frame" a mozbrowser/mozapp is all moving around as part of Mounir's patches. We have the information, for sure, but mounir needs to figure out where it's going to live.

We need the patch here very urgently. I don't see any value in blocking this on moving some things around.

The value is that it's less work for mounir when he goes to move things around, which means that we get his permission work quicker. But it's not worth arguing about; I'll write this patch this afternoon.

Yes, but the work here is pretty much the simplest possible use of mozapp/mozbrowser bits. I don't expect this to create nontrivial "porting" effort.

The handling of these lists (which are deleted when they become empty) is pretty ugly. I have a plan to fix it, but since this is urgent, I don't think we should block here.

Comment on attachment 641075 [details] [diff] [review] Part 1: IPC changes. We need to shut down "app" subprocesses when the app dies, and we would like to be able to use the isApp bit to make some decisions in the subprocess, but I'm ok doing that in followups.

> The handling of these lists (which are deleted when they become empty) is > pretty ugly. I have a plan to fix it This is bug 772987.

Comment on attachment 641076 [details] [diff] [review] Part 2: Content/DOM changes. Review of attachment 641076 [details] [diff] [review]: ----------------------------------------------------------------- r=me ::: content/html/content/src/nsGenericHTMLFrameElement.cpp @@ +330,5 @@ > + // TODO: We surely need a permissions check here, particularly once we no > + // longer rely on the mozbrowser permission check. > + > + nsCOMPtr<nsIAppsService> appsService = do_GetService(APPS_SERVICE_CONTRACTID); > + NS_ENSURE_STATE(appsService); nit: NS_ENSURE_STATE() is evil, we should use more explicit macros. @@ +333,5 @@ > + nsCOMPtr<nsIAppsService> appsService = do_GetService(APPS_SERVICE_CONTRACTID); > + NS_ENSURE_STATE(appsService); > + > + nsAutoString manifestURL; > + GetAttr(kNameSpaceID_None, nsGkAtoms::mozapp, manifestURL); You could do an early check way before with HasAttr(). Also, you could at least check if manifestURL is the empty string. In that case, there is no need bothering the service.

Actually, it would be quite easy to shut down "app subprocesses" when their content is closed, so now I'm thinking I want to do that here.

(In reply to Chris Jones [:cjones] [:warhammer] from comment #23) > Actually, it would be quite easy to shut down "app subprocesses" when their > content is closed, so now I'm thinking I want to do that here. Up to you; I can land this now, or I can figure out killing the subprocesses on Monday.

Land. I'll probably be able to look this weekend. (Simple in theory but might get hairy.)

https://hg.mozilla.org/mozilla-central/rev/d3212385b1af https://hg.mozilla.org/mozilla-central/rev/9ddc7f3d8271