CSSStyleSheet::document() tries to find the owner node and - if it's a document - return it. This should be in its own function, which will come in useful for https://bugs.webkit.org/show_bug.cgi?id=67720. Also, the function itself can be streamlined.
Created attachment 111235 [details] Patch
Comment on attachment 111235 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=111235&action=review > Source/WebCore/css/CSSStyleSheet.cpp:285 > + Unnecessary newline. > Source/WebCore/css/CSSStyleSheet.h:91 > + Node* findOwnerNode() const; I think this name is way too easily confused with StyleSheet::ownerNode() which doesn't climb.
(In reply to comment #2) > > Source/WebCore/css/CSSStyleSheet.h:91 > > + Node* findOwnerNode() const; > > I think this name is way too easily confused with StyleSheet::ownerNode() which doesn't climb. Well, that's what 'find' is supposed to imply, but I'm of course open to suggestions.
Comment on attachment 111235 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=111235&action=review >> Source/WebCore/css/CSSStyleSheet.h:91 >> + Node* findOwnerNode() const; > > I think this name is way too easily confused with StyleSheet::ownerNode() which doesn't climb. Kling-san is right. Are there any examples of similar searching in the code? Perhaps we could steal naming convention from there?
Comment on attachment 111235 [details] Patch Removing from review queue since this patch doesn't apply anymore.
Created attachment 113827 [details] de-conflicted patch AFAICT there's no naming convention for tree climbing - e.g., CSSRule's parentStyleSheet() implicitly climbs. Renamed the function to 'styleSheetOwnerNode'.
Comment on attachment 113827 [details] de-conflicted patch Clearing flags on attachment: 113827 Committed r99426: <http://trac.webkit.org/changeset/99426>
All reviewed patches have been landed. Closing bug.
(In reply to comment #6) > AFAICT there's no naming convention for tree climbing - e.g., CSSRule's parentStyleSheet() implicitly climbs. Renamed the function to 'styleSheetOwnerNode'. In the past, we have sometimes used the word “find” as the prefix for “climbing”. Heh, now that I write this it seems that’s what you used and Andreas talked you out of it!
(In reply to comment #9) > (In reply to comment #6) > > AFAICT there's no naming convention for tree climbing - e.g., CSSRule's parentStyleSheet() implicitly climbs. Renamed the function to 'styleSheetOwnerNode'. > > In the past, we have sometimes used the word “find” as the prefix for “climbing”. Heh, now that I write this it seems that’s what you used and Andreas talked you out of it! D'oh! Well I'm certainly not opposed to the "find" prefix, especially if that's an existing convention. I was merely pointing out the possibly confusing similarity between "ownerNode" and "findOwnerNode" :-)
(In reply to comment #10) > I was merely pointing out the possibly confusing similarity between "ownerNode" and "findOwnerNode" :-) Yes, I agree completely with that. We can use the “find” prefix to allude to the fact that we’re not just getting something we have directly, say in a data member, and instead having to do a bit of searching. But when it comes to the thing after “find” it should not just be the same as some other function in the same class.
Want me to rename it to findStyleSheetOwnerNode?
(In reply to comment #12) > Want me to rename it to findStyleSheetOwnerNode? That might be good. On the other hand, since in turn it’s called in a function named just “document”, I am not sure that adding a find just to this function’s name pays off all that well in reminding people about performance cost.
Ask, and ye shall receive! ;) -> https://bugs.webkit.org/show_bug.cgi?id=71764