Unused CSSRuleList vector in CSSParser class.
Created attachment 76460 [details] Patch
Comment on attachment 76460 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=76460&action=review > WebCore/css/CSSRuleList.h:37 > + CSSRuleList(); It would be better to use releaseRef() instead of exposing the constructor.
Created attachment 76963 [details] Patch
I used leakRef() instead of releaseRef() as it seems releaseRef() is being removed at some point?
Comment on attachment 76963 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=76963&action=review Overall, this feel tricky. (Leaking the pointer and then in another function in another file doing the adoptRef.) Reinforcing this is the fact that is there is a memory leak bug which several of us missed. (Eric missed it. I didn't see it for a while, and you missed it.) So I'm wondering what benefit is it to remove this variable? (It looks like CSSParser is not a long lived object. It also doesn't appear that lots of them are created at the same time.) > WebCore/css/CSSGrammar.y:415 > + if (!$$) { Nit: No { for single line statements. > WebCore/css/CSSParser.cpp:5518 > if (!media || !rules || !m_styleSheet) The ref count on rules leaks here. I'd suggest putting rules in a RefPtr<> = adoptRef(rules); immediately upon entering this function and then doing a .releaseRef() below.
CSSGrammer.y does not exist on Webkit Github repo as of today. Further, in CSSParser.cpp, there is no mention of createMediaRule and nothing about createRuleList: https://github.com/WebKit/WebKit/blob/a8766e9cb3f1d5446226413afe0de6b0a64dc996/Source/WebCore/css/parser/CSSParser.cpp I think CSSPraser has been changed significantly since this patch and it cannot be applied directly. I am going to mark this as "RESOLVED WONTFIX". Please add anything for future reference or if this is incorrect conclusion. Thanks!