Unreviewed, rolling out r140307, r140411, and r140512.
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 23 Jan 2013 18:25:06 +0000 (18:25 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 23 Jan 2013 18:25:06 +0000 (18:25 +0000)
http://trac.webkit.org/changeset/140307
http://trac.webkit.org/changeset/140411
http://trac.webkit.org/changeset/140512
https://bugs.webkit.org/show_bug.cgi?id=107689

Perf regression on DOMDivWalk (bug 106726) (Requested by
falken on #webkit).

Patch by Sheriff Bot <webkit.review.bot@gmail.com> on 2013-01-23

Source/WebCore:

* dom/Element.cpp:
(WebCore::Element::removedFrom):
(WebCore):
(WebCore::Element::isInTopLayer):
(WebCore::Element::setIsInTopLayer):
* dom/Element.h:
* dom/ElementRareData.h:
(ElementRareData):
(WebCore::ElementRareData::isInTopLayer):
(WebCore::ElementRareData::setIsInTopLayer):
(WebCore::ElementRareData::ElementRareData):
* dom/Node.cpp:
* dom/Node.h:
(Node):
* dom/NodeRenderingContext.cpp:
(WebCore::isRendererReparented):
(WebCore::NodeRenderingContext::previousRenderer):
(WebCore::NodeRenderingContext::parentRenderer):
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::isInTopLayer):
(WebCore::RenderLayer::rebuildZOrderLists):

LayoutTests:

* fast/dom/HTMLDialogElement/removed-element-is-removed-from-top-layer-expected.html: Removed.
* fast/dom/HTMLDialogElement/removed-element-is-removed-from-top-layer.html: Removed.
* fast/dom/HTMLDialogElement/top-layer-stacking-correct-order-remove-readd-expected.html: Removed.
* fast/dom/HTMLDialogElement/top-layer-stacking-correct-order-remove-readd.html: Removed.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@140546 268f45cc-cd09-0410-ab3c-d52691b4dbfc

13 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/dom/HTMLDialogElement/removed-element-is-removed-from-top-layer-expected.html [deleted file]
LayoutTests/fast/dom/HTMLDialogElement/removed-element-is-removed-from-top-layer.html [deleted file]
LayoutTests/fast/dom/HTMLDialogElement/top-layer-stacking-correct-order-remove-readd-expected.html [deleted file]
LayoutTests/fast/dom/HTMLDialogElement/top-layer-stacking-correct-order-remove-readd.html [deleted file]
Source/WebCore/ChangeLog
Source/WebCore/dom/Element.cpp
Source/WebCore/dom/Element.h
Source/WebCore/dom/ElementRareData.h
Source/WebCore/dom/Node.cpp
Source/WebCore/dom/Node.h
Source/WebCore/dom/NodeRenderingContext.cpp
Source/WebCore/rendering/RenderLayer.cpp

index 1a95cb4..a4344be 100644 (file)
@@ -1,3 +1,19 @@
+2013-01-23  Sheriff Bot  <webkit.review.bot@gmail.com>
+
+        Unreviewed, rolling out r140307, r140411, and r140512.
+        http://trac.webkit.org/changeset/140307
+        http://trac.webkit.org/changeset/140411
+        http://trac.webkit.org/changeset/140512
+        https://bugs.webkit.org/show_bug.cgi?id=107689
+
+        Perf regression on DOMDivWalk (bug 106726) (Requested by
+        falken on #webkit).
+
+        * fast/dom/HTMLDialogElement/removed-element-is-removed-from-top-layer-expected.html: Removed.
+        * fast/dom/HTMLDialogElement/removed-element-is-removed-from-top-layer.html: Removed.
+        * fast/dom/HTMLDialogElement/top-layer-stacking-correct-order-remove-readd-expected.html: Removed.
+        * fast/dom/HTMLDialogElement/top-layer-stacking-correct-order-remove-readd.html: Removed.
+
 2013-01-23  Rafael Weinstein  <rafaelw@chromium.org>
 
         REGRESSION(r140101): caused debug asserts in fast/forms/associated-element-crash.html and html5lib/run-template.html
diff --git a/LayoutTests/fast/dom/HTMLDialogElement/removed-element-is-removed-from-top-layer-expected.html b/LayoutTests/fast/dom/HTMLDialogElement/removed-element-is-removed-from-top-layer-expected.html
deleted file mode 100644 (file)
index 219b156..0000000
+++ /dev/null
@@ -1,40 +0,0 @@
-<!DOCTYPE html>
-<html>
-<head>
-<script>
-if (window.internals)
-    internals.settings.setDialogElementEnabled(true);
-</script>
-<style>
-.pseudodialog {
-    position: absolute;
-    left: 0; right: 0;
-    margin: auto;
-    border: solid;
-    padding: 1em;
-    background: white;
-    color: black;
-    height: 100px;
-    width: 100px;
-}
-
-#bottomDialog {
-    background-color: blue;
-    top: 100px;
-}
-
-#topDialog {
-    background-color: green;
-    top: 150px;
-    left: 50px;
-}
-</style>
-</head>
-<body>
-<p>Bug <a href="https://bugs.webkit.org/show_bug.cgi?id=105489">105489</a>: Elements must be reattached when inserted/removed from top layer
-<p>The test passes if you see a green rectangle stacked on top of a blue rectangle.
-<div id="bottomDialog" class="pseudodialog"></div>
-<div id="topDialog" class="pseudodialog"></div>
-</body>
-</html>
-
diff --git a/LayoutTests/fast/dom/HTMLDialogElement/removed-element-is-removed-from-top-layer.html b/LayoutTests/fast/dom/HTMLDialogElement/removed-element-is-removed-from-top-layer.html
deleted file mode 100644 (file)
index a6cd37d..0000000
+++ /dev/null
@@ -1,42 +0,0 @@
-<!DOCTYPE html>
-<html>
-<head>
-<script>
-if (window.internals)
-    internals.settings.setDialogElementEnabled(true);
-</script>
-<style>
-dialog {
-    height: 100px;
-    width: 100px;
-}
-
-#bottomDialog {
-    background-color: blue;
-    top: 100px;
-}
-
-#topDialog {
-    background-color: green;
-    top: 150px;
-    left: 50px;
-}
-</style>
-</head>
-<body>
-<p>Bug <a href="https://bugs.webkit.org/show_bug.cgi?id=105489">105489</a>: Elements must be reattached when inserted/removed from top layer
-<p>The test passes if you see a green rectangle stacked on top of a blue rectangle.
-<dialog id="bottomDialog"></dialog>
-<dialog id="topDialog"></dialog>
-<script>
-document.getElementById('topDialog').showModal();
-var bottomDialog = document.getElementById('bottomDialog');
-bottomDialog.showModal();
-bottomDialog.offsetTop;  // force a layout
-var parent = bottomDialog.parentNode;
-parent.removeChild(bottomDialog);
-parent.appendChild(bottomDialog);
-</script>
-</body>
-</html>
-
diff --git a/LayoutTests/fast/dom/HTMLDialogElement/top-layer-stacking-correct-order-remove-readd-expected.html b/LayoutTests/fast/dom/HTMLDialogElement/top-layer-stacking-correct-order-remove-readd-expected.html
deleted file mode 100644 (file)
index b2900d2..0000000
+++ /dev/null
@@ -1,29 +0,0 @@
-<!DOCTYPE html>
-<html>
-<head>
-<script>
-if (window.internals)
-    internals.settings.setDialogElementEnabled(true);
-</script>
-<style>
-.pseudodialog {
-    height: 100px;
-    width: 100px;
-    position: absolute;
-    left: 0; right: 0;
-    margin: auto;
-    border: solid;
-    padding: 1em;
-    background: white;
-    color: black;
-}
-</style>
-</head>
-<body>
-<p>Bug <a href="https://bugs.webkit.org/show_bug.cgi?id=105489">105489</a>: Elements must be reattached when inserted/removed from top layer
-<p>The test passes if you see a green rectangle stacked on top of a blue rectangle.
-
-<div class="pseudodialog" style="top: 100px; background-color: blue"></div>
-<div class="pseudodialog" style="top: 150px; left: 50px; background-color: green"></div>
-</body>
-</html>
diff --git a/LayoutTests/fast/dom/HTMLDialogElement/top-layer-stacking-correct-order-remove-readd.html b/LayoutTests/fast/dom/HTMLDialogElement/top-layer-stacking-correct-order-remove-readd.html
deleted file mode 100644 (file)
index 87694ce..0000000
+++ /dev/null
@@ -1,42 +0,0 @@
-<!DOCTYPE html>
-<html>
-<head>
-<script>
-if (window.internals)
-    internals.settings.setDialogElementEnabled(true);
-</script>
-<style>
-dialog {
-    height: 100px;
-    width: 100px;
-}
-
-#bottomDialog {
-    background-color: blue;
-    top: 100px;
-}
-
-#topDialog {
-    background-color: green;
-    top: 150px;
-    left: 50px;
-}
-</style>
-</head>
-<body>
-<p>Bug <a href="https://bugs.webkit.org/show_bug.cgi?id=105489">105489</a>: Elements must be reattached when inserted/removed from top layer
-<p>The test passes if you see a green rectangle stacked on top of a blue rectangle.
-
-<dialog id="topDialog"></dialog>
-<dialog id="bottomDialog"></dialog>
-<script>
-var topDialog = document.getElementById('topDialog');
-var bottomDialog = document.getElementById('bottomDialog');
-topDialog.showModal();
-bottomDialog.showModal();
-topDialog.offsetTop;  // force a layout
-topDialog.close();
-topDialog.showModal();
-</script>
-</body>
-</html>
index 6fbaadd..d21538f 100644 (file)
@@ -1,3 +1,36 @@
+2013-01-23  Sheriff Bot  <webkit.review.bot@gmail.com>
+
+        Unreviewed, rolling out r140307, r140411, and r140512.
+        http://trac.webkit.org/changeset/140307
+        http://trac.webkit.org/changeset/140411
+        http://trac.webkit.org/changeset/140512
+        https://bugs.webkit.org/show_bug.cgi?id=107689
+
+        Perf regression on DOMDivWalk (bug 106726) (Requested by
+        falken on #webkit).
+
+        * dom/Element.cpp:
+        (WebCore::Element::removedFrom):
+        (WebCore):
+        (WebCore::Element::isInTopLayer):
+        (WebCore::Element::setIsInTopLayer):
+        * dom/Element.h:
+        * dom/ElementRareData.h:
+        (ElementRareData):
+        (WebCore::ElementRareData::isInTopLayer):
+        (WebCore::ElementRareData::setIsInTopLayer):
+        (WebCore::ElementRareData::ElementRareData):
+        * dom/Node.cpp:
+        * dom/Node.h:
+        (Node):
+        * dom/NodeRenderingContext.cpp:
+        (WebCore::isRendererReparented):
+        (WebCore::NodeRenderingContext::previousRenderer):
+        (WebCore::NodeRenderingContext::parentRenderer):
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::isInTopLayer):
+        (WebCore::RenderLayer::rebuildZOrderLists):
+
 2013-01-23  Dmitry Gozman  <dgozman@chromium.org>
 
         Web Inspector: allow user to resize inspector window by dragging the toolbar
