Changeset 87123 in webkit

Timestamp:

May 23, 2011 8:31:34 PM (13 years ago)

Author:

morrita@google.com

Message:

2011-05-20 MORITA Hajime <​morrita@google.com>

Reviewed by Dimitri Glazkov.

[Refactoring] attach() following detach() should be replaced with Node::reattach()
​https://bugs.webkit.org/show_bug.cgi?id=61011

• Renamed forceReattach() to reattach()
• Introduced reattachIfAttached() as a variant.

No new tests. No behavior change.

• dom/CharacterData.cpp: (WebCore::CharacterData::updateRenderer):
• dom/Element.cpp: (WebCore::Element::recalcStyle):
• dom/Node.h: (WebCore::Node::reattach): (WebCore::Node::reattachIfAttached):
• dom/Text.cpp: (WebCore::Text::recalcStyle):
• html/HTMLDetailsElement.cpp: (WebCore::HTMLDetailsElement::refreshMainSummary): (WebCore::HTMLDetailsElement::parseMappedAttribute):
• html/HTMLInputElement.cpp: (WebCore::HTMLInputElement::parseMappedAttribute):
• html/HTMLObjectElement.cpp: (WebCore::HTMLObjectElement::renderFallbackContent):
• html/HTMLPlugInImageElement.cpp: (WebCore::HTMLPlugInImageElement::recalcStyle):
• html/HTMLSelectElement.cpp: (WebCore::HTMLSelectElement::parseMappedAttribute):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• dom/CharacterData.cpp (modified) (1 diff)
• dom/Element.cpp (modified) (1 diff)
• dom/Node.h (modified) (2 diffs)
• dom/ShadowContentSelector.cpp (modified) (1 diff)
• dom/ShadowRoot.cpp (modified) (1 diff)
• dom/Text.cpp (modified) (1 diff)
• html/HTMLDetailsElement.cpp (modified) (2 diffs)
• html/HTMLInputElement.cpp (modified) (1 diff)
• html/HTMLObjectElement.cpp (modified) (1 diff)
• html/HTMLPlugInImageElement.cpp (modified) (1 diff)
• html/HTMLSelectElement.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-05-20  MORITA Hajime  <morrita@google.com>
+
+        Reviewed by Dimitri Glazkov.
+        
+        [Refactoring] attach() following detach() should be replaced with Node::reattach()
+        https://bugs.webkit.org/show_bug.cgi?id=61011
+
+        - Renamed forceReattach() to reattach()
+        - Introduced reattachIfAttached() as a variant.
+        
+        No new tests. No behavior change.
+
+        * dom/CharacterData.cpp:
+        (WebCore::CharacterData::updateRenderer):
+        * dom/Element.cpp:
+        (WebCore::Element::recalcStyle):
+        * dom/Node.h:
+        (WebCore::Node::reattach):
+        (WebCore::Node::reattachIfAttached):
+        * dom/Text.cpp:
+        (WebCore::Text::recalcStyle):
+        * html/HTMLDetailsElement.cpp:
+        (WebCore::HTMLDetailsElement::refreshMainSummary):
+        (WebCore::HTMLDetailsElement::parseMappedAttribute):
+        * html/HTMLInputElement.cpp:
+        (WebCore::HTMLInputElement::parseMappedAttribute):
+        * html/HTMLObjectElement.cpp:
+        (WebCore::HTMLObjectElement::renderFallbackContent):
+        * html/HTMLPlugInImageElement.cpp:
+        (WebCore::HTMLPlugInImageElement::recalcStyle):
+        * html/HTMLSelectElement.cpp:
+        (WebCore::HTMLSelectElement::parseMappedAttribute):
+
 2011-05-23  Mark Rowe  <mrowe@apple.com>
 

trunk/Source/WebCore/dom/CharacterData.cpp

