Title: [221838] trunk/Source/WebCore
Revision
221838
Author
da...@apple.com
Date
2017-09-10 15:17:59 -0700 (Sun, 10 Sep 2017)

Log Message

Refactor Document::updateTitleElement to use traits instead of function pointers
https://bugs.webkit.org/show_bug.cgi?id=176671

Reviewed by Sam Weinig.

This template implementation seems slightly more readable and
also likely to be slightly more efficient. Also takes a suggestion
from Antti of factoring out the "select a new title element" into a
function, which is a natural thing to do in this version.

* dom/Document.cpp:
(WebCore::TitleTraits<HTMLTitleElement>::isInEligibleLocation): Added.
(WebCore::TitleTraits<HTMLTitleElement>::findTitleElement): Added.
(WebCore::TitleTraits<SVGTitleElement>::isInEligibleLocation): Added.
(WebCore::TitleTraits<SVGTitleElement>::findTitleElement): Added.
(WebCore::selectNewTitleElement): Added.
(WebCore::findHTMLTitle): Deleted.
(WebCore::isHTMLTitle): Deleted.
(WebCore::isHTMLTitleEligible): Deleted.
(WebCore::findSVGTitle): Deleted.
(WebCore::isSVGTitle): Deleted.
(WebCore::isSVGTitleEligible): Deleted.
(WebCore::Document::updateTitleElement): Call selectNewTitleElement
instead of having the logic here.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (221837 => 221838)


--- trunk/Source/WebCore/ChangeLog	2017-09-10 21:49:13 UTC (rev 221837)
+++ trunk/Source/WebCore/ChangeLog	2017-09-10 22:17:59 UTC (rev 221838)
@@ -1,3 +1,30 @@
+2017-09-10  Darin Adler  <da...@apple.com>
+
+        Refactor Document::updateTitleElement to use traits instead of function pointers
+        https://bugs.webkit.org/show_bug.cgi?id=176671
+
+        Reviewed by Sam Weinig.
+
+        This template implementation seems slightly more readable and
+        also likely to be slightly more efficient. Also takes a suggestion
+        from Antti of factoring out the "select a new title element" into a
+        function, which is a natural thing to do in this version.
+
+        * dom/Document.cpp:
+        (WebCore::TitleTraits<HTMLTitleElement>::isInEligibleLocation): Added.
+        (WebCore::TitleTraits<HTMLTitleElement>::findTitleElement): Added.
+        (WebCore::TitleTraits<SVGTitleElement>::isInEligibleLocation): Added.
+        (WebCore::TitleTraits<SVGTitleElement>::findTitleElement): Added.
+        (WebCore::selectNewTitleElement): Added.
+        (WebCore::findHTMLTitle): Deleted.
+        (WebCore::isHTMLTitle): Deleted.
+        (WebCore::isHTMLTitleEligible): Deleted.
+        (WebCore::findSVGTitle): Deleted.
+        (WebCore::isSVGTitle): Deleted.
+        (WebCore::isSVGTitleEligible): Deleted.
+        (WebCore::Document::updateTitleElement): Call selectNewTitleElement
+        instead of having the logic here.
+
 2017-09-07  Darin Adler  <da...@apple.com>
 
         Fix double resolve assertion in FontFaceSet seen while running tests

Modified: trunk/Source/WebCore/dom/Document.cpp (221837 => 221838)


--- trunk/Source/WebCore/dom/Document.cpp	2017-09-10 21:49:13 UTC (rev 221837)
+++ trunk/Source/WebCore/dom/Document.cpp	2017-09-10 22:17:59 UTC (rev 221838)
@@ -1542,63 +1542,47 @@
         updateTitle({ title, LTR });
 }
 
-static Element* findHTMLTitle(Document& document)
-{
-    return descendantsOfType<HTMLTitleElement>(document).first();
-};
+template<typename> struct TitleTraits;
 
-static bool isHTMLTitle(Element& element)
-{
-    return is<HTMLTitleElement>(element);
+template<> struct TitleTraits<HTMLTitleElement> {
+    static bool isInEligibleLocation(HTMLTitleElement& element) { return element.isConnected() && !element.isInShadowTree(); }
+    static HTMLTitleElement* findTitleElement(Document& document) { return descendantsOfType<HTMLTitleElement>(document).first(); }
 };
 
-static bool isHTMLTitleEligible(Element& element)
-{
-    return element.isConnected() && !element.isInShadowTree();
+template<> struct TitleTraits<SVGTitleElement> {
+    static bool isInEligibleLocation(SVGTitleElement& element) { return element.parentNode() == element.document().documentElement(); }
+    static SVGTitleElement* findTitleElement(Document& document) { return childrenOfType<SVGTitleElement>(*document.documentElement()).first(); }
 };
 
-static Element* findSVGTitle(Document& document)
+template<typename TitleElement> Element* selectNewTitleElement(Document& document, Element* oldTitleElement, Element& changingTitleElement)
 {
-    return childrenOfType<SVGTitleElement>(*document.documentElement()).first();
-};
+    using Traits = TitleTraits<TitleElement>;
 
-static bool isSVGTitle(Element& element)
-{
-    return is<SVGTitleElement>(element);
-};
+    if (!is<TitleElement>(changingTitleElement)) {
+        ASSERT(oldTitleElement == Traits::findTitleElement(document));
+        return oldTitleElement;
+    }
 
-static bool isSVGTitleEligible(Element& element)
-{
-    return element.parentNode() == element.document().documentElement();
-};
+    if (oldTitleElement)
+        return Traits::findTitleElement(document);
 
+    // Optimized common case: We have no title element yet.
+    // We can figure out which title element should be used without searching.
+    bool isEligible = Traits::isInEligibleLocation(downcast<TitleElement>(changingTitleElement));
+    auto* newTitleElement = isEligible ? &changingTitleElement : nullptr;
+    ASSERT(newTitleElement == Traits::findTitleElement(document));
+    return newTitleElement;
+}
+
 void Document::updateTitleElement(Element& changingTitleElement)
 {
     // Most documents use HTML title rules.
     // Documents with SVG document elements use SVG title rules.
-    bool useSVGTitle = is<SVGSVGElement>(documentElement());
-    auto findTitle = useSVGTitle ? findSVGTitle : findHTMLTitle;
-    auto isTitle = useSVGTitle ? isSVGTitle : isHTMLTitle;
-    auto isTitleEligible = useSVGTitle ? isSVGTitleEligible : isHTMLTitleEligible;
-
-    if (!isTitle(changingTitleElement)) {
-        ASSERT(m_titleElement == findTitle(*this));
-        return;
-    }
-
-    Element* newTitleElement;
-    if (m_titleElement)
-        newTitleElement = findTitle(*this);
-    else {
-        // Optimized common case: We have no title element yet.
-        // We can figure out which title element should be used without searching.
-        newTitleElement = isTitleEligible(changingTitleElement) ? &changingTitleElement : nullptr;
-        ASSERT(newTitleElement == findTitle(*this));
-    }
-
+    auto selectTitleElement = is<SVGSVGElement>(documentElement())
+        ? selectNewTitleElement<SVGTitleElement> : selectNewTitleElement<HTMLTitleElement>;
+    auto newTitleElement = selectTitleElement(*this, m_titleElement.get(), changingTitleElement);
     if (m_titleElement == newTitleElement)
         return;
-
     m_titleElement = newTitleElement;
     updateTitleFromTitleElement();
 }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to