Closed
Bug 627464
Opened 13 years ago
Closed 13 years ago
Annotate crash reports about gfx features in use
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | - |
People
(Reporter: jrmuizel, Assigned: bjacob)
References
Details
Attachments
(1 file, 2 obsolete files)
25.68 KB,
patch
|
jrmuizel
:
review+
joe
:
approval2.0+
|
Details | Diff | Splinter Review |
We should do this so we can more easily ignore these crashes
Comment 2•13 years ago
|
||
Maybe you can convince me this should block, Jeff, but it seems like it'd be purely gravy, not necessarily required for 4.0 I would a+ a patch SO HARD.
blocking2.0: ? → -
Assignee | ||
Comment 3•13 years ago
|
||
This would indeed have saved me 2 days on bug 629265.
Blocks: 629265
Assignee | ||
Comment 4•13 years ago
|
||
This patch adds App Notes for actually tried/successful/failed Gfx features, like this: D2D? D2D+ DWrite? DWrite+ D3D10 Layers? D3D10 Layers+ WebGL? EGL? EGL+ GL Context? GL Context+ WebGL+ Indeed, we discussed that knowing what had actually happened was more useful than knowing the prefs, and that info, together with info we already get in crash reports, allows us to know when users have force-enabled stuff anyway. The Xxx? part is added to AppNotes before the feature is tried. That will help debugging crashes that occurred during initialization of said feature. The feature name is repeated (Xxx? Xxx+) for the following reason. Each of these strings is printed at most once to avoid flooding the AppNotes. So if Xxx succeeded, then Yyy succeeded, then Xxx failed, we get: Xxx? Xxx+ Yyy? Yyy+ Xxx- since the Xxx? is not repeated. That's why it's useful to repeat the feature name (Xxx- instead of just -).
Attachment #513225 -
Flags: review?(jmuizelaar)
Reporter | ||
Comment 5•13 years ago
|
||
Comment on attachment 513225 [details] [diff] [review] report gfx features >+ >+void >+ScopedGfxFeatureReporter::WriteAppNote(char statusChar) >+{ >+#ifdef MOZ_GFXFEATUREREPORTER >+ static nsCString featuresAlreadyReported; >+ >+ nsACString::const_iterator start, end; >+ featuresAlreadyReported.BeginReading(start); >+ featuresAlreadyReported.EndReading(end); >+ Please use a list of strings instead of a string of strings. > > // if initialization fails, ensure all symbols are zero, to avoid hard-to-understand bugs > if (!mInitialized) >- mSymbols.Zero(); >+ mSymbols.Zero(); >+ >+ if (mInitialized) >+ reporter.SetSuccessful(); maybe use an else here.
Attachment #513225 -
Flags: review?(jmuizelaar) → review-
Assignee | ||
Comment 6•13 years ago
|
||
applied your comments. amazing how the array-of-strings idea turns out to be more concise.
Attachment #513225 -
Attachment is obsolete: true
Attachment #514912 -
Flags: review?(jmuizelaar)
Reporter | ||
Comment 7•13 years ago
|
||
Comment on attachment 514912 [details] [diff] [review] report gfx features, updated >+ >+void >+ScopedGfxFeatureReporter::WriteAppNote(char statusChar) >+{ >+#ifdef MOZ_GFXFEATUREREPORTER >+ static nsTArray<nsCString> featuresAlreadyReported; >+ >+ nsCAutoString featureString; >+ featureString.AppendPrintf("%s%c%c", >+ mFeature, >+ statusChar, >+ statusChar == '?' ? ' ' : '\n'); >+ >+ if (!featuresAlreadyReported.Contains(featureString)) { >+ featuresAlreadyReported.AppendElement(featureString); >+ CrashReporter::AppendAppNotesToCrashReport(featureString); >+ } >+#else >+ (void) statusChar; >+#endif >+} >+ It would be cleaner to have a single #ifdef and do a completely separate stub implementation.
Attachment #514912 -
Flags: review?(jmuizelaar) → review+
Comment 8•13 years ago
|
||
Comment on attachment 514912 [details] [diff] [review] report gfx features, updated Looks good.
Attachment #514912 -
Flags: approval2.0+
Updated•13 years ago
|
Assignee: nobody → bjacob
Updated•13 years ago
|
Version: unspecified → Trunk
Assignee | ||
Comment 9•13 years ago
|
||
Here is a version that hopefully will please tryserver... http://tbpl.mozilla.org/?tree=MozillaTry&rev=661e198bd49d
Attachment #514912 -
Attachment is obsolete: true
Attachment #516039 -
Flags: review?(jmuizelaar)
Reporter | ||
Updated•13 years ago
|
Attachment #516039 -
Flags: review?(jmuizelaar) → review+
Assignee | ||
Comment 10•13 years ago
|
||
OK, the .forget() was bad here, removing it makes it all green.
Updated•13 years ago
|
Attachment #516039 -
Flags: approval2.0+
Assignee | ||
Updated•13 years ago
|
Summary: Annotate crash reports if the forced-enabled prefs are set → Annotate crash reports about gfx features in use
Assignee | ||
Comment 11•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/b6ca6d65fafa
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•