RESOLVED FIXED Bug 82940
Simplify CSSStyleSelector constructor through the use of a helper function 
https://bugs.webkit.org/show_bug.cgi?id=82940
Summary Simplify CSSStyleSelector constructor through the use of a helper function
Eric Seidel (no email)
Reported 2012-04-02 13:36:49 PDT
Simplify CSSStyleSelector constructor through the use of a helper function
Attachments
Patch (4.18 KB, patch)
2012-04-02 13:37 PDT, Eric Seidel (no email) 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Eric Seidel (no email)
Comment 1 2012-04-02 13:37:34 PDT
Created attachment 135171 [details] Patch
Alexis Menard (darktears)
Comment 2 2012-04-02 13:55:18 PDT
Comment on attachment 135171 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=135171&action=review > Source/WebCore/css/CSSStyleSelector.cpp:410 > +void CSSStyleSelector::addAuthorRulesAndCollectUserRulesFromSheets(const Vector<RefPtr<CSSStyleSheet> >* userSheets, RuleSet& userStyle) While I see the benefit of the patch to avoid code duplication, I'm not sure whether it's part of the usage to pass a object by reference to modify it.
Eric Seidel (no email)
Comment 3 2012-04-02 13:57:35 PDT
See http://www.webkit.org/coding/coding-style.html "Pointers and References" for minimal guidance on the subject. In short: yes, we do pass by reference at times.
Eric Seidel (no email)
Comment 4 2012-04-02 13:58:09 PDT
"An out argument of a function should be passed by reference except rare cases where it is optional in which case it should be passed by pointer." seems to suggest that we prefer references to pointers. :)
Gustavo Noronha (kov)
Comment 5 2012-04-02 14:56:51 PDT
Comment on attachment 135171 [details] Patch Attachment 135171 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/12310679
Eric Seidel (no email)
Comment 6 2012-04-02 15:10:11 PDT
Comment on attachment 135171 [details] Patch gtk lies!
WebKit Review Bot
Comment 7 2012-04-03 05:11:28 PDT
Comment on attachment 135171 [details] Patch Clearing flags on attachment: 135171 Committed r113016: <http://trac.webkit.org/changeset/113016>
WebKit Review Bot
Comment 8 2012-04-03 05:11:34 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.