Make abspos <dialog>'s containing block be the ICB.

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 aca7e5f678957806584ebd4a7f6f2f5bb89b0497..e871b8958c1fd95233c176c20071f8c6da4c634a 100644
--- a/Source/core/dom/NodeRenderingContext.cpp
+++ b/Source/core/dom/NodeRenderingContext.cpp
@@ -79,7 +79,7 @@ static bool isRendererReparented(const RenderObject* renderer)
         return false;
     if (renderer->style() && !renderer->style()->flowThread().isEmpty())
         return true;
-    if (toElement(renderer->node())->isInTopLayer())
+    if (toElement(renderer->node())->shouldBeReparentedUnderRenderView(renderer->style()))
         return true;
     return false;
 }
@@ -90,6 +90,8 @@ RenderObject* NodeRenderingContext::nextRenderer() const
         return renderer->nextSibling();
 
     Element* element = m_node->isElementNode() ? toElement(m_node) : 0;
+    if (element && element->shouldBeReparentedUnderRenderView(m_style.get()) && !element->isInTopLayer())

[Julien - ping for review, 2013/05/09 18:46:58]

I misled you here. I was thinking about checking element->isDialogElement() &&
!element->isInTopLayer() as it would totally explain the intent (probably with a
comment about why we ignore them)

I would love some explanations about why we want a <dialog> outside the top
layer to be always be appended last instead of being where its renderer should
be (ie before its next sibling's renderer).

[falken, 2013/05/10 05:50:25]

I see now how the code is confusing and probably didn't fully understand myself
until you asked. I've added a FIXME comment and explain in more detail below.

The problem with reparenting is that parentRenderer must return RenderView, but
what nextRenderer does is a bigger task. I think we want two things:
(1) Top layer renderers are inserted in top layer order. This allows the top
layer rendering to work: RenderLayer collects the top layer elements in order
and they are painted from bottom to top.
(2) Non-top-layer renderers are inserted in DOM tree order. This means they
would be painted in DOM tree order. I think this is not explicitly required by
the spec (as reparenting is not a spec concept) but seems like the expected
behavior.

The implementation already does (1). With this patch, we still do (1) correctly
but not (2). It's not completely obvious the right way to do (2). We must find
the renderer of the next reparented element in tree order. We can't just use
NodeRenderingTraversal since it doesn't know about reparenting. The correct
thing to do may be to make NodeRenderingTraversal reparenting-aware, and
traverse the tree until we find the next reparented element, get its renderer,
and return it.

If possible, I'd like to handle (2) in a subsequent patch. The accomplishment of
this patch is to make non-modal dialog's containing block the ICB; the only
issue is that the stacking order of them relative to each other may be random.

+        return 0;
     if (element && element->isInTopLayer()) {
         const Vector<RefPtr<Element> >& topLayerElements = element->document()->topLayerElements();
         size_t position = topLayerElements.find(element);
@@ -123,10 +125,10 @@ RenderObject* NodeRenderingContext::previousRenderer() const
     if (RenderObject* renderer = m_node->renderer())
         return renderer->previousSibling();
 
-    // FIXME: This doesn't work correctly for things in the top layer that are
+    // FIXME: This doesn't work correctly for reparented elements that are
     // display: none. We'd need to duplicate the logic in nextRenderer, but since
     // nothing needs that yet just assert.
-    ASSERT(!m_node->isElementNode() || !toElement(m_node)->isInTopLayer());
+    ASSERT(!m_node->isElementNode() || !toElement(m_node)->shouldBeReparentedUnderRenderView(m_style.get()));
 
     if (m_parentFlowRenderer)
         return m_parentFlowRenderer->previousRendererForNode(m_node);
@@ -147,8 +149,8 @@ RenderObject* NodeRenderingContext::parentRenderer() const
     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 (m_node->isElementNode() && toElement(m_node)->shouldBeReparentedUnderRenderView(m_style.get())) {
+        // The parent renderer of reparented 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