@@ -183 +183 @@
 {
     if ((!renderer() || !rendererIsNeeded(renderer()->style())) && attached()) {
-        detach();
-        attach();
+        reattach();
     } else if (renderer())
         toRenderText(renderer())->setTextWithOffset(m_data, offsetOfReplacedData, lengthOfReplacedData);

trunk/Source/WebCore/dom/Element.cpp

@@ -1087 +1087 @@
         StyleChange ch = diff(currentStyle.get(), newStyle.get());
         if (ch == Detach || !currentStyle) {
-            if (attached())
-                detach();
-            attach(); // FIXME: The style gets computed twice by calling attach. We could do better if we passed the style along.
+            // FIXME: The style gets computed twice by calling attach. We could do better if we passed the style along.
+            reattach();
             // attach recalulates the style for all children. No need to do it twice.
             clearNeedsStyleRecalc();

trunk/Source/WebCore/dom/Node.h

@@ -452 +452 @@
     virtual void detach();
 
-    void forceReattach();
+    void reattach();
+    void reattachIfAttached();
 
     virtual void willRemove();
@@ -740 +741 @@
 }
 
-inline void Node::forceReattach()
+inline void Node::reattach()
 {
-    if (!attached())
-        return;
-    detach();
+    if (attached())
+        detach();
     attach();
+}
+
+inline void Node::reattachIfAttached()
+{
+    if (attached())
+        reattach();
 }
 

trunk/Source/WebCore/dom/ShadowContentSelector.cpp

@@ -63 +63 @@
             continue;
 
-        child->forceReattach();
+        child->reattach();
         m_children[i] = 0;
     }

trunk/Source/WebCore/dom/ShadowRoot.cpp

@@ -85 +85 @@
 {
     if (hasContentElement())
-        forceReattach();
+        reattach();
     else {
         for (Node* n = firstChild(); n; n = n->nextSibling())

trunk/Source/WebCore/dom/Text.cpp

@@ -266 +266 @@
             if (renderer()->isText())
                 toRenderText(renderer())->setText(dataImpl());
-        } else {
-            if (attached())
-                detach();
-            attach();
-        }
+        } else
+            reattach();
     }
     clearNeedsStyleRecalc();

trunk/Source/WebCore/html/HTMLDetailsElement.cpp

@@ -147 +147 @@
         return;
 
-    if (oldSummary && oldSummary->parentNodeForRenderingAndStyle()) {
-        oldSummary->detach();
-        oldSummary->attach();
-    }
-        
-    if (m_mainSummary && refreshRenderer == RefreshRendererAllowed) {
-        m_mainSummary->detach();
-        m_mainSummary->attach();
-    }
+    if (oldSummary && oldSummary->parentNodeForRenderingAndStyle())
+        oldSummary->reattach();
+    if (m_mainSummary && refreshRenderer == RefreshRendererAllowed)
+        m_mainSummary->reattach();
 }
 
@@ -197 +192 @@
         bool oldValue = m_isOpen;
         m_isOpen =  !attr->value().isNull();
-        if (attached() && oldValue != m_isOpen) {
-            detach();
-            attach();
-        }
+        if (oldValue != m_isOpen)
+            reattachIfAttached();
     } else
         HTMLElement::parseMappedAttribute(attr);

trunk/Source/WebCore/html/HTMLInputElement.cpp

@@ -722 +722 @@
         // FIXME: Detaching just for maxResults change is not ideal.  We should figure out the right
         // time to relayout for this change.
-        if (m_maxResults != oldResults && (m_maxResults <= 0 || oldResults <= 0) && attached()) {
-            detach();
-            attach();
-        }
+        if (m_maxResults != oldResults && (m_maxResults <= 0 || oldResults <= 0))
+            reattachIfAttached();
         setNeedsStyleRecalc();
     } else if (attr->name() == autosaveAttr || attr->name() == incrementalAttr)

trunk/Source/WebCore/html/HTMLObjectElement.cpp

@@ -379 +379 @@
         if (!isImageType()) {
             // If we don't think we have an image type anymore, then clear the image from the loader.
-            m_imageLoader->setImage(0);        
-            detach();
-            attach();
+            m_imageLoader->setImage(0);
+            reattach();
             return;
         }

trunk/Source/WebCore/html/HTMLPlugInImageElement.cpp

@@ -125 +125 @@
 {
     // FIXME: Why is this necessary?  Manual re-attach is almost always wrong.
-    if (!useFallbackContent() && needsWidgetUpdate() && renderer() && !isImageType()) {
-        detach();
-        attach();
-    }
+    if (!useFallbackContent() && needsWidgetUpdate() && renderer() && !isImageType())
+        reattach();
     HTMLPlugInElement::recalcStyle(ch);
 }

trunk/Source/WebCore/html/HTMLSelectElement.cpp

@@ -265 +265 @@
         setNeedsValidityCheck();
         if ((oldUsesMenuList != m_data.usesMenuList() || (!oldUsesMenuList && m_data.size() != oldSize)) && attached()) {
-            detach();
-            attach();
+            reattach();
             setRecalcListItems();
         }