766437 - Fire an event that allows mozbrowser embedders to deduce that mozbrowser content has crashed/died

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

I'm in favor of not introducing OS processes into the "web abstractions", so if we can repurpose existing events to this end, I would be happier than with a new one like "oncrash".

Any ideas?

We need this work to allow gaia to detect when apps are lowmemkilled/die/crash.  Currently we're just left with a blank iframe, and AFAIK gaia doesn't know anything has happened.

Comment 1

[Mounir Lamouri (:mounir)]

Just throwing an idea here: can't we change the location to "about:crashed" which will fire a "locationchange" event so the embedder will be able to check the new url?

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

We can't navigate mozapp frames.

Comment 3

[Justin Lebar (not reading bugmail)]

(In reply to Chris Jones [:cjones] [:warhammer] from comment #2)
> We can't navigate mozapp frames.

Maybe you mean something different by "we" than Mounir does.

If the process hadn't just crashed, we can navigate the mozapp frame wherever we want.  There are some same-origin restrictions, but we could (and probably would need to) relax those for about: pages.

Since the process has crashed, we can't navigate the frame anywhere without either starting up a new content process (probably a bad idea) or switching to an in-process browser frame (possibly difficult).

We could fire a fake locationchange to "about:crashed", so long as pages (apps and browsers) couldn't navigate themselves to that location...

Comment 4

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Can we not just reuse something like onunload?  I feel like we're trying to get too cute here.

Comment 5

[Justin Lebar (not reading bugmail)]

(In reply to Chris Jones [:cjones] [:warhammer] from comment #4)
> Can we not just reuse something like onunload?  I feel like we're trying to
> get too cute here.

If I have an in-process iframe, iframe.onload fires when a new page loads into the iframe.  It seems to me that using iframe.onunload for "this iframe crashed" is not at all the same sort of thing.

Comment 6

[Mounir Lamouri (:mounir)]

(In reply to Justin Lebar [:jlebar] from comment #3)
> We could fire a fake locationchange to "about:crashed", so long as pages
> (apps and browsers) couldn't navigate themselves to that location...

AFAIK, pages can't navigate themselves to about: pages.
So, I'm not sure why we don't go for that solution?

Comment 7

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

David, Ben, suppose a "window" in the window manager crashes, or a "tab" in the browser crashes.  What would be the most convenient way to notify you?

(I tried to CC Dale but I can't seem to find his account in bz.)

Comment 8

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

djf and I discussed this for a bit on IRC.  Another issue with crashed content is that after it crashes, the pixels in the iframe are basically "undefined".  (Should normally be blank, but I'm not 100% sure.  Probably whatever the iframe background style is.)

So I propose something along the lines of what Mounir suggested
 - let the mozbrowser embedder set a "fallback URI", or "error URI" (whatever, name TBD)
 - when content crashes, gecko is notified asap (not instantly, but soon)
 - gecko navigates the iframe (in-process) to the fallback URI.  The "app" loses its old security principal and gets a totally untrusted one.
 - the mozbrowser embedder is notified through the normal locationchange event

There are two use cases I have in mind.  The first is, the embedder sets the fallback URI to something like "about:blank?nonce=[randomnumber]" and stores the random number on the iframe element.  When there's a location change to "about:blank?nonce=[randomnumber]", the embedder determines there was a crash and kills the window.

Second case is, embedder sets the fallback URI to something like "https://embedder/reportcrash.html?uri=[content URI]".  When there's a location change, the embedder does nothing.  The user has the option of reporting the crash, or just closing the tab/app.

wdyt?

Comment 9

[Dale Harvey (:daleharvey)]

I dont think there is any problem with either locationchange or unload in the embedder, unload seems to make the most sense to me, but locationchange would be fine

One thing I dont understand if we dont start a new content process, we will want to be changing the page contents to some "page has crashed" page, and the user will expect to be able to navigate onwards, although we could just replace with a new tab in the browser, (I would expect mozapp to just close)

Comment 10

[Ben Francis [:benfrancis]]

A mozbrowsercrashed/mozappcrashed event on the iframe would be the obvious answer but like you say, that introduces operating system concepts into the web layer which is probably wrong.

I actually quite like the idea of a mozbrowserlocation changed event to about:crashed because it doesn't break the abstraction.

Alternatively you could fire an alert event, informing the user that the page has crashed but that's maybe a little clunky.

Comment 11

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

"about:crashed" or $GECKO_URI_FOO is not a path to a standardizable API.  It's just more gecko magic to build into the clients.

Comment 12

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

But to be perfectly honest, I'm still happy with firing some event that means "crashed" but doesn't adopt the OS-process terminology.

Comment 13

[Justin Lebar (not reading bugmail)]

From an implementation point of view, replacing the OOP iframe with an in-process iframe is tricky.

My preference would be to simplify the approach in comment 8 to the following:

 - When the content process crashes, we fire a locationchange "about:crashed" (or whatever) event and blank the iframe's contents.
 - The iframe is now in a "crashed" state and rejects or ignores all further attempts to interact with it.
 - The embedder now can handle things as it pleases.  It can show a custom reportcrash page in a new non-remote iframe, it can give the user the option of trying to recover by creating a new remote iframe, it can close the app...whatever.

This is simpler to implement and is more flexible than the approach in comment 8.  (Not all implementations will want to replace the remote iframe with an in-process one, for example.)

Comment 14

[Justin Lebar (not reading bugmail)]

(In reply to Chris Jones [:cjones] [:warhammer] from comment #11)
> "about:crashed" or $GECKO_URI_FOO is not a path to a standardizable API. 
> It's just more gecko magic to build into the clients.

If you don't want "about:crashed", then let's fire a new event.  I think asking the embedder to specify a fallback URL would be a bad API.  For example, what do we do if the embedder doesn't specify this URL?  And again, actually loading this URL in process but inside the previously-oop iframe is complicated.

Comment 15

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

onfail

Objections?

Comment 16

[Justin Lebar (not reading bugmail)]

It would be "mozbrowserfail", like all the rest of the mozbrowser events, but "fail" is OK with me.

Comment 17

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

mozbrowserfail

Objections?

Comment 18

[Mounir Lamouri (:mounir)]

If we really want to use that event for crashes and only crashes, why not simply naming it 'mozbrowsercrash'?

Comment 19

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

We don't want to use it for crashes and only crashes.

Comment 20

[Ben Francis [:benfrancis]]

No objections.

http://people.mozilla.com/~bfrancis/mozbrowserfail.png

Comment 21

[Justin Lebar (not reading bugmail)]

Not to re-open this can of worms, but I think we should combine this with bug 768842 (notify when we would otherwise show Gecko error pages).

Comment 22

[Justin Lebar (not reading bugmail)]

Marking as depends on bug 768842.  That bug adds a 'mozbrowsererror' event.  That event's detail has the form

 {type: TYPE, ... other stuff if applicable}.

For example, we could (but don't currently) send {type: neterror, url: http://does.not.exist}.

So I propose that for a crash, we send mozbrowsererror {type: fatal}.

Comment 23

[Justin Lebar (not reading bugmail)]

Attachment: Patch, v1

Comment 24

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 648085 [details] [diff] [review]
Patch, v1

I really like it when patches are as simple as you hope they'd be.

Comment 25

[Justin Lebar (not reading bugmail)]

FYI, the way I tested this was to call ContentChild::QuickExit() on a timer 10 seconds after ContentChild::InitXPCOM.  But I was not determined enough to write an automated test.

Comment 26

[Justin Lebar (not reading bugmail)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/719b1d10b554

Comment 27

[Justin Lebar (not reading bugmail)]

(In reply to Chris Jones [:cjones] [:warhammer] from comment #24)
> I really like it when patches are as simple as you hope they'd be.

The hardest bug I had here turned out to be a merge conflict.  :)

Comment 28

[Justin Lebar (not reading bugmail)]

https://github.com/mozilla-b2g/gaia/issues/3125

Comment 29

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/719b1d10b554