Changeset 155116 in webkit

Timestamp:

Sep 5, 2013, 7:45:58 AM (11 years ago)

Author:

Antti Koivisto

Message:

Call createTextRenderersForSiblingsAfterAttachIfNeeded only for the attach root
​https://bugs.webkit.org/show_bug.cgi?id=120770

Reviewed by Andreas Kling.

There is no need to call this during recursive attach as siblings are going to be attached normally anyway.
Move call sites to attach roots only.

• style/StyleResolveTree.cpp:

(WebCore::Style::createTextRenderersForSiblingsAfterAttachIfNeeded):

Factor to take reference and do the inital tests itself.

(WebCore::Style::createTextRendererIfNeeded):
(WebCore::Style::updateTextRendererAfterContentChange):
(WebCore::Style::attachChildren):

Also tightened the condition where previously attached children may be encountered.

(WebCore::Style::attachRenderTree):
(WebCore::Style::resolveLocal):
(WebCore::Style::updateTextStyle):
(WebCore::Style::reattachRenderTree):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• style/StyleResolveTree.cpp (modified) (10 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-09-05  Antti Koivisto  <antti@apple.com>
+
+        Call createTextRenderersForSiblingsAfterAttachIfNeeded only for the attach root
+        https://bugs.webkit.org/show_bug.cgi?id=120770
+
+        Reviewed by Andreas Kling.
+
+        There is no need to call this during recursive attach as siblings are going to be attached normally anyway.
+        Move call sites to attach roots only.
+
+        * style/StyleResolveTree.cpp:
+        (WebCore::Style::createTextRenderersForSiblingsAfterAttachIfNeeded):
+        
+            Factor to take reference and do the inital tests itself.
+
+        (WebCore::Style::createTextRendererIfNeeded):
+        (WebCore::Style::updateTextRendererAfterContentChange):
+        (WebCore::Style::attachChildren):
+        
+            Also tightened the condition where previously attached children may be encountered.
+
+        (WebCore::Style::attachRenderTree):
+        (WebCore::Style::resolveLocal):
+        (WebCore::Style::updateTextStyle):
+        (WebCore::Style::reattachRenderTree):
+
 2013-09-05  Enrique Ocaña González  <eocanha@igalia.com>
 

trunk/Source/WebCore/style/StyleResolveTree.cpp

@@ -281 +281 @@
 }
 
-static void createTextRenderersForSiblingsAfterAttachIfNeeded(Node* sibling)
-{
-    ASSERT(sibling->previousSibling());
-    ASSERT(sibling->previousSibling()->renderer());
-    ASSERT(!sibling->renderer());
-    ASSERT(sibling->attached());
+static void createTextRenderersForSiblingsAfterAttachIfNeeded(Node& node)
+{
+    if (!node.renderer())
+        return;
     // If this node got a renderer it may be the previousRenderer() of sibling text nodes and thus affect the
     // result of Text::textRendererIsNeeded() for those nodes.
-    for (; sibling; sibling = sibling->nextSibling()) {
+    for (Node* sibling = node.nextSibling(); sibling; sibling = sibling->nextSibling()) {
         if (sibling->renderer())
             break;
@@ -296 +294 @@
         if (!sibling->isTextNode())
             continue;
-        ASSERT(!sibling->renderer());
         attachTextRenderer(*toText(sibling));
         // If we again decided not to create a renderer for next, we can bail out the loop,
@@ -386 +383 @@
     newRenderer->setStyle(style.release());
     parentRenderer->addChild(newRenderer, nextRenderer);
-
-    Node* sibling = textNode.nextSibling();
-    if (sibling && !sibling->renderer() && sibling->attached())
-        createTextRenderersForSiblingsAfterAttachIfNeeded(sibling);
 }
 
@@ -415 +408 @@
     if (!textRenderer) {
         attachTextRenderer(textNode);
+        createTextRenderersForSiblingsAfterAttachIfNeeded(textNode);
         return;
     }
@@ -426 +420 @@
 }
 
-#ifndef NDEBUG
-static bool childAttachedAllowedWhenAttachingChildren(ContainerNode& node)
-{
-    if (node.isShadowRoot())
-        return true;
-    if (node.isInsertionPoint())
-        return true;
-    if (node.isElementNode() && toElement(&node)->shadowRoot())
-        return true;
-    return false;
-}
-#endif
-
 static void attachChildren(ContainerNode& current)
 {
     for (Node* child = current.firstChild(); child; child = child->nextSibling()) {
-        ASSERT(!child->attached() || childAttachedAllowedWhenAttachingChildren(current));
+        ASSERT(!child->attached() || current.shadowRoot());
         if (child->attached())
             continue;
@@ -494 +475 @@
 
     attachChildren(current);
-
-    Node* sibling = current.nextSibling();
-    if (current.renderer() && sibling && !sibling->renderer() && sibling->attached())
-        createTextRenderersForSiblingsAfterAttachIfNeeded(sibling);
 
     current.setAttached(true);
@@ -613 +590 @@
             detachRenderTree(current, ReattachDetach);
         attachRenderTree(current, newStyle.get());
+        createTextRenderersForSiblingsAfterAttachIfNeeded(current);
+
         return Detach;
     }
@@ -653 +632 @@
     if (renderer)
         renderer->setText(text.dataImpl());
-    else
+    else {
         attachTextRenderer(text);
+        createTextRenderersForSiblingsAfterAttachIfNeeded(text);
+    }
     text.clearNeedsStyleRecalc();
 }
@@ -837 +818 @@
 {
     attachRenderTree(element, nullptr);
+    createTextRenderersForSiblingsAfterAttachIfNeeded(element);
 }
 
@@ -853 +835 @@
     if (current.attached())
         detachRenderTree(current, ReattachDetach);
-    attachRenderTree(current, nullptr);
-}
-
-}
-}
+    attachRenderTree(current);
+}
+
+}
+}