[CSS Regions] Elements in a region should be assignable to a named flow

Source/core/dom/NodeRenderingContext.cpp

Index: Source/core/dom/NodeRenderingContext.cpp
diff --git a/Source/core/dom/NodeRenderingContext.cpp b/Source/core/dom/NodeRenderingContext.cpp
index 2138acf8d7992e20e76dc902f663eacb4cbe0fd2..778b682c78bd4bade8182f9947b99dd9336fc8d2 100644
--- a/Source/core/dom/NodeRenderingContext.cpp
+++ b/Source/core/dom/NodeRenderingContext.cpp
@@ -189,47 +189,86 @@ bool NodeRenderingContext::shouldCreateRenderer() const
     RenderObject* parentRenderer = this->parentRenderer();
     if (!parentRenderer)
         return false;
-    if (!parentRenderer->canHaveChildren())
-        return false;
+    if (!parentRenderer->canHaveChildren()) {
+        if (parentRenderer->canDOMChildrenHaveRenderParent()) {
+            if (!shouldMoveToFlowThread())
+                return false;
+        } else {
+            return false;
+        }
+    }
     if (!m_renderingParent->childShouldCreateRenderer(*this))
         return false;
     return true;
 }
 
-void NodeRenderingContext::moveToFlowThreadIfNeeded()
+bool NodeRenderingContext::shouldMoveToFlowThread() const
 {
-    ASSERT(m_node->isElementNode());
-    ASSERT(m_style);
+    if (!m_node->isElementNode())
+        return false;
+    Element* element = toElement(m_node);
+
     if (!RuntimeEnabledFeatures::cssRegionsEnabled())
-        return;
+        return false;
 
-    if (m_style->flowThread().isEmpty())
-        return;
+    if (element && FullscreenController::isActiveFullScreenElement(element))
+        return false;
+
+    if (m_node->isInShadowTree())
+        return false;
 
     // As per http://dev.w3.org/csswg/css3-regions/#flow-into, pseudo-elements such as ::first-line, ::first-letter, ::before or ::after
     // cannot be directly collected into a named flow.
     if (m_node->isPseudoElement())
-        return;
-
-    // FIXME: Do not collect elements if they are in shadow tree.
-    if (m_node->isInShadowTree())
-        return;
-
-    if (m_node->isElementNode() && FullscreenController::isActiveFullScreenElement(toElement(m_node)))
-        return;
+        return false;
 
     // Allow only svg root elements to be directly collected by a render flow thread.
     if (m_node->isSVGElement()
         && (!(m_node->hasTagName(SVGNames::svgTag) && m_node->parentNode() && !m_node->parentNode()->isSVGElement())))
+        return false;
+
+    if (!m_style)
+        m_style = element->styleForRenderer();
+    if (!m_style)
+        return false;
+
+    if (m_style->flowThread().isEmpty())
+        return false;
+
+    if (m_node->document()->renderView()->flowThreadController()->isContentNodeRegisteredWithAnyNamedFlow(m_node))
+        return false;
+
+    return true;
+}
+
+void NodeRenderingContext::moveToFlowThreadIfNeeded()
+{
+    if (!shouldMoveToFlowThread())
         return;
 
-    m_flowThread = m_style->flowThread();
+    moveToFlowThread();
+}
+
+void NodeRenderingContext::moveToFlowThread()
+{
+    ASSERT(m_node->isElementNode());
+    ASSERT(shouldMoveToFlowThread());
+
     ASSERT(m_node->document()->renderView());
     FlowThreadController* flowThreadController = m_node->document()->renderView()->flowThreadController();
-    m_parentFlowRenderer = flowThreadController->ensureRenderFlowThreadWithName(m_flowThread);
+    m_parentFlowRenderer = flowThreadController->ensureRenderFlowThreadWithName(getStyleForRenderer()->flowThread());
     flowThreadController->registerNamedFlowContentNode(m_node, m_parentFlowRenderer);
 }
 
+PassRefPtr<RenderStyle> NodeRenderingContext::getStyleForRenderer() const
+{
+    ASSERT(m_node->isElementNode());
+    if (!m_style)
+        m_style = toElement(m_node)->styleForRenderer();
+    ASSERT(m_style);
+    return m_style;
+}
+
 bool NodeRenderingContext::isOnEncapsulationBoundary() const
 {
     return isOnUpperEncapsulationBoundary() || isLowerEncapsulationBoundary(m_parentDetails.insertionPoint()) || isLowerEncapsulationBoundary(m_node->parentNode());
@@ -244,15 +283,30 @@ void NodeRenderingContext::createRendererForElementIfNeeded()
 {
     ASSERT(!m_node->renderer());
 
+    ASSERT(m_node->isElementNode());
     Element* element = toElement(m_node);
 
+    // Look for nodes inside a region that are flowed into a 2nd region.
+    // This avoids doing a style resolution for all nodes.
+    bool haveTestedForMoveToFlowThread = false;
+    for (const Element* parentElement = m_node->parentElement(); parentElement; parentElement = parentElement->parentElement()) {

[esprehn, 2013/06/17 21:41:02]

Woah, this isn't right. This is going to cause perf regressions since you walk
up the tree. It's also not correct since it's not Shadow DOM aware.

+        if (const RenderObject* rendererParentElement = parentElement->renderer()) {
+            if (!rendererParentElement->canHaveChildren() && rendererParentElement->canDOMChildrenHaveRenderParent()) {
+                moveToFlowThreadIfNeeded();
+                haveTestedForMoveToFlowThread = true;
+                break;
+            }
+        }
+    }
+
     if (!shouldCreateRenderer())
         return;
-    m_style = element->styleForRenderer();
-    ASSERT(m_style);
 
-    moveToFlowThreadIfNeeded();
+    // The usual case when elements collected in a flow are not children of a region.
+    if (!haveTestedForMoveToFlowThread)
+        moveToFlowThreadIfNeeded();
 
+    getStyleForRenderer(); // rendererIsNeeded will call NodeRenderingContext::style()

[esprehn, 2013/06/17 21:41:02]

Calling a method with a return value and ignoring it is bad. Lets not do this.

[maerean, 2013/06/17 21:58:45]

I have tried using getStyleForRenderer in Element::rendererIsNeeded but
1. that exploded in a big number of changes since rendererIsNeeded is overridden
in the SVG element classes.
2. that is not correct since NodeRenderingContent::m_style can store many
things: styleForRenderer(), document->styleResolver()->defaultStyleForElement()
and parentRenderer->style().

     if (!element->rendererIsNeeded(*this))
         return;
 
@@ -260,7 +314,7 @@ void NodeRenderingContext::createRendererForElementIfNeeded()
     RenderObject* nextRenderer = this->nextRenderer();
 
     Document* document = element->document();
-    RenderObject* newRenderer = element->createRenderer(document->renderArena(), m_style.get());
+    RenderObject* newRenderer = element->createRenderer(document->renderArena(), getStyleForRenderer().get());
     if (!newRenderer)
         return;