index 42bfb4a..5d8f945 100644 (file)
@@ -1182,22 +1182,13 @@ void Element::removedFrom(ContainerNode* insertionPoint)
     if (Element* after = pseudoElement(AFTER))
         after->removedFrom(insertionPoint);
 
-    // FIXME: Fullscreen should be migrated to use the top layer (bug 107617). Then this can just be a single check.
-    // The purpose of the outer if statement is to quickly determine whether we must do the additional
-    // checks in a slow path.
-#if ENABLE(DIALOG_ELEMENT) || ENABLE(FULLSCREEN_API)
-    if (hasRareData() || isInTopLayer()) {
 #if ENABLE(DIALOG_ELEMENT)
-        if (isInTopLayer())
-            document()->removeFromTopLayer(this);
+    setIsInTopLayer(false);
 #endif
 #if ENABLE(FULLSCREEN_API)
-        if (containsFullScreenElement())
-            setContainsFullScreenElementOnAncestorsCrossingFrameBoundaries(false);
+    if (containsFullScreenElement())
+        setContainsFullScreenElementOnAncestorsCrossingFrameBoundaries(false);
 #endif
-    }
-#endif
-
 #if ENABLE(POINTER_LOCK)
     if (document()->page())
         document()->page()->pointerLockController()->elementRemoved(this);
