Last Comment Bug 766437 - Fire an event that allows mozbrowser embedders to deduce that mozbrowser content has crashed/died
: Fire an event that allows mozbrowser embedders to deduce that mozbrowser cont...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla17
Assigned To: Justin Lebar (not reading bugmail)
:
Mentors:
Depends on: 768842
Blocks: browser-api b2g-e10s-work 768832
  Show dependency treegraph
 
Reported: 2012-06-19 19:48 PDT by Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
Modified: 2012-08-02 19:09 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+


Attachments
Patch, v1 (3.97 KB, patch)
2012-08-01 14:27 PDT, Justin Lebar (not reading bugmail)
cjones.bugs: review+
21: review+
Details | Diff | Review

Description Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-06-19 19:48:27 PDT
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) 2012-06-20 01:43:58 PDT
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 2012-06-20 11:23:05 PDT
We can't navigate mozapp frames.
Comment 3 Justin Lebar (not reading bugmail) 2012-06-20 11:30:30 PDT
(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 2012-06-20 13:08:09 PDT
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) 2012-06-20 13:25:18 PDT
(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) 2012-06-21 10:49:05 PDT
(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 2012-06-21 12:40:26 PDT
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 2012-06-21 13:07:15 PDT
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) 2012-06-21 13:12:24 PDT
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] 2012-06-21 13:14:50 PDT
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 2012-06-21 13:16:51 PDT
"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 2012-06-21 13:18:18 PDT
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) 2012-06-21 13:20:03 PDT
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) 2012-06-21 13:22:31 PDT
(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 2012-06-21 14:11:57 PDT
onfail

Objections?
Comment 16 Justin Lebar (not reading bugmail) 2012-06-21 14:16:54 PDT
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 2012-06-21 14:26:08 PDT
mozbrowserfail

Objections?
Comment 18 Mounir Lamouri (:mounir) 2012-06-21 14:33:52 PDT
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 2012-06-21 15:00:07 PDT
We don't want to use it for crashes and only crashes.
Comment 20 Ben Francis [:benfrancis] 2012-06-22 05:06:46 PDT
No objections.

http://people.mozilla.com/~bfrancis/mozbrowserfail.png
Comment 21 Justin Lebar (not reading bugmail) 2012-06-27 05:44:20 PDT
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) 2012-06-28 03:26:24 PDT
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) 2012-08-01 14:27:26 PDT
Created attachment 648085 [details] [diff] [review]
Patch, v1
Comment 24 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-01 23:25:00 PDT
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) 2012-08-02 07:24:11 PDT
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) 2012-08-02 07:27:33 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/719b1d10b554
Comment 27 Justin Lebar (not reading bugmail) 2012-08-02 08:03:33 PDT
(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) 2012-08-02 11:39:39 PDT
https://github.com/mozilla-b2g/gaia/issues/3125
Comment 29 Ryan VanderMeulen [:RyanVM] 2012-08-02 19:09:26 PDT
https://hg.mozilla.org/mozilla-central/rev/719b1d10b554

Note You need to log in before you can comment on or make changes to this bug.