329 bytes, text/html
5.20 KB, text/plain
3.46 KB, patch
|Details | Diff | Splinter Review|
909 bytes, patch
|Details | Diff | Splinter Review|
Assertion failure: !aIID.Equals((::nsISupports::COMTypeInfo<int>::kIID)), at ../../../layout/style/nsDOMCSSRGBColor.cpp:37
ugh, I'm a clown who shoot himself in the foot with the QI macros again :(
NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY is indeed a footgun. How about we fix it to not be one?
What is the problem exactly? NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY should have gone first? I'm not familiar with it.
The problem is that NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY totally fails if it's the last thing in the QI, because it does not have a trailing else.
Not yet clear why this should track. What's the user impact? Did this regress in FF19? Bug 821593 was landed in FF20.
The main impact is lack of fuzzing coverage. I don't think there should be other user impact here, offhand. I don't think this is a problem in 19, indeed.
I could fix the QI but it seems just as easy to nuke it so...
Attachment #713613 - Flags: review?(bzbarsky)
Comment on attachment 713613 [details] [diff] [review] patch I'm not actually sure how non-isupports CC works. Andrew, can you review this? If not, I guess we pass it off to Peter?
Attachment #713613 - Flags: review?(bzbarsky) → review?(continuation)
Comment on attachment 713613 [details] [diff] [review] patch Review of attachment 713613 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. ::: layout/style/nsDOMCSSRGBColor.cpp @@ +32,4 @@ > NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE_4(nsDOMCSSRGBColor, mAlpha, mBlue, mGreen, mRed) > + > + NS_IMPL_CYCLE_COLLECTION_ROOT_NATIVE(nsDOMCSSRGBColor, AddRef) > + NS_IMPL_CYCLE_COLLECTION_UNROOT_NATIVE(nsDOMCSSRGBColor, Release) Two extra spaces before both the ROOT and UNROOT lines.
Attachment #713613 - Flags: review?(continuation) → review+
Is there a bug on fixing NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY?
(In reply to Jesse Ruderman from comment #11) > Is there a bug on fixing NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY? bug 798188?
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Not tracking but would take an uplift if deemed low risk so we can restore coverage. Please nominate if that's the case.
I think we could do a very low-risk patch here for 20, and should....
Comment on attachment 715244 [details] [diff] [review] safe patch [Approval Request Comment] Bug caused by (feature/regressing bug #): User impact if declined: makes fuzzing harder / lost test coverage Testing completed (on m-c, etc.): it builds and shouldn't effect tests Risk to taking this patch (and alternatives if risky): should be pretty low String or UUID changes made by this patch: none
Attachment #715244 - Flags: approval-mozilla-aurora?
Comment on attachment 715244 [details] [diff] [review] safe patch Approving for aurora uplift, please try to land this before merge tomorrow (tues) morning PT - otherwise this will need to be approved for beta.
Attachment #715244 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Issue reproducible with the debug build from 2013-02-13. (assertion failure + Firefox crash) Verified fixed with Firefox 20 beta 7 debug build, on Mac OSX 10.8.2 (no assertion failure + no Firefox crash) Build ID: 20130323031722
No assertion failure - No crash Verified fixed on Firefox 21 beta 2 (latest beta debug build), on Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:21.0) Gecko/20130411 Firefox/21.0 Build ID: 20130411111427
You need to log in before you can comment on or make changes to this bug.