[webkit-reviews] review granted: [Bug 38131] REGRESSION (r57292): 1.5% page load speed regression from visited link information leak fix : [Attachment 54339] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Apr 26 15:23:24 PDT 2010


Darin Adler <darin at apple.com> has granted Maciej Stachowiak <mjs at apple.com>'s
request for review:
Bug 38131: REGRESSION (r57292): 1.5% page load speed regression from visited
link information leak fix
https://bugs.webkit.org/show_bug.cgi?id=38131

Attachment 54339: Patch
https://bugs.webkit.org/attachment.cgi?id=54339&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> Index: WebCore/css/CSSMutableStyleDeclaration.cpp
> ===================================================================
> --- WebCore/css/CSSMutableStyleDeclaration.cpp	(revision 58222)
> +++ WebCore/css/CSSMutableStyleDeclaration.cpp	(working copy)
> @@ -629,7 +629,7 @@ void CSSMutableStyleDeclaration::setLeng
>  
>  unsigned CSSMutableStyleDeclaration::length() const
>  {
> -    return m_properties.size();
> +    return mutableLength();
>  }
>  
>  String CSSMutableStyleDeclaration::item(unsigned i) const
> Index: WebCore/css/CSSMutableStyleDeclaration.h
> ===================================================================
> --- WebCore/css/CSSMutableStyleDeclaration.h	(revision 58222)
> +++ WebCore/css/CSSMutableStyleDeclaration.h	(working copy)
> @@ -89,6 +89,8 @@ public:
>      virtual void setCssText(const String&, ExceptionCode&);
>  
>      virtual unsigned length() const;
> +    unsigned mutableLength() const { return m_properties.size(); }

In person, I suggested another way of doing this optimization that is better.
Make the length function in the base class non-virtual, and have it be an
inline that calls a virtual. Then this class can override length.

This pattern is used for EventListener::isAttribute, Node::localName,
Node::namespaceURI, Node::prefix, Node::computedStyle,
ScriptExecutionContext::url, ScriptExecutionContext::completeURL, and
HTMLElement::form.

This eliminates the unpleasant "mutableLength()" name.

r=me if you change that


More information about the webkit-reviews mailing list