Closed
Bug 593744
Opened 14 years ago
Closed 13 years ago
nsWindow.cpp should assert if mLayerManager is null before Returning from nsWindow::GetLayerManager()
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla10
People
(Reporter: Callek, Assigned: mmarcottulio)
References
Details
(Whiteboard: [good first bug])
Attachments
(1 file)
907 bytes,
patch
|
joe
:
review+
Callek
:
feedback+
|
Details | Diff | Splinter Review |
From an IRC discussion, following http://hg.mozilla.org/mozilla-central/rev/6d1ca8f869dd mLayerManager can be returned null in (currently unsupported) cases. I suggested we assert that it is not null on a return here, since I was also told it is expected not to be. (Before this cset we never returned null here). The assert will help prevent future mistakes that can happen much easier given the web of ifdefs inside this function.
Reporter | ||
Updated•13 years ago
|
Whiteboard: [good first bug]
Assignee | ||
Comment 1•13 years ago
|
||
Attachment #550597 -
Flags: review?(bugspam.Callek)
Reporter | ||
Comment 2•13 years ago
|
||
Comment on attachment 550597 [details] [diff] [review] Added the requested assert I'm not a reviewer here, and it looks like the code has changed significantly since I filed this. I wasn't able to detangle it at first glance, but it does look like we still use |new| to create a LayerManager in this fallback (even though the fallback is no longer #ifdef'ed out in some cases). What I am not sure about is if this is an infallible alloc or not in this part of the code. The actual addition of the assert is technically correct, over to Joe for the concept on if this assert is actually useful here now.
Attachment #550597 -
Flags: review?(joe)
Attachment #550597 -
Flags: review?(bugspam.Callek)
Attachment #550597 -
Flags: feedback+
Reporter | ||
Updated•13 years ago
|
Assignee: nobody → mmarcottulio
Status: NEW → ASSIGNED
Updated•13 years ago
|
Attachment #550597 -
Flags: review?(joe) → review+
Comment 3•13 years ago
|
||
new is always infallible in Mozilla code; you have to use |new (fallible_t())| to get a fallible new.
Reporter | ||
Comment 4•13 years ago
|
||
Marco, I'm sorry we didn't catch this before now. Typically once you have review you would add checkin-needed so someone can get around to actually pushing this live. (I'm hoping the patch has not bitrotted since review)
Keywords: checkin-needed
Comment 5•13 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/9fc9bf0a5ec0
Keywords: checkin-needed
Target Milestone: --- → mozilla10
Comment 6•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/9fc9bf0a5ec0
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•