@@ -2402,6 +2393,19 @@ void Element::setContainsFullScreenElementOnAncestorsCrossingFrameBoundaries(boo
 }
 #endif    
 
+#if ENABLE(DIALOG_ELEMENT)
+bool Element::isInTopLayer() const
+{
+    return hasRareData() && elementRareData()->isInTopLayer();
+}
+
+void Element::setIsInTopLayer(bool inTopLayer)
+{
+    ensureElementRareData()->setIsInTopLayer(inTopLayer);
+    setNeedsStyleRecalc(SyntheticStyleChange);
+}
+#endif
+
 #if ENABLE(POINTER_LOCK)
 void Element::webkitRequestPointerLock()
 {
index 8475b60..6d18dcc 100644 (file)
@@ -464,6 +464,11 @@ public:
     void webkitRequestFullscreen();
 #endif
 
+#if ENABLE(DIALOG_ELEMENT)
+    bool isInTopLayer() const;
+    void setIsInTopLayer(bool);
+#endif
+
 #if ENABLE(POINTER_LOCK)
     void webkitRequestPointerLock();
 #endif
index 5bf8254..b347b28 100644 (file)
@@ -65,6 +65,11 @@ public:
     void setContainsFullScreenElement(bool value) { m_containsFullScreenElement = value; }
 #endif
 
+#if ENABLE(DIALOG_ELEMENT)
+    bool isInTopLayer() const { return m_isInTopLayer; }
+    void setIsInTopLayer(bool value) { m_isInTopLayer = value; }
+#endif
+
     bool childrenAffectedByHover() const { return m_childrenAffectedByHover; }
     void setChildrenAffectedByHover(bool value) { m_childrenAffectedByHover = value; }
     bool childrenAffectedByActive() const { return m_childrenAffectedByActive; }
@@ -130,6 +135,9 @@ private:
 #if ENABLE(FULLSCREEN_API)
     unsigned m_containsFullScreenElement : 1;
 #endif
+#if ENABLE(DIALOG_ELEMENT)
+    unsigned m_isInTopLayer : 1;
+#endif
 #if ENABLE(SVG)
     unsigned m_hasPendingResources : 1;
 #endif
@@ -177,6 +185,9 @@ inline ElementRareData::ElementRareData(RenderObject* renderer)
 #if ENABLE(FULLSCREEN_API)
     , m_containsFullScreenElement(false)
 #endif
+#if ENABLE(DIALOG_ELEMENT)
+    , m_isInTopLayer(false)
+#endif
 #if ENABLE(SVG)
     , m_hasPendingResources(false)
 #endif
index 7310fc3..ef264d9 100644 (file)
@@ -2620,17 +2620,6 @@ void Node::decrementConnectedSubframeCount(unsigned amount)
     rareData()->decrementConnectedSubframeCount(amount);
 }
 
