Closed Bug 1506202 Opened 6 years ago Closed 6 years ago

Annotate GPU process crash reports with URL

Categories

(Core :: Graphics: WebRender, enhancement, P4)

enhancement

Tracking

()

RESOLVED DUPLICATE of bug 1510900
Tracking Status
firefox65 --- affected

People

(Reporter: aosmond, Assigned: aosmond)

References

Details

The GPU process crash reports appear to never contain the URL the user last browsed to, like the content and/or main processes do. This would be useful information to help diagnose WebRender crashes. We appear to set it for the content process here:

https://searchfox.org/mozilla-central/rev/a3894a4dcfb5d42f2e6eee6cb9faf7141879ef1a/dom/ipc/TabChild.cpp#1113
Assignee: nobody → aosmond
Priority: -- → P2
(In reply to Andrew Osmond [:aosmond] from comment #0)
> The GPU process crash reports appear to never contain the URL the user last
> browsed to, like the content and/or main processes do. This would be useful
> information to help diagnose WebRender crashes. We appear to set it for the
> content process here:
> 
> https://searchfox.org/mozilla-central/rev/
> a3894a4dcfb5d42f2e6eee6cb9faf7141879ef1a/dom/ipc/TabChild.cpp#1113

Turns out that may have just worked for Firefox OS. There is also:

https://searchfox.org/mozilla-central/rev/a3894a4dcfb5d42f2e6eee6cb9faf7141879ef1a/browser/base/content/browser.js#4883
One thing I had completely ignored was how the consent for crash URLs work. There is a pref, but it seems to be mostly for the default in the form, rather than something we can rely upon to shove the URL in the report. It should be feasible to open a new tab instead of reloading the original tab (with some work in browser/modules/ContentCrashHandlers.jsm) to request, but this is a somewhat radical departure from how things operate today. The best we might be able to do is include the URL if the user has indicated we should auto submit reports. I think at this point it is looking like too much work for me to feel comfortable leaving this as a P2/wr-stage-trains. For now, I'll put it against wr-stage-next.
Blocks: stage-wr-stages
No longer blocks: stage-wr-trains
Blocks: stage-wr-next
No longer blocks: stage-wr-stages
Bug 1507686  might be related.
Having this would be super useful, especially since we're moving most of gfx's work into the GPU process.

I had a quick look, and I think copying the TabChild code would work, but we still have the permissions issue.

Ted, do you know who we should talk to about how to best handle permissions for sharing URLs from the GPU process?
Flags: needinfo?(ted)
Blocks: stage-wr-trains
No longer blocks: stage-wr-next
Hm, I don't actually know offhand. Maybe gsvelto or mconley have ideas here?
Flags: needinfo?(ted)
The way the URL annotation works is kinda magic. What happens is that when a content process crash report is assembled the URL value is taken from the *main* process disregarding whatever URL the content process had written in its .extra file.
I did a bit of research and from what I can tell the annotation being added at [1] is not being used in crash reports at all, not even for content process crashes. That annotation is overwritten by the ones taken in [2] and [3]. Fun fact, it appears that we overwrite the annotation with the same value _multiple times_ per navigation event which is actually pretty bad because we rewrite the .extra file every time we do that.

So back on topic, I have to reproduce how GPU crash reports are being submitted to figure out why they don't have a URL entry.

[1] https://searchfox.org/mozilla-central/rev/a3894a4dcfb5d42f2e6eee6cb9faf7141879ef1a/dom/ipc/TabChild.cpp#1113
[2] https://searchfox.org/mozilla-central/rev/3fdc51e66c9487b39220ad58dcee275fca070ccd/browser/base/content/browser.js#4875
[3] https://searchfox.org/mozilla-central/rev/3fdc51e66c9487b39220ad58dcee275fca070ccd/toolkit/actors/WebNavigationChild.jsm#98
Dropping this to P4 for now, since we're hoping to solve the use case using bug 1510900.
Priority: P2 → P4
See Also: → 1510900
See bug 1510900 comment 2 for a potential way of doing this. I might have enough time to cook up a patch tomorrow implementing what I described there.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.