Lazily create HTMLInputElement's inputType and shadow subtree
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 11 Nov 2014 07:19:53 +0000 (07:19 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 11 Nov 2014 07:19:53 +0000 (07:19 +0000)
https://bugs.webkit.org/show_bug.cgi?id=138524

Reviewed by Ryosuke Niwa.

When an HTMLInputElement was created by the parser, we would first call
HTMLInputElement::create(), then call Element::parserSetAttributes() on
the constructed input. With the previous implementation, this was a bit
inefficient because HTMLInputElement::create() would construct a
TextInputType inputType (as this is the default) as well as its
corresponding shadow subtree. Then, parserSetAttributes() would often
set the |type| attribute and would need to destroy this input type as
well as its subtree if the new |type| is not 'text', to create a new
inputType / shadow subtree of the right type. The profiler showed that
this was fairly expensive.

To improve this, this patch delays the inputType / shadow subtree
creation when the HTMLInputElement is constructed by the parser, until
the attributes are actually set by the parser. This way, we directly
create an inputType / shadow subtree of the right type.

I see a 1.4% speed up on speedometer (73.95 -> 75.0).

No new tests, no behavior change.

* dom/Element.cpp:
(WebCore::Element::parserSetAttributes):
(WebCore::Element::parserDidFinishParsingAttributes):
* dom/Element.h:
* html/HTMLInputElement.cpp:
(WebCore::HTMLInputElement::HTMLInputElement):
(WebCore::HTMLInputElement::create):
(WebCore::HTMLInputElement::updateType):
(WebCore::HTMLInputElement::runPostTypeUpdateTasks):
(WebCore::HTMLInputElement::ensureInputType):
(WebCore::HTMLInputElement::parseAttribute):
(WebCore::HTMLInputElement::parserDidFinishParsingAttributes):
* html/HTMLInputElement.h:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@175852 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/dom/Element.cpp
Source/WebCore/dom/Element.h
Source/WebCore/html/HTMLInputElement.cpp
Source/WebCore/html/HTMLInputElement.h

index 1990abd..5bab383 100644 (file)
@@ -1,3 +1,44 @@
+2014-11-10  Chris Dumez  <cdumez@apple.com>
+
+        Lazily create HTMLInputElement's inputType and shadow subtree
+        https://bugs.webkit.org/show_bug.cgi?id=138524
+
+        Reviewed by Ryosuke Niwa.
+
+        When an HTMLInputElement was created by the parser, we would first call
+        HTMLInputElement::create(), then call Element::parserSetAttributes() on
+        the constructed input. With the previous implementation, this was a bit
+        inefficient because HTMLInputElement::create() would construct a
+        TextInputType inputType (as this is the default) as well as its
+        corresponding shadow subtree. Then, parserSetAttributes() would often
+        set the |type| attribute and would need to destroy this input type as
+        well as its subtree if the new |type| is not 'text', to create a new
+        inputType / shadow subtree of the right type. The profiler showed that
+        this was fairly expensive.
+
+        To improve this, this patch delays the inputType / shadow subtree
+        creation when the HTMLInputElement is constructed by the parser, until
+        the attributes are actually set by the parser. This way, we directly
+        create an inputType / shadow subtree of the right type.
+
+        I see a 1.4% speed up on speedometer (73.95 -> 75.0).
+
+        No new tests, no behavior change.
+
+        * dom/Element.cpp:
+        (WebCore::Element::parserSetAttributes):
+        (WebCore::Element::parserDidFinishParsingAttributes):
+        * dom/Element.h:
+        * html/HTMLInputElement.cpp:
+        (WebCore::HTMLInputElement::HTMLInputElement):
+        (WebCore::HTMLInputElement::create):
+        (WebCore::HTMLInputElement::updateType):
+        (WebCore::HTMLInputElement::runPostTypeUpdateTasks):
+        (WebCore::HTMLInputElement::ensureInputType):
+        (WebCore::HTMLInputElement::parseAttribute):
+        (WebCore::HTMLInputElement::parserDidFinishParsingAttributes):
+        * html/HTMLInputElement.h:
+
 2014-11-10  Benjamin Poulain  <bpoulain@apple.com>
 
         Add parsing support for the extended :nth-last-child(An+B of selector-list) defined
index 5381bc8..11f9083 100644 (file)
@@ -1233,17 +1233,22 @@ void Element::parserSetAttributes(const Vector<Attribute>& attributeVector)
     ASSERT(!parentNode());
     ASSERT(!m_elementData);
 
-    if (attributeVector.isEmpty())
-        return;
+    if (!attributeVector.isEmpty()) {
+        if (document().sharedObjectPool())
+            m_elementData = document().sharedObjectPool()->cachedShareableElementDataWithAttributes(attributeVector);
+        else
+            m_elementData = ShareableElementData::createWithAttributes(attributeVector);
 
-    if (document().sharedObjectPool())
-        m_elementData = document().sharedObjectPool()->cachedShareableElementDataWithAttributes(attributeVector);
-    else
-        m_elementData = ShareableElementData::createWithAttributes(attributeVector);
+        // Use attributeVector instead of m_elementData because attributeChanged might modify m_elementData.
+        for (const auto& attribute : attributeVector)
+            attributeChanged(attribute.name(), nullAtom, attribute.value(), ModifiedDirectly);
+    }
 
-    // Use attributeVector instead of m_elementData because attributeChanged might modify m_elementData.
-    for (unsigned i = 0; i < attributeVector.size(); ++i)
-        attributeChanged(attributeVector[i].name(), nullAtom, attributeVector[i].value(), ModifiedDirectly);
+    parserDidFinishParsingAttributes();
+}
+
+void Element::parserDidFinishParsingAttributes()
+{
 }
 
 bool Element::hasAttributes() const
