gfxCriticalError/LogFailure interferes with parts of about:support graphics

RESOLVED FIXED in Firefox 38

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: milan, Assigned: milan)

Tracking

(Blocks 1 bug)

unspecified
mozilla40
x86
All
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox38 fixed, firefox39 fixed, firefox40 fixed)

Details

(Whiteboard: gfx-noted)

Attachments

(1 attachment, 2 obsolete attachments)

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: nobody → milan
Blocks: 1112828
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)
Whiteboard: gfx-noted
Attachment #8589646 - Flags: review?(dvander)
I want to take a look at this a bit more.
Attachment #8589646 - Attachment is obsolete: true
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+
(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.
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?
https://hg.mozilla.org/mozilla-central/rev/45d577adc4b0
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
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+

Updated

4 years ago
Duplicate of this bug: 1144389
You need to log in before you can comment on or make changes to this bug.