Changeset 136575 in webkit

Timestamp:

Dec 4, 2012 2:43:26 PM (11 years ago)

Author:

dpranke@chromium.org

Message:

setIsInTopLayer is not really a setter
​https://bugs.webkit.org/show_bug.cgi?id=103912

Patch by Elliott Sprehn <​esprehn@chromium.org> on 2012-12-04
Reviewed by Eric Seidel.

Element::setInTopLayer isn't just a setter, it also adds the element
to the top layer vector in the document which changes it's display and
causes a reattach.

To make this more obvious make HTMLDialogElement use the methods on
Document and make those update the state boolean.

There also doesn't seem to be any reason for setInTopLayer
or isInTopLayer to be virtual, it appears it was a typo.

No new tests, just refactoring.

• dom/Document.cpp:

(WebCore::Document::addToTopLayer):
(WebCore::Document::removeFromTopLayer):

• dom/Element.cpp:

(WebCore::Element::setIsInTopLayer):

• dom/Element.h:
• html/HTMLDialogElement.cpp:

(WebCore::HTMLDialogElement::close):
(WebCore::HTMLDialogElement::showModal):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• dom/Document.cpp (modified) (1 diff)
• dom/Element.cpp (modified) (1 diff)
• dom/Element.h (modified) (1 diff)
• html/HTMLDialogElement.cpp (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-12-04  Elliott Sprehn  <esprehn@chromium.org>
+
+        setIsInTopLayer is not really a setter
+        https://bugs.webkit.org/show_bug.cgi?id=103912
+
+        Reviewed by Eric Seidel.
+
+        Element::setInTopLayer isn't just a setter, it also adds the element
+        to the top layer vector in the document which changes it's display and
+        causes a reattach.
+
+        To make this more obvious make HTMLDialogElement use the methods on
+        Document and make those update the state boolean.
+
+        There also doesn't seem to be any reason for setInTopLayer
+        or isInTopLayer to be virtual, it appears it was a typo.
+
+        No new tests, just refactoring.
+
+        * dom/Document.cpp:
+        (WebCore::Document::addToTopLayer):
+        (WebCore::Document::removeFromTopLayer):
+        * dom/Element.cpp:
+        (WebCore::Element::setIsInTopLayer):
+        * dom/Element.h:
+        * html/HTMLDialogElement.cpp:
+        (WebCore::HTMLDialogElement::close):
+        (WebCore::HTMLDialogElement::showModal):
+
 2012-12-04  Elliott Sprehn  <esprehn@chromium.org>
 

trunk/Source/WebCore/dom/Document.cpp

@@ -5426 +5426 @@
 void Document::addToTopLayer(Element* element)
 {
+    if (element->isInTopLayer())
+        return;
     ASSERT(!m_topLayerElements.contains(element));
     m_topLayerElements.append(element);
+    element->setIsInTopLayer(true);
 }
 
 void Document::removeFromTopLayer(Element* element)
 {
+    if (!element->isInTopLayer())
+        return;
     size_t position = m_topLayerElements.find(element);
     ASSERT(position != notFound);
     m_topLayerElements.remove(position);
+    element->setIsInTopLayer(false);
 }
 #endif

trunk/Source/WebCore/dom/Element.cpp

@@ -2231 +2231 @@
 void Element::setIsInTopLayer(bool inTopLayer)
 {
-    if (isInTopLayer() == inTopLayer)
-        return;
-
     ensureElementRareData()->setIsInTopLayer(inTopLayer);
-
-    if (inTopLayer)
-        document()->addToTopLayer(this);
-    else
-        document()->removeFromTopLayer(this);
     setNeedsStyleRecalc(SyntheticStyleChange);
 }

trunk/Source/WebCore/dom/Element.h

@@ -438 +438 @@
 
 #if ENABLE(DIALOG_ELEMENT)
-    virtual bool isInTopLayer() const;
-    virtual void setIsInTopLayer(bool);
+    bool isInTopLayer() const;
+    void setIsInTopLayer(bool);
 #endif
 

trunk/Source/WebCore/html/HTMLDialogElement.cpp

@@ -54 +54 @@
     }
     setBooleanAttribute(openAttr, false);
-    setIsInTopLayer(false);
+    document()->removeFromTopLayer(this);
 }
 
@@ -71 +71 @@
     }
     setBooleanAttribute(openAttr, true);
-    setIsInTopLayer(true);
+    document()->addToTopLayer(this);
 }