Closed
Bug 1506202
Opened 6 years ago
Closed 6 years ago
Annotate GPU process crash reports with URL
Categories
(Core :: Graphics: WebRender, enhancement, P4)
Core
Graphics: WebRender
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 | ||
Updated•6 years ago
|
Assignee | ||
Comment 1•6 years ago
|
||
(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
Assignee | ||
Comment 2•6 years ago
|
||
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.
Updated•6 years ago
|
Comment 3•6 years ago
|
||
Bug 1507686 might be related.
Comment 4•6 years ago
|
||
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)
Updated•6 years ago
|
Comment 5•6 years ago
|
||
Hm, I don't actually know offhand. Maybe gsvelto or mconley have ideas here?
Flags: needinfo?(ted)
Comment 6•6 years ago
|
||
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.
Comment 7•6 years ago
|
||
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
Comment 8•6 years ago
|
||
Dropping this to P4 for now, since we're hoping to solve the use case using bug 1510900.
Priority: P2 → P4
Comment 9•6 years ago
|
||
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.
Assignee | ||
Updated•6 years ago
|
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.
Description
•