1112828 - Unify gfxCriticalError and gfxInfo->LogFailure()

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

LogFailure() has the advantage that its messages are exposed to about:support. Otherwise, these are mostly the same.

Comment 1

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Part 1. Move functionality from CrashStatsLogForwarder (derived from LogForwarder) into a new class SpecialCircularBuffer (a base class for LogForwarder).

Comment 2

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Part 2: Have GfxInfo::LogFailure use gfxCriticalLog and entries from gfxCriticalLog be available in about:support graphics section. r=jmuizelaar

Comment 3

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8538783 [details] [diff] [review]
Part 2: Have GfxInfo::LogFailure use gfxCriticalLog and entries from gfxCriticalLog be available in about:support graphics section. r=jmuizelaar

Review of attachment 8538783 [details] [diff] [review]:
-----------------------------------------------------------------

Is gfxCriticalLog threadsafe?

Comment 4

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #3)
> ...
> Is gfxCriticalLog threadsafe?

Not really.  There are a few aspect to it, the main ones being the std::vector that we use for circular buffer (not thread safe), and the nsDataHashtable we use for the crash reporter annotation table (not sure, but I don't think so), and then there is the copying into the char*** for the about:support (was thread safe, now is not.)

Comment 5

[Milan Sreckovic [:milan] (needinfo for best results)]

I think the current version of GfxInfoBase "log failure related" code isn't thread safe - the LogFailure() method itself is, but GetFailures() method is looking at the data that LogFailure() is modifying, without locking.

Comment 6

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Have GfxInfo::LogFailure use gfxCriticalLog and entries from gfxCriticalLog be available in about:support graphics section. r=jmuizelaar

Note this needs the patch from bug 1113695 to be thread safe.

Comment 7

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Milan Sreckovic [:milan] from comment #1)
> Created attachment 8538779 [details] [diff] [review]
> Part 1. Move functionality from CrashStatsLogForwarder (derived from
> LogForwarder) into a new class SpecialCircularBuffer (a base class for
> LogForwarder).

Skipped this change (for now), the thread safety trumped it and we can't use Mutex inside of gfx/2d, it's XPCOM...

Comment 8

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Have GfxInfo::LogFailure use gfxCriticalLog and entries from gfxCriticalLog be available in about:support graphics section. r=jmuizelaar

Make the critical error not assert for LogFailure case.

Comment 9

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Have GfxInfo::LogFailure use gfxCriticalLog and entries from gfxCriticalLog be available in about:support graphics section. r=jmuizelaar

Change the formatting in about:support to group these entries.

Comment 10

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Have GfxInfo::LogFailure use gfxCriticalLog and entries from gfxCriticalLog be available in about:support graphics section. r=jmuizelaar

I'll attach what about:support looks like with this change.

Comment 11

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Screengrab of what about:support looks like with the above patch.

Comment 12

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Have GfxInfo::LogFailure use gfxCriticalLog and entries from gfxCriticalLog be available in about:support graphics section. Carry r=jmuizelaar

Rebased. https://treeherder.mozilla.org/#/jobs?repo=try&revision=64d2e116aa3b

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/09c23f8dea86

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/09c23f8dea86