New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 683377 link

Starred by 2 users

Issue metadata

Status: Duplicate
Merged: issue 813232
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug

Blocking:
issue 733312
issue 652407



Sign in to add a comment

Add BIND_ABOVE_CLIENT from OOPIF renderer -> parent renderer

Project Member Reported by agrieve@chromium.org, Jan 20 2017

Issue description

This provides a strong hint to the OS when out-of-process iframes are enabled and there is not enough RAM on the device, iframes should be killed before the top-level document.


 

Comment 1 by creis@chromium.org, Apr 4 2017

Cc: creis@chromium.org nasko@chromium.org
Components: Internals>Sandbox>SiteIsolation
Do you think you'll be able to make progress on this in the next little while?
I hesitated to respond because I wanted to say yes. My realistic guess is no though. 
Blocking: 733312
Cc: lukasza@chromium.org
"renderer X hosts a parent frame of a frame hosted by renderer Y" is a relationship that can form cycles.  Example: a(b(a)) frame structure (a-site frames hosted in one renderer and b-site frames hosted in another renderer).  Even if Android OS is resilient to circular BIND_ABOVE_CLIENT, we probably should try to avoid forming circular BIND_ABOVE_CLIENT relationships.  Maybe BIND_ABOVE_CLIENT(parent-renderer, subframe-renderer) should only be established if the parent renderer hosts the main frame?  Or only if subframe_renderer doesn't already appear on the lhs in another pair of renderers?

I assume that we not only want to "add BIND_ABOVE_CLIENT from OOPIF renderer -> parent renderer" but we also want to remove BIND_ABOVE_CLIENT if a subframe gets destroyed and/or transferred to another renderer.  I am guessing that this would be done by calling Context.unbindService(ServiceConnection).
Status: WontFix (was: Assigned)
Not sure how I missed this in my previous testing, but it looks like renderers are not allowed to bind to each other:

java.lang.SecurityException: Isolated process not allowed to call bindService

Given this, I actually don't think there's anything that can be done here :(.
Cc: boliu@chromium.org
Status: Available (was: WontFix)
Actually, we probably could still use FORE vs VIS when chrome is the active app in order to put the top frame above the iframe (and move moderate bindings to PERCEPTIBLE). There's nothing we can do once Chrome gets backgrounded though :(
RE: There's nothing we can do once Chrome gets backgrounded

Q1: Do we need to fix this?  What scenario would be broken (i.e. broken = main frame can be killed by the OS before subframes) if we don't fix this?
- I don't know if "FORE vs VIS when chrome is the active app" would still be in effect after Chrome gets backgrounded.
- OTOH, maybe there are scenarios when a new page/frames are opened while Chrome stays backgrounded.

Q2: If we need to fix this, then would this require requesting additional OS support for our scenario?  Can we ask for such OS support in the next version of the Android OS?
Cc: agrieve@chromium.org
Owner: boliu@chromium.org
Bo - I think you're now more up-to-date than myself on this topic. Do you know the answers here?

Comment 10 by boliu@chromium.org, Aug 22 2017

> Q1: Do we need to fix this?

Not really? Right now if OOPIF is enabled on android, then all processes for a particular tab will share the same "importance", so android oom killer might kill them in arbitrary order. I think the point here is maybe chrome would prefer subframes to be killed before the main frame?

Probably not super urgent because
* processes *between* tabs will still have the correct relatively importance
* there are only 3 levels of hint to android, so say lowering the visible iframe process by one level might actually be over correcting. and I'd prefer not having avoid dynamic policy (eg lower level only after everything else has been killed)
* I'm assuming we won't ever turn on oopif on low ram devices to save memory. And all of this matter a lot less on high ram devices.

> There's nothing we can do once Chrome gets backgrounded though :(

That's not actually true for all android versions/devices, but probably true for a lot of them..
RE: I'm assuming we won't ever turn on oopif on low ram devices to save memory.

We *were* [1] in fact wondering if we can turn on OOPIFs (TopDocumentIsolation) on low ram devices *to save memory*.  This might sounds counterintuitive at first, so let me try to explain the thinking process:
- Less renderers = less memory required = less risk that Android OS will start killing *important* processes.
- Some renderers (i.e. the renderer hosting OOPIF ads detected by TopDocumentIsolation) might host content that is less important than the conent in other renderers.
- If we can tell the Android OS which renderers are less important than others, then we can graciously handle low memory conditions (by accepting killing of less important renderers, while still having functional main content).

Let's look at various situations and order them from lowest to highest memory usage:
- Situation #1. Single renderer hosting just main-site content (this is what we get with TDI enabled after Android OS kills the TDI process)
- Situation #2. Single renderer hosting both main-site and cross-site content of a page (this is what we get without TDI / without OOPIFs;  since this situation uses more memory than situation #1, then it carries more risk that Android OS starts killing processes [and in particular - killing this renderer will kill the main-site content - the important content])
- Situation #3. Two renderers: Renderer hosting main-site content + TDI renderer hosting cross-site ads (this is what we get with TDI before Android OS starts killing renderers)

So - based on the above we can expect that A) turning OOPIFs/TDI on will increase memory usage (situation #3), but B) in return will allow graceful killing of renderers hosting secondary content (i.e. allowing situation #1 which can be argued is better than situation #2).

Disclaimer: before we can consider turning on TDI on low ram devices we need to not only 1) figure out a way to tell the Android OS to first kill less important renderer processes but we need to also 2) make TDI shippable (IMO the main blocker is issue 736113, although there is also other work like issue 393285).


RE: Q: Do we need to fix this?  A: Not really?

Can you clarify if you are answering:
1. If we need to fix this bug (if we need to indicate to Android OS that OOPIFs should be killed before main frames)
2. If fixing this bug requires that we need to first get extra support from Android OS (e.g. to support binding between isolated processes?  to support indicating importance of backgrounded apps/processes?)

I was trying to ask about #2 above.  Sorry if this wasn't clear.



[1] AFAIK, the original idea was floated by ojan@ in an discussion thread titled "TDI just for ads as a memory saving initiative".

Comment 12 by boliu@chromium.org, Aug 22 2017

> - Less renderers = less memory required = less risk that Android OS will start killing *important* processes.

512MB devices has trouble keeping more than one renderer alive, since need to be nice and allow the previously foreground app to run as well. That changes this argument a bit I imagine.

> 1. If we need to fix this bug (if we need to indicate to Android OS that OOPIFs should be killed before main frames)

This definitely needs to be fixed before enabling oopif *on low ram devices*

> 2. If fixing this bug requires that we need to first get extra support from Android OS (e.g. to support binding between isolated processes?  to support indicating importance of backgrounded apps/processes?)

No. we are stuck with what we have.

Comment 13 by boliu@chromium.org, Feb 16 2018

Mergedinto: 813232
Status: Duplicate (was: Available)

Sign in to add a comment