The cost control widget code does not handle the app crashing. It simply creates a new iframe the next time it wants to load cost control without removing the old one, potentially leaking an unbounded number of <iframe mozbrowser>s

Pointer to Github pull-request

Blocks a blocker.

(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #0) > The cost control widget code does not handle the app crashing. It simply > creates a new iframe the next time it wants to load cost control without > removing the old one, potentially leaking an unbounded number of <iframe > mozbrowser>s I think no. The condition in line 29: https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/cost_control.js#L29 prevents the code to create another new iframe. In case of error, the `killed` attribute is set so a reconfiguration is forced but not a new creation. What do you think?

Yes, I believe you are correct. There is only ever one iframe. But we can still keep that iframe around (which causes things like leaks of ContentParent objects (e.g. bug 893012)). Sorry I confused this with some of the other gaia bugs I was investigating yesterday :-)

AFAIK, the `mozbrowser` should detach the iframe from the main process and we should not have ContentParent leaks as it is impossible to communicate from one to another via JavaScript, isn't it? I'm not an expert here so asking for further clarifications. Please, explain me what kind of leak is a ContentParent leak and why we could have one? Thank you.

If you leave a crashed iframe mozbrowser in the DOM, it will hold onto the ContentParent and associated IPC machinery in the parent process.

I see. And when you reset the attributes, are not these mechanisms wiped out?

They are, but the attributes are not reset until _ensureWidget is called again. That could be some time from when the process crashes.

Hi, Salva. This is a critical bug that we need to fix as soon as possible, so Leo can continue testing devices for stability. Please let me know if you can't get to this review in the next day or two; I'm happy to find another reviewer.

Per comment 1, this is not even happening, this is only a potential risk and per comment 3 the risk is even lower. But I'm testing just right now to ensure nothing breaks and I grant the r+ as soon as possible.

Comment on attachment 779470 [details] Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/11101 Nice and easy. Your last comment convinced me. Thank you!

Pushed to master : https://github.com/mozilla-b2g/gaia/commit/8b49be437ee81aa76ac8e267afeb7d25520068fc

[v1-train 055c122] Bug 896755: Handle app crashes in the cost control widget. r=salva