index c82ce97..9895924 100644 (file)
@@ -555,6 +555,7 @@ protected:
     virtual void removedFrom(ContainerNode&) override;
     virtual void childrenChanged(const ChildChange&) override;
     virtual void removeAllEventListeners() override final;
+    virtual void parserDidFinishParsingAttributes();
 
     void clearTabIndexExplicitlyIfNeeded();    
     void setTabIndexExplicitly(short);
index 16177ba..869e370 100644 (file)
@@ -121,7 +121,9 @@ HTMLInputElement::HTMLInputElement(const QualifiedName& tagName, Document& docum
 #if ENABLE(TOUCH_EVENTS)
     , m_hasTouchEventHandler(false)
 #endif
-    , m_inputType(InputType::createText(*this))
+    // m_inputType is lazily created when constructed by the parser to avoid constructing unnecessarily a text inputType and
+    // its shadow subtree, just to destroy them when the |type| attribute gets set by the parser to something else than 'text'.
+    , m_inputType(createdByParser ? nullptr : InputType::createText(*this))
 {
     ASSERT(hasTagName(inputTag) || hasTagName(isindexTag));
     setHasCustomStyleResolveCallbacks();
@@ -129,8 +131,10 @@ HTMLInputElement::HTMLInputElement(const QualifiedName& tagName, Document& docum
 
 PassRefPtr<HTMLInputElement> HTMLInputElement::create(const QualifiedName& tagName, Document& document, HTMLFormElement* form, bool createdByParser)
 {
+    bool shouldCreateShadowRootLazily = createdByParser;
     RefPtr<HTMLInputElement> inputElement = adoptRef(new HTMLInputElement(tagName, document, form, createdByParser));
-    inputElement->ensureUserAgentShadowRoot();
+    if (!shouldCreateShadowRootLazily)
+        inputElement->ensureUserAgentShadowRoot();
     return inputElement.release();
 }
 
@@ -432,6 +436,7 @@ void HTMLInputElement::setType(const AtomicString& type)
 
 void HTMLInputElement::updateType()
 {
+    ASSERT(m_inputType);
     auto newType = InputType::create(*this, fastGetAttribute(typeAttr));
     bool hadType = m_hasType;
     m_hasType = true;
@@ -457,17 +462,6 @@ void HTMLInputElement::updateType()
     m_inputType->createShadowSubtree();
     updateInnerTextElementEditability();
 
-#if ENABLE(TOUCH_EVENTS)
-    bool hasTouchEventHandler = m_inputType->hasTouchEventHandler();
-    if (hasTouchEventHandler != m_hasTouchEventHandler) {
-        if (hasTouchEventHandler)
-            document().didAddTouchEventHandler(this);
-        else
-            document().didRemoveTouchEventHandler(this);
-        m_hasTouchEventHandler = hasTouchEventHandler;
-    }
-#endif
-
     setNeedsWillValidateCheck();
 
     bool willStoreValue = m_inputType->storesValueSeparateFromAttribute();
@@ -503,6 +497,23 @@ void HTMLInputElement::updateType()
             attributeChanged(alignAttr, nullAtom, align->value());
     }
 
+    runPostTypeUpdateTasks();
+}
+
+inline void HTMLInputElement::runPostTypeUpdateTasks()
+{
+    ASSERT(m_inputType);
+#if ENABLE(TOUCH_EVENTS)
+    bool hasTouchEventHandler = m_inputType->hasTouchEventHandler();
+    if (hasTouchEventHandler != m_hasTouchEventHandler) {
+        if (hasTouchEventHandler)
+            document().didAddTouchEventHandler(this);
+        else
+            document().didRemoveTouchEventHandler(this);
+        m_hasTouchEventHandler = hasTouchEventHandler;
+    }
+#endif
+
     if (renderer())
         setNeedsStyleRecalc(ReconstructRenderTree);
 
@@ -596,8 +607,35 @@ void HTMLInputElement::collectStyleForPresentationAttribute(const QualifiedName&
         HTMLTextFormControlElement::collectStyleForPresentationAttribute(name, value, style);
 }
 
+inline void HTMLInputElement::ensureInputType()
+{
+    ASSERT(m_parsingInProgress);
+    if (m_inputType)
+        return;
+
+    if (!hasAttribute(typeAttr)) {
+        m_inputType = InputType::createText(*this);
+        ensureUserAgentShadowRoot();
+        return;
+    }
+
+    m_hasType = true;
+    m_inputType = InputType::create(*this, fastGetAttribute(typeAttr));
+    ensureUserAgentShadowRoot();
+    registerForSuspensionCallbackIfNeeded();
+    runPostTypeUpdateTasks();
+}
+
 void HTMLInputElement::parseAttribute(const QualifiedName& name, const AtomicString& value)
 {
+    if (m_parsingInProgress) {
+        // A lot of the code below requires m_inputType to be initialized so make sure we do.
+        // By the time parseAttribute() is called during parsing anyway, all attributes have
+        // been set on the element already so there is no point in delaying m_inputType
+        // initialization further.
+        ensureInputType();
+    }
+
     if (name == nameAttr) {
         removeFromRadioButtonGroup();
         m_name = value;
@@ -705,6 +743,12 @@ void HTMLInputElement::parseAttribute(const QualifiedName& name, const AtomicStr
     m_inputType->attributeChanged();
 }
 
+void HTMLInputElement::parserDidFinishParsingAttributes()
+{
+    ASSERT(m_inputType || !hasAttributes());
+    ensureInputType();
+}
+
 void HTMLInputElement::finishParsingChildren()
 {
     m_parsingInProgress = false;
index 277f772..d27edcb 100644 (file)
@@ -359,6 +359,7 @@ private:
     virtual bool isPresentationAttribute(const QualifiedName&) const override;
     virtual void collectStyleForPresentationAttribute(const QualifiedName&, const AtomicString&, MutableStyleProperties&) override;
     virtual void finishParsingChildren() override;
+    virtual void parserDidFinishParsingAttributes() override final;
 
     virtual void copyNonAttributePropertiesFromElement(const Element&) override;
 
@@ -398,7 +399,9 @@ private:
     virtual bool recalcWillValidate() const override;
     virtual void requiredAttributeChanged() override;
 
+    void ensureInputType();
     void updateType();
+    void runPostTypeUpdateTasks();
     
     virtual void subtreeHasChanged() override;