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

Source/core/dom/NodeRenderingContext.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All rights reserved.
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  * Copyright (C) 2011 Google Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ skipping 124 matching lines @@
     // however, when I tried adding it, several tests failed.
     for (Node* sibling = NodeRenderingTraversal::previousSibling(m_node); sibling; sibling = NodeRenderingTraversal::previousSibling(sibling)) {
         RenderObject* renderer = sibling->renderer();
         if (renderer && !isRendererReparented(renderer))
             return renderer;
     }
 
     return 0;
 }
 
-RenderObject* NodeRenderingContext::parentRenderer() const

[Julien - ping for review, 2013/05/09 15:30:18]

That's a bad change on its own. Why would parentRenderer modify this?

[Mihai Maerean, 2013/05/09 16:25:55]

The bug was caused by a "race condition": it wasn't clear in the old code when
the NamedFlowThread should be created.
The fix consists in creating the NamedFlowThread when we encounter the 1st
element that needs to be flowed into it.
The parent renderer for such an element is its flow thread. If the flow thread
doesn't exist, it must be created.
This is why this method is not const.

[Julien - ping for review, 2013/05/10 00:13:05]

I will need to read deeper but it seems wrong that a simple getter is now
modifying the object. This is a definite sign that the change is probably going
in the wrong direction and that the architecture isn't accommodating regions as
it should.

+RenderObject* NodeRenderingContext::parentRenderer()
 {
     if (RenderObject* renderer = m_node->renderer())
         return renderer->parent();
 
     if (m_node->isElementNode() && toElement(m_node)->isInTopLayer()) {
         // The parent renderer of top layer elements is the RenderView, but only
         // if the normal parent would have had a renderer.
         // FIXME: This behavior isn't quite right as the spec for top layer
         // only talks about display: none ancestors so putting a <dialog> inside
         // an <optgroup> seems like it should still work even though this check
         // will prevent it.
         if (!m_renderingParent || !m_renderingParent->renderer())
             return 0;
         return m_node->document()->renderView();
     }
 
     if (m_parentFlowRenderer)
         return m_parentFlowRenderer;
 
+    if (m_node->isElementNode() && toElement(m_node)->moveToFlowThreadIsNeeded(m_style)) {
+        moveToFlowThread();
+        return m_parentFlowRenderer;
+    }
+
     return m_renderingParent ? m_renderingParent->renderer() : 0;
 }
 
-bool NodeRenderingContext::shouldCreateRenderer() const

[Julien - ping for review, 2013/05/09 15:30:18]

Ditto.

+bool NodeRenderingContext::shouldCreateRenderer()
 {
     if (!m_node->document()->shouldCreateRenderers())
         return false;
     if (!m_renderingParent)
         return false;
     RenderObject* parentRenderer = this->parentRenderer();
     if (!parentRenderer)
         return false;
-    if (!parentRenderer->canHaveChildren())
-        return false;
+    if (!parentRenderer->canHaveChildren()) {
+        if (parentRenderer->canDOMChildrenHaveRenderParent()) {
+            // In a region, only the children that need to be in a flow thread should have a renderer.
+            bool shouldBeInNamedFlow = m_node->isElementNode() && toElement(m_node)->moveToFlowThreadIsNeeded(m_style);
+            if (!shouldBeInNamedFlow)
+                return false;
+        } else
+            return false;
+    }
+
     if (!m_renderingParent->childShouldCreateRenderer(*this))
         return false;
     return true;
 }
 
 void NodeRenderingContext::moveToFlowThreadIfNeeded()
 {
     ASSERT(m_node->isElementNode());
-    ASSERT(m_style);
-    if (!RuntimeEnabledFeatures::cssRegionsEnabled())
+
+    if (!toElement(m_node)->moveToFlowThreadIsNeeded(m_style))
         return;
 
-    if (m_style->flowThread().isEmpty())
-        return;
+    moveToFlowThread();
+}
 
-    // 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;
+void NodeRenderingContext::moveToFlowThread()
+{
+    ASSERT(m_node->isElementNode());
+    ASSERT(toElement(m_node)->moveToFlowThreadIsNeeded(m_style));
 
-    // FIXME: Do not collect elements if they are in shadow tree.
-    if (m_node->isInShadowTree())
-        return;
-
-    Document* document = m_node->document();
-    if (document->webkitIsFullScreen() && document->webkitCurrentFullScreenElement() == m_node)
-        return;
-
-#if ENABLE(SVG)
-    // 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;
-#endif
-
-    m_flowThread = m_style->flowThread();
+    if (!m_style)
+        m_style = toElement(m_node)->styleForRenderer();
+    ASSERT(m_style);
     ASSERT(m_node->document()->renderView());
     FlowThreadController* flowThreadController = m_node->document()->renderView()->flowThreadController();
-    m_parentFlowRenderer = flowThreadController->ensureRenderFlowThreadWithName(m_flowThread);
+    m_parentFlowRenderer = flowThreadController->ensureRenderFlowThreadWithName(m_style->flowThread());
     flowThreadController->registerNamedFlowContentNode(m_node, m_parentFlowRenderer);
 }
 
 bool NodeRenderingContext::isOnEncapsulationBoundary() const
 {
     return isOnUpperEncapsulationBoundary() || isLowerEncapsulationBoundary(m_parentDetails.insertionPoint()) || isLowerEncapsulationBoundary(m_node->parentNode());
 }
 
 bool NodeRenderingContext::isOnUpperEncapsulationBoundary() const
 {
     return m_node->parentNode() && m_node->parentNode()->isShadowRoot();
 }
 
 void NodeRenderingContext::createRendererForElementIfNeeded()
 {
     ASSERT(!m_node->renderer());
 
     Element* element = toElement(m_node);
 
     if (!shouldCreateRenderer())
         return;
-    m_style = element->styleForRenderer();
+    if (!m_style)
+        m_style = element->styleForRenderer();
     ASSERT(m_style);
 
     moveToFlowThreadIfNeeded();
 
     if (!element->rendererIsNeeded(*this))
         return;
 
     RenderObject* parentRenderer = this->parentRenderer();
     RenderObject* nextRenderer = this->nextRenderer();
 
@@ skipping 56 matching lines @@
     newRenderer->setFlowThreadState(parentRenderer->flowThreadState());
 
     RenderObject* nextRenderer = this->nextRenderer();
     textNode->setRenderer(newRenderer);
     // Parent takes care of the animations, no need to call setAnimatableStyle.
     newRenderer->setStyle(m_style.release());
     parentRenderer->addChild(newRenderer, nextRenderer);
 }
 
 }