Changeset 90536 in webkit

Timestamp:

Jul 6, 2011 9:22:17 PM (13 years ago)

Author:

morrita@google.com

Message:

2011-07-06 MORITA Hajime <​morrita@google.com>

[ShadowContentElement] Redundant RenderText objects are created on the content boundaries.
​https://bugs.webkit.org/show_bug.cgi?id=63977

Reviewed by Dimitri Glazkov.

This test have two set of fixture DOM tree:
One is for shadow tree which has one content element,
another is for light DOM tree, whose root is to have a shadow DOM.

Then the test makes 2 complete DOM trees (DOM tree with shadow and
flattened dom) for each combination of the fixture items. And
compare their layout result.

• fast/dom/shadow/content-element-renderers-expected.txt: Added.
• fast/dom/shadow/content-element-renderers.html: Added.

2011-07-06 MORITA Hajime <​morrita@google.com>

[ShadowContentElement] Redundant RenderText objects are created on the content boundaries.
​https://bugs.webkit.org/show_bug.cgi?id=63977

Reviewed by Dimitri Glazkov.

NodeRenderingContext::nextRenderer() and previousRenderer() didn't work well
when it crosses content elements:

• It doesn't step into forwarded children of ShadowContentElement.
• It doesn't step out from traversing forwarded children to neighbors of the content element of that forwarded chidren.

This change makes it to step into and out from content element.

Test: fast/dom/shadow/content-element-renderers.html

• dom/NodeRenderingContext.cpp: (WebCore::firstRendererOf): Added (WebCore::lastRendererOf): Added (WebCore::NodeRenderingContext::nextRenderer): (WebCore::NodeRenderingContext::previousRenderer):

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/dom/shadow/content-element-renderers-expected.txt (added)
• LayoutTests/fast/dom/shadow/content-element-renderers.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/NodeRenderingContext.cpp (modified) (5 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2011-07-06  MORITA Hajime  <morrita@google.com>
+
+        [ShadowContentElement] Redundant RenderText objects are created on the content boundaries.
+        https://bugs.webkit.org/show_bug.cgi?id=63977
+
+        Reviewed by Dimitri Glazkov.
+
+        This test have two set of fixture DOM tree: 
+        One is for shadow tree which has one content element,
+        another is for light DOM tree, whose root is to have a shadow DOM.
+        
+        Then the test makes 2 complete DOM trees (DOM tree with shadow and
+        flattened dom) for each combination of the fixture items. And
+        compare their layout result.
+        
+        * fast/dom/shadow/content-element-renderers-expected.txt: Added.
+        * fast/dom/shadow/content-element-renderers.html: Added.
+
 2011-07-06  Juan C. Montemayor  <jmont@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-07-06  MORITA Hajime  <morrita@google.com>
+
+        [ShadowContentElement] Redundant RenderText objects are created on the content boundaries.
+        https://bugs.webkit.org/show_bug.cgi?id=63977
+
+        Reviewed by Dimitri Glazkov.
+
+        NodeRenderingContext::nextRenderer() and previousRenderer() didn't work well
+        when it crosses content elements:
+        - It doesn't step into forwarded children of ShadowContentElement.
+        - It doesn't step out from traversing forwarded children to
+          neighbors of the content element of that forwarded chidren.
+        
+        This change makes it to step into and out from content element.
+        
+        Test: fast/dom/shadow/content-element-renderers.html
+
+        * dom/NodeRenderingContext.cpp:
+        (WebCore::firstRendererOf): Added
+        (WebCore::lastRendererOf): Added
+        (WebCore::NodeRenderingContext::nextRenderer):
+        (WebCore::NodeRenderingContext::previousRenderer):
+
 2011-07-06  Gyuyoung Kim  <gyuyoung.kim@samsung.com>
 

trunk/Source/WebCore/dom/NodeRenderingContext.cpp

@@ -131 +131 @@
 }
 
+static RenderObject* firstRendererOf(ShadowContentElement* parent)
+{
+    size_t inclusionCount = parent->inclusionCount();
+    for (size_t i = 0; i < inclusionCount; ++i) {
+        Node* candidate = parent->inclusionAt(i);
+        if (RenderObject* renderer = candidate->renderer())
+            return renderer;
+    }
+
+    return 0;
+}
+
+static RenderObject* lastRendererOf(ShadowContentElement* parent)
+{
+    size_t inclusionCount = parent->inclusionCount();
+    for (size_t i = 0; i < inclusionCount; ++i) {
+        Node* candidate = parent->inclusionAt(inclusionCount - i - 1);
+        if (RenderObject* renderer = candidate->renderer())
+            return renderer;
+    }
+
+    return 0;
+}
+
 RenderObject* NodeRenderingContext::nextRenderer() const
 {
@@ -137 +161 @@
         return renderer->nextSibling();
 
-    if (m_phase == AttachContentForwarded)
-        return nextRendererOf(m_contentElement, m_node);
+    if (m_phase == AttachContentForwarded) {
+        if (RenderObject* found = nextRendererOf(m_contentElement, m_node))
+            return found;
+        return NodeRenderingContext(m_contentElement).nextRenderer();
+    }
 
     // Avoid an O(n^2) problem with this function by not checking for
@@ -148 +175 @@
         if (node->renderer())
             return node->renderer();
+        if (node->isContentElement()) {
+            if (RenderObject* first = firstRendererOf(toShadowContentElement(node)))
+                return first;
+        }
     }
 
@@ -159 +190 @@
         return renderer->previousSibling();
 
-    if (m_phase == AttachContentForwarded)
-        return previousRendererOf(m_contentElement, m_node);
+    if (m_phase == AttachContentForwarded) {
+        if (RenderObject* found = previousRendererOf(m_contentElement, m_node))
+            return found;
+        return NodeRenderingContext(m_contentElement).previousRenderer();
+    }
 
     // FIXME: We should have the same O(N^2) avoidance as nextRenderer does
@@ -167 +201 @@
         if (node->renderer())
             return node->renderer();
+        if (node->isContentElement()) {
+            if (RenderObject* last = lastRendererOf(toShadowContentElement(node)))
+                return last;
+        }
     }