When content crashes, we should provide the URL annotation as we do for chrome-process crashes. Obviously not always accurate, but I think we typically use the most-recently-loaded URL, or maybe the currently-displayed-tab.

I don't know whether this needs to hard-block fennec 4.0, but it would certainly make analysis easier.

I'm presuming this relies on content processes being able to make annotations that get reporter with the crash.

No, I don't think it does, we can annotate from the chrome process, since it knows what URLs are being loaded.

Is this the same as bug 654312?

Yes, but that bug is a hacky solution, this bug is for the clean solution once bug 581341 gets fixed.

Comment 3 seems to disagree; what's the clean solution you have in mind?

Something other than what that patch did, certainly.

I don't necessarily think that I'm the best choice to do this work, but it should be done soon.

Mass tracking-e10s flag change. Filter bugmail on "2be0fcce-e36a-4e2c-aa80-0e3d33eb5406".

Am I right in presuming this might have been fixed by bug 821710? I'm seeing this in TabChild::RecvLoadURL: CrashReporter::AnnotateCrashReport(NS_LITERAL_CSTRING("URL"), uri); That looks like the right thing there. Did I just get a freebie?

(In reply to Mike Conley (:mconley) from comment #10) > That looks like the right thing there. Did I just get a freebie? So the answer to this is "no" - I just intentionally triggered a crash here on Bugzilla, and the crash report said the URL was "about:blank". No free lunch today.

Comment on attachment 8424334 [details] [diff] [review] Content processes should annotate URLs for crashes. r=? Ok, this seems to work. I triggered a crash locally, and when I examined the .extra file in my crash report, I saw the URL I was on in my remote browser.

Two questions: I wonder why the code mentioned in comment 10 is not being useful for this.. Any ideas? And there's something to think about: with this patch the crash report will be annotated with the last loaded URL. Which on principle sounds useful, but on normal builds we annotate the crash with the URL from the current active tab, AFAIK. I guess it depends on what we think is more likely to happen: a crash due to a page being interacted with, or a crash due to a page being just loaded. Since even in the latter case it's likely that the page just loaded is also in the foreground, I think that sounds more useful. As you are also working on bug 585780 maybe that will provide an easy hook to do this too.

(In reply to :Felipe Gomes from comment #14) > And there's something to think about: with this patch the crash report will > be annotated with the last loaded URL. Which on principle sounds useful, but > on normal builds we annotate the crash with the URL from the current active > tab, AFAIK. > I guess it depends on what we think is more likely to happen: a crash due to > a page being interacted with, or a crash due to a page being just loaded. Benjamin, what is your take on this? We usually look at the URLs mostly to find reproducible cases for a crash, so IMHO we should report what we think is most useful for that.

We're guessing, and we don't really know what the best guess is. Back when people loaded webpages, whatever loaded last was probably the best guess. Nowadays with highly interactive apps, whatever they are currently looking at is probably the best guess. With multiple content processes we'll be able to narrow this down further, but until then let's do the simple thing.

(In reply to Benjamin Smedberg [:bsmedberg] from comment #16) > We're guessing, and we don't really know what the best guess is. Back when > people loaded webpages, whatever loaded last was probably the best guess. > Nowadays with highly interactive apps, whatever they are currently looking > at is probably the best guess. > > With multiple content processes we'll be able to narrow this down further, > but until then let's do the simple thing. I'll assume the "simple thing" is to go with my current approach, which is to annotate using the most recently requested URI received by the content process.

I really don't understand why the TabChild code doesn't give us this already.

(In reply to Josh Matthews [:jdm] from comment #18) > I really don't understand why the TabChild code doesn't give us this already. Yeah, I'm digging into that - I should have an answer shortly.

Strangely, it appears that TabParent::LoadURL is never called when just casually browsing about using e10s tabs. The only time it appears to be called is when opening new e10s windows, via nsFrameLoader::ReallyStartLoadingInternal. General web navigation appears to go through RemoteWebNavigation.jsm instead. "WebNavigation:LoadURI" is sent through the message manager to the browser, and browser-child.js handles it. That appears to be stuff that evilpie worked on, so over to him - Tom - do you know why web navigation in e10s tabs doesn't use TabParent::LoadURL?

Ok, I just spoke to evilpie. From what he recalls, going through RemoteWebNavigation.jsm and using sendMessage was simply more convenient (he just had to write something that implemented nsIWebNavigation for remote browsers), and then browsing would "just work". It also appears that there's some B2G / Appy stuff going on in TabParent::LoadURL that RemoteWebNavigation.jsm avoids. So I guess that's why we can't just count on TabParent::LoadURL here.

Comment on attachment 8424334 [details] [diff] [review] Content processes should annotate URLs for crashes. r=? With the above information, are we good to give this another shot?

Comment on attachment 8424334 [details] [diff] [review] Content processes should annotate URLs for crashes. r=? Review of attachment 8424334 [details] [diff] [review]: ----------------------------------------------------------------- Yep, we're clear