Closed
Bug 1152331
Opened 8 years ago
Closed 8 years ago
gfxCriticalError/LogFailure interferes with parts of about:support graphics
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla40
People
(Reporter: milan, Assigned: milan)
References
(Blocks 1 open bug)
Details
(Whiteboard: gfx-noted)
Attachments
(1 file, 2 obsolete files)
1.40 KB,
patch
|
dvander
:
review+
Sylvestre
:
approval-mozilla-aurora+
Sylvestre
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
If LogFailure or any other gfxCriticalError fires, we get the message in graphics section of about support; however, we lose parts of the standard information in that section, like the graphics card, WebGL info, etc. This was bad from the time this landed with bug 1112828.
Assignee | ||
Comment 1•8 years ago
|
||
Somebody that understands JS/C++ interaction better can explain the underlying mechanism, but it seems unsafe to keep a .value of a returned IPDL array, as it may lead to a memory corruption. David, if you understand this, great, you can review it and tell me if it makes sense; if you don't, any suggestions as to who a good person for it would be?
Attachment #8589646 -
Flags: review?(dvander)
Assignee | ||
Updated•8 years ago
|
Whiteboard: gfx-noted
Assignee | ||
Comment 2•8 years ago
|
||
Try as a sanity check. https://treeherder.mozilla.org/#/jobs?repo=try&revision=86341f71427e
Assignee | ||
Updated•8 years ago
|
Attachment #8589646 -
Flags: review?(dvander)
Assignee | ||
Comment 3•8 years ago
|
||
I want to take a look at this a bit more.
Assignee | ||
Updated•8 years ago
|
Attachment #8589646 -
Attachment is obsolete: true
Assignee | ||
Comment 4•8 years ago
|
||
Attachment #8589927 -
Flags: review?(dvander)
Assignee | ||
Comment 5•8 years ago
|
||
Attachment #8589927 -
Attachment is obsolete: true
Attachment #8589927 -
Flags: review?(dvander)
Attachment #8590286 -
Flags: review?(dvander)
Assignee | ||
Comment 6•8 years ago
|
||
Planning on asking for the uplift of this to 38 & 39, so review with extra care :)
Comment on attachment 8590286 [details] [diff] [review] Leaving data.indices around breaks the display of the rest of data. r=dvander Review of attachment 8590286 [details] [diff] [review]: ----------------------------------------------------------------- Looks right to me. This file is pretty crazy. It would have been a lot simpler to copy the properties we want than create a prototype chain and delete the ones we don't... but alas.
Attachment #8590286 -
Flags: review?(dvander) → review+
Assignee | ||
Comment 8•8 years ago
|
||
(In reply to David Anderson [:dvander] from comment #7) > Looks right to me. This file is pretty crazy. It would have been a lot > simpler to copy the properties we want than create a prototype chain and > delete the ones we don't... but alas. Agreed. Filed one follow up bug to start, but wanted this patch simple enough that we could uplift it if approved.
Assignee | ||
Comment 9•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=185dce5a2a97
Keywords: checkin-needed
Assignee | ||
Comment 10•8 years ago
|
||
Comment on attachment 8590286 [details] [diff] [review] Leaving data.indices around breaks the display of the rest of data. r=dvander Approval Request Comment [Feature/regressing bug #]: 1112828 (in 38) [User impact if declined]: If a graphics error is encountered about:support will contain it, but not contain the standard information about the user's system. This will give us less useful information from the users that do encounter this problem, so all the direct impact is on us, not getting the correct data. Note this does not affect the crash reports or telemetry, just the info displayed in about:support. [Describe test coverage new/current, TreeHerder]: n/a [Risks and why]: Low to none. We are removing the field that we were not supposed to be processing down the line, nor do we process in the scenario where there are no errors to be reported. [String/UUID change made/needed]: n/a
Attachment #8590286 -
Flags: approval-mozilla-beta?
Attachment #8590286 -
Flags: approval-mozilla-aurora?
Comment 11•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/45d577adc4b0
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/45d577adc4b0
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Updated•8 years ago
|
status-firefox38:
--- → affected
status-firefox39:
--- → affected
Comment 13•8 years ago
|
||
Comment on attachment 8590286 [details] [diff] [review] Leaving data.indices around breaks the display of the rest of data. r=dvander Should be in 38 beta 5
Attachment #8590286 -
Flags: approval-mozilla-beta?
Attachment #8590286 -
Flags: approval-mozilla-beta+
Attachment #8590286 -
Flags: approval-mozilla-aurora?
Attachment #8590286 -
Flags: approval-mozilla-aurora+
You need to log in
before you can comment on or make changes to this bug.
Description
•