-#if ENABLE(DIALOG_ELEMENT)
-void Node::setIsInTopLayer(bool isInTopLayer)
-{
-    setFlag(isInTopLayer, IsInTopLayer);
-
-    // We must ensure a reattach occurs so the renderer is inserted in the correct sibling order under RenderView according to its
-    // top layer position, or in its usual place if not in the top layer.
-    reattachIfAttached();
-}
-#endif
-
 void Node::registerScopedHTMLStyleChild()
 {
     setHasScopedHTMLStyleChild(true);
index 16ec873..161e5ec 100644 (file)
@@ -682,14 +682,6 @@ public:
     void incrementConnectedSubframeCount(unsigned amount = 1);
     void decrementConnectedSubframeCount(unsigned amount = 1);
 
-#if ENABLE(DIALOG_ELEMENT)
-    bool isInTopLayer() const { return getFlag(IsInTopLayer); }
-    void setIsInTopLayer(bool);
-#else
-    bool isInTopLayer() const { return false; }
-    void setIsInTopLayer(bool) { }
-#endif
-
 private:
     enum NodeFlags {
         IsTextFlag = 1,
@@ -727,14 +719,11 @@ private:
         V8CollectableDuringMinorGCFlag = 1 << 24,
         NeedsShadowTreeWalkerFlag = 1 << 25,
         IsInShadowTreeFlag = 1 << 26,
-#if ENABLE(DIALOG_ELEMENT)
-        IsInTopLayer = 1 << 27,
-#endif
 
         DefaultNodeFlags = IsParsingChildrenFinishedFlag
     };
 
-    // 4 bits remaining
+    // 5 bits remaining
 
     bool getFlag(NodeFlags mask) const { return m_nodeFlags & mask; }
     void setFlag(bool f, NodeFlags mask) const { m_nodeFlags = (m_nodeFlags & ~mask) | (-(int32_t)f & mask); } 
index 2dbf3a4..bae51a7 100644 (file)
@@ -75,10 +75,12 @@ NodeRenderingContext::~NodeRenderingContext()
 
 static bool isRendererReparented(const RenderObject* renderer)
 {
-    if (renderer->node()->isElementNode() && renderer->style() && !renderer->style()->flowThread().isEmpty())
+    if (!renderer->node()->isElementNode())
+        return false;
+    if (renderer->style() && !renderer->style()->flowThread().isEmpty())
         return true;
 #if ENABLE(DIALOG_ELEMENT)
-    if (renderer->node()->isInTopLayer())
+    if (toElement(renderer->node())->isInTopLayer())
         return true;
 #endif
     return false;
@@ -129,7 +131,7 @@ RenderObject* NodeRenderingContext::previousRenderer() const
     // FIXME: This doesn't work correctly for things in the top layer that are
     // display: none. We'd need to duplicate the logic in nextRenderer, but since
     // nothing needs that yet just assert.
-    ASSERT(!m_node->isInTopLayer());
+    ASSERT(!m_node->isElementNode() || !toElement(m_node)->isInTopLayer());
 #endif
 
     if (m_parentFlowRenderer)
@@ -152,7 +154,7 @@ RenderObject* NodeRenderingContext::parentRenderer() const
         return renderer->parent();
 
 #if ENABLE(DIALOG_ELEMENT)
-    if (m_node->isInTopLayer()) {
+    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
index 9e2992a..b89ccd3 100644 (file)
@@ -4014,7 +4014,7 @@ Node* RenderLayer::enclosingElement() const
 bool RenderLayer::isInTopLayer() const
 {
     Node* node = renderer()->node();
-    return node && node->isInTopLayer();
+    return node && node->isElementNode() && toElement(node)->isInTopLayer();
 }
 
 bool RenderLayer::isInTopLayerSubtree() const
@@ -5167,7 +5167,8 @@ void RenderLayer::rebuildZOrderLists()
     if (isRootLayer()) {
         RenderObject* view = renderer()->view();
         for (RenderObject* child = view->firstChild(); child; child = child->nextSibling()) {
-            if (child->node() && child->node()->isInTopLayer()) {
+            Element* childElement = (child->node() && child->node()->isElementNode()) ? toElement(child->node()) : 0;
+            if (childElement && childElement->isInTopLayer()) {
                 RenderLayer* layer = toRenderLayerModelObject(child)->layer();
                 m_posZOrderList->append(layer);
             }