Closed
Bug 1281702
Opened 6 years ago
Closed 6 years ago
"An assert from the graphics logger" assertion message should include the log message
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla50
Tracking | Status | |
---|---|---|
firefox50 | --- | fixed |
People
(Reporter: jruderman, Assigned: milan)
References
Details
(Keywords: feature, Whiteboard: [gfx-noted])
Attachments
(1 file, 1 obsolete file)
https://hg.mozilla.org/mozilla-central/file/c9edfe35619f/gfx/2d/Logging.h#l519 produces output like: [GFX1]: Texture deallocated too late during shutdown Assertion failure: false (An assert from the graphics logger), at gfx/2d/Logging.h:519 Having these on separate lines confuses anything that tries to bucket failures by the assertion message, such as FuzzManager. Bugs on oranges are indistinguishable: https://bugzilla.mozilla.org/buglist.cgi?quicksearch=ALL%20su%3A%22An%20assert%20from%20the%20graphics%20logger%22&order=bugs.bug_id%20desc&list_id=13081556 jwalden, what's the best way to fix this? MOZ_ASSERT only takes string literals; what can this code use instead?
Flags: needinfo?(jwalden+bmo)
Assignee | ||
Comment 1•6 years ago
|
||
Would the world explode if we changed: MOZ_ASSERT(false, "An assert from the graphics logger"); to MOZ_ASSERT(false, "%s", aString.c_str()); where aString is a std::string?
Assignee | ||
Comment 2•6 years ago
|
||
Heh, I guess not; I thought the vargs in the macro was for that kind of thing, but not really.
Assignee | ||
Comment 3•6 years ago
|
||
Something like this? Jesse, does it fix your issue?
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → milan
Assignee | ||
Updated•6 years ago
|
Attachment #8764712 -
Attachment is obsolete: true
Assignee | ||
Comment 4•6 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/64670/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/64670/
Attachment #8771550 -
Flags: review?(nfroyd)
![]() |
||
Updated•6 years ago
|
Attachment #8771550 -
Flags: review?(nfroyd)
![]() |
||
Comment 5•6 years ago
|
||
Comment on attachment 8771550 [details] Bug 1281702: Report the actual error message, rather than a generic one from the graphics logger. https://reviewboard.mozilla.org/r/64670/#review61978 I see that we have a number of places in the tree that simply make direct calls to what `MOZ_ASSERT` expands to: ``` MOZ_ReportAssertionFailure(...); MOZ_CRASH(); ``` I think that is a better option than adding another case to `MOZ_ASSERT`.
Assignee | ||
Comment 6•6 years ago
|
||
(In reply to Nathan Froyd [:froydnj] from comment #5) > ... > I think that is a better option than adding another case to `MOZ_ASSERT`. Good idea.
Flags: needinfo?(jwalden+bmo)
OS: Unspecified → All
Assignee | ||
Updated•6 years ago
|
Attachment #8771550 -
Attachment is obsolete: true
Assignee | ||
Comment 7•6 years ago
|
||
Comment on attachment 8771550 [details] Bug 1281702: Report the actual error message, rather than a generic one from the graphics logger. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64670/diff/1-2/
Attachment #8771550 -
Attachment description: Bug 1281702: Add MOZ_ASSERT with a third parameter, a non-string-literal detailed message, and use it in the graphics asserts. → Bug 1281702: Report the actual error message, rather than a generic one from the graphics logger.
Attachment #8771550 -
Attachment is obsolete: false
Attachment #8771550 -
Flags: review?(mchang)
![]() |
||
Comment 8•6 years ago
|
||
My understanding of Jesse's request is that the actual crash message differ based on the particular thing we are logging, and this new patch doesn't fix that? Is that correct, Jesse?
Flags: needinfo?(jruderman)
Assignee | ||
Comment 9•6 years ago
|
||
It's the assert message that will be different - the crash message "doesn't matter" (e.g., it was empty before.)
Comment 10•6 years ago
|
||
Comment on attachment 8771550 [details] Bug 1281702: Report the actual error message, rather than a generic one from the graphics logger. https://reviewboard.mozilla.org/r/64670/#review62260
Attachment #8771550 -
Flags: review?(mchang) → review+
Comment 11•6 years ago
|
||
Pushed by msreckovic@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/3f45d79f6bc7 Report the actual error message, rather than a generic one from the graphics logger. r=mchang
Comment 12•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/3f45d79f6bc7
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
![]() |
||
Updated•6 years ago
|
Flags: needinfo?(jruderman)
You need to log in
before you can comment on or make changes to this bug.
Description
•