Bug 38131

Summary: REGRESSION (r57292): 1.5% page load speed regression from visited link information leak fix
Product: WebKit Reporter: Maciej Stachowiak <mjs>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal Keywords: InRadar
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
Patch darin: review+

Description Maciej Stachowiak 2010-04-26 11:18:22 PDT
REGRESSION (r57292): 1.5% page load speed regression from visited link information leak fix
Comment 1 Maciej Stachowiak 2010-04-26 15:11:47 PDT
<rdar://problem/7859794>
Comment 2 Maciej Stachowiak 2010-04-26 15:14:00 PDT
Created attachment 54339 [details]
Patch
Comment 3 Darin Adler 2010-04-26 15:23:25 PDT
Comment on attachment 54339 [details]
Patch

> 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
Comment 4 Maciej Stachowiak 2010-04-26 17:10:18 PDT
Committed r58273: <http://trac.webkit.org/changeset/58273>