1235437 - Keep a crash annotation for major preferences

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

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

Make it easier to figure out if the users have preferences set that are forcing a non-default behaviour.

Comment 1

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

Attachment: Have an annotation that records backend altering preferences. r=bgirard

Comment 2

[Benoit Girard (:BenWa)]

Comment on attachment 8702371 [details] [diff] [review]
Have an annotation that records backend altering preferences. r=bgirard

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

::: gfx/src/gfxCrashReporterUtils.cpp
@@ +125,5 @@
> +void
> +ScopedGfxFeatureReporter::AppNote(const nsACString& aMessage)
> +{
> +  nsCOMPtr<nsIRunnable> r = new AppendAppNotesRunnable(aMessage);
> +  NS_DispatchToMainThread(r);

Not related to your patch but I wonder if we're already on the main thread if this would queue the app note runnable at the back of the event queue. This means that the following code on the main thread would not get an app note:

AppNote("SomethingUseful+");
abort();

instead if we executed the runnable immediately if we're on the main thread then it would be included.

@@ +127,5 @@
> +{
> +  nsCOMPtr<nsIRunnable> r = new AppendAppNotesRunnable(aMessage);
> +  NS_DispatchToMainThread(r);
> +}
> +  

extra whitespace here and below.

::: gfx/thebes/gfxPlatform.cpp
@@ +517,5 @@
> +    // but that any new ones should be just appended, to make the interpretation
> +    // of a cryptic string like FP(00001010001010010010101011) easier.
> +    {
> +      nsAutoCString forcedPrefs;
> +      forcedPrefs.AppendPrintf("FP(%d%d%d%d%d%d%d%d-",

I don't understand why this is broken up into 3 parts, separated by '-'. Also it seems hat the placement of '-' is arbitrary. Why not use it to delimit the layers and the WebGL preferences?

Comment 3

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

Attachment: MozReview Request: Bug 1235437: Add annotation for major graphics preferences. r?bgirard

Review commit: https://reviewboard.mozilla.org/r/31265/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/31265/

Comment 4

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

Comment on attachment 8709113 [details]
MozReview Request: Bug 1235437: Add annotation for major graphics preferences. r?bgirard

I put the wrong name in the reviewboard.

Comment 5

[Benoit Girard (:BenWa)]

Comment on attachment 8709113 [details]
MozReview Request: Bug 1235437: Add annotation for major graphics preferences. r?bgirard

https://reviewboard.mozilla.org/r/31265/#review29201

Comment 6

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

Comment on attachment 8709113 [details]
MozReview Request: Bug 1235437: Add annotation for major graphics preferences. r?bgirard

https://reviewboard.mozilla.org/r/31265/#review29379

Comment 7

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d9cac9f0a128

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9ac0b31bf6bb

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9ac0b31bf6bb

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9ac0b31bf6bb