Closed Bug 1235437 Opened 4 years ago Closed 4 years ago

Keep a crash annotation for major preferences

Categories

(Core :: Graphics, defect)

43 Branch
Unspecified
All
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: milan, Assigned: milan)

References

Details

(Keywords: feature, Whiteboard: [gfx-noted])

Attachments

(1 file, 1 obsolete file)

Make it easier to figure out if the users have preferences set that are forcing a non-default behaviour.
Assignee: nobody → milan
Summary: Drop a crash annotation for major preferences → Keep a crash annotation for major preferences
Whiteboard: [gfx-noted]
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?
Attachment #8702371 - Flags: review?(bgirard) → review+
Attachment #8702371 - Attachment is obsolete: true
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.
Attachment #8709113 - Flags: review?(bgirard)
Comment on attachment 8709113 [details]
MozReview Request: Bug 1235437: Add annotation for major graphics preferences. r?bgirard

https://reviewboard.mozilla.org/r/31265/#review29201
Attachment #8709113 - Flags: review?(bgirard) → review+
Comment on attachment 8709113 [details]
MozReview Request: Bug 1235437: Add annotation for major graphics preferences. r?bgirard

https://reviewboard.mozilla.org/r/31265/#review29379
Attachment #8709113 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/9ac0b31bf6bb
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.