Changeset 244642 in webkit

Timestamp:

Apr 25, 2019, 2:55:38 AM (6 years ago)

Author:

Antti Koivisto

Message:

Visited link hash should be computed only once
​https://bugs.webkit.org/show_bug.cgi?id=197229
<rdar://problem/48438924>

Reviewed by Alex Christensen.

Source/WebCore:

Test: fast/history/visited-href-mutation.html

Visited link style is now based on the first target URL of the link element. Further href mutations don't affect styling.

• dom/Document.cpp:

(WebCore::Document::updateBaseURL):

• dom/VisitedLinkState.cpp:

(WebCore::linkAttribute):
(WebCore::linkHashForElement):

Visited link support is now limited to HTML and SVG <a> elements.

(WebCore::VisitedLinkState::invalidateStyleForLink):
(WebCore::VisitedLinkState::determineLinkStateSlowCase):

• html/HTMLAnchorElement.cpp:

(WebCore::HTMLAnchorElement::HTMLAnchorElement):
(WebCore::HTMLAnchorElement::parseAttribute):

• html/HTMLAnchorElement.h:

(WebCore::HTMLAnchorElement::visitedLinkHash const):
(WebCore::HTMLAnchorElement::invalidateCachedVisitedLinkHash): Deleted.

• svg/SVGAElement.cpp:

(WebCore::SVGAElement::visitedLinkHash const):

• svg/SVGAElement.h:

LayoutTests:

• fast/history/visited-href-mutation-expected.html: Added.
• fast/history/visited-href-mutation.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/history/visited-href-mutation-expected.html (added)
• LayoutTests/fast/history/visited-href-mutation.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/Document.cpp (modified) (1 diff)
• Source/WebCore/dom/VisitedLinkState.cpp (modified) (5 diffs)
• Source/WebCore/html/HTMLAnchorElement.cpp (modified) (2 diffs)
• Source/WebCore/html/HTMLAnchorElement.h (modified) (3 diffs)
• Source/WebCore/svg/SVGAElement.cpp (modified) (1 diff)
• Source/WebCore/svg/SVGAElement.h (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2019-04-25  Antti Koivisto  <antti@apple.com>
+
+        Visited link hash should be computed only once
+        https://bugs.webkit.org/show_bug.cgi?id=197229
+        <rdar://problem/48438924>
+
+        Reviewed by Alex Christensen.
+
+        * fast/history/visited-href-mutation-expected.html: Added.
+        * fast/history/visited-href-mutation.html: Added.
+
 2019-04-25  Philippe Normand  <pnormand@igalia.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-04-25  Antti Koivisto  <antti@apple.com>
+
+        Visited link hash should be computed only once
+        https://bugs.webkit.org/show_bug.cgi?id=197229
+        <rdar://problem/48438924>
+
+        Reviewed by Alex Christensen.
+
+        Test: fast/history/visited-href-mutation.html
+
+        Visited link style is now based on the first target URL of the link element. Further href mutations don't affect styling.
+
+        * dom/Document.cpp:
+        (WebCore::Document::updateBaseURL):
+        * dom/VisitedLinkState.cpp:
+        (WebCore::linkAttribute):
+        (WebCore::linkHashForElement):
+
+        Visited link support is now limited to HTML and SVG <a> elements.
+
+        (WebCore::VisitedLinkState::invalidateStyleForLink):
+        (WebCore::VisitedLinkState::determineLinkStateSlowCase):
+        * html/HTMLAnchorElement.cpp:
+        (WebCore::HTMLAnchorElement::HTMLAnchorElement):
+        (WebCore::HTMLAnchorElement::parseAttribute):
+        * html/HTMLAnchorElement.h:
+        (WebCore::HTMLAnchorElement::visitedLinkHash const):
+        (WebCore::HTMLAnchorElement::invalidateCachedVisitedLinkHash): Deleted.
+        * svg/SVGAElement.cpp:
+        (WebCore::SVGAElement::visitedLinkHash const):
+        * svg/SVGAElement.h:
+
 2019-04-25  Philippe Normand  <pnormand@igalia.com>
 

trunk/Source/WebCore/dom/Document.cpp

@@ -3185 +3185 @@
     if (!m_baseURL.isValid())
         m_baseURL = URL();
-
-    if (!equalIgnoringFragmentIdentifier(oldBaseURL, m_baseURL)) {
-        // Base URL change changes any relative visited links.
-        // FIXME: There are other URLs in the tree that would need to be re-evaluated on dynamic base URL change. Style should be invalidated too.
-        for (auto& anchor : descendantsOfType<HTMLAnchorElement>(*this))
-            anchor.invalidateCachedVisitedLinkHash();
-    }
 }
 

trunk/Source/WebCore/dom/VisitedLinkState.cpp

@@ -34 +34 @@
 #include "HTMLAnchorElement.h"
 #include "Page.h"
+#include "SVGAElement.h"
 #include "SVGNames.h"
 #include "VisitedLinkStore.h"
@@ -45 +46 @@
 {
     if (!element.isLink())
-        return 0;
+        return nullptr;
     if (element.isHTMLElement())
         return &element.attributeWithoutSynchronization(HTMLNames::hrefAttr);
     if (element.isSVGElement())
         return &element.getAttribute(SVGNames::hrefAttr, XLinkNames::hrefAttr);
-    return 0;
+    return nullptr;
 }
 
@@ -68 +69 @@
 }
 
-inline static SharedStringHash linkHashForElement(Document& document, const Element& element)
+inline static Optional<SharedStringHash> linkHashForElement(const Element& element)
 {
     if (is<HTMLAnchorElement>(element))
         return downcast<HTMLAnchorElement>(element).visitedLinkHash();
-    if (const AtomicString* attribute = linkAttribute(element))
-        return computeVisitedLinkHash(document.baseURL(), *attribute);
-    return 0;
+    if (is<SVGAElement>(element))
+        return downcast<SVGAElement>(element).visitedLinkHash();
+    return WTF::nullopt;
 }
 
@@ -82 +83 @@
         return;
     for (auto& element : descendantsOfType<Element>(m_document)) {
-        if (linkHashForElement(m_document, element) == linkHash)
+        if (element.isLink() && linkHashForElement(element) == linkHash)
             element.invalidateStyleForSubtree();
     }
@@ -95 +96 @@
         return InsideLink::NotInside;
 
-    // An empty href refers to the document itself which is always visited. It is useful to check this explicitly so
+    auto hashIfFound = linkHashForElement(element);
+
+    if (!hashIfFound)
+        return attribute->isEmpty() ? InsideLink::InsideVisited : InsideLink::InsideUnvisited;
+
+    auto hash = *hashIfFound;
+
+    // An empty href (hash==0) refers to the document itself which is always visited. It is useful to check this explicitly so
     // that visited links can be tested in platform independent manner, without explicit support in the test harness.
-    if (attribute->isEmpty())
+    if (!hash)
         return InsideLink::InsideVisited;
-
-    SharedStringHash hash;
-    if (is<HTMLAnchorElement>(element))
-        hash = downcast<HTMLAnchorElement>(element).visitedLinkHash();
-    else
-        hash = computeVisitedLinkHash(element.document().baseURL(), *attribute);
-
-    if (!hash)
-        return InsideLink::InsideUnvisited;
 
     Frame* frame = element.document().frame();

trunk/Source/WebCore/html/HTMLAnchorElement.cpp

@@ -69 +69 @@
     , m_hasRootEditableElementForSelectionOnMouseDown(false)
     , m_wasShiftKeyDownOnMouseDown(false)
-    , m_cachedVisitedLinkHash(0)
 {
 }
@@ -249 +248 @@
             }
         }
-        invalidateCachedVisitedLinkHash();
     } else if (name == nameAttr || name == titleAttr) {
         // Do nothing.

trunk/Source/WebCore/html/HTMLAnchorElement.h

@@ -67 +67 @@
     
     SharedStringHash visitedLinkHash() const;
-    void invalidateCachedVisitedLinkHash() { m_cachedVisitedLinkHash = 0; }
 
     WEBCORE_EXPORT DOMTokenList& relList() const;
@@ -116 +115 @@
     bool m_wasShiftKeyDownOnMouseDown;
     OptionSet<Relation> m_linkRelations;
-    mutable SharedStringHash m_cachedVisitedLinkHash;
+
+    // This is computed only once and must not be affected by subsequent URL changes.
+    mutable Optional<SharedStringHash> m_storedVisitedLinkHash;
 
     mutable std::unique_ptr<DOMTokenList> m_relList;
@@ -123 +124 @@
 inline SharedStringHash HTMLAnchorElement::visitedLinkHash() const
 {
-    if (!m_cachedVisitedLinkHash)
-        m_cachedVisitedLinkHash = computeVisitedLinkHash(document().baseURL(), attributeWithoutSynchronization(HTMLNames::hrefAttr));
-    return m_cachedVisitedLinkHash; 
+    ASSERT(isLink());
+    if (!m_storedVisitedLinkHash)
+        m_storedVisitedLinkHash = computeVisitedLinkHash(document().baseURL(), attributeWithoutSynchronization(HTMLNames::hrefAttr));
+    return *m_storedVisitedLinkHash;
 }
 

trunk/Source/WebCore/svg/SVGAElement.cpp

@@ -218 +218 @@
 }
 
+SharedStringHash SVGAElement::visitedLinkHash() const
+{
+    ASSERT(isLink());
+    if (!m_storedVisitedLinkHash)
+        m_storedVisitedLinkHash = computeVisitedLinkHash(document().baseURL(), getAttribute(SVGNames::hrefAttr, XLinkNames::hrefAttr));
+    return *m_storedVisitedLinkHash;
+}
+
 } // namespace WebCore

trunk/Source/WebCore/svg/SVGAElement.h

@@ -26 +26 @@
 #include "SVGGraphicsElement.h"
 #include "SVGURIReference.h"
+#include "SharedStringHash.h"
 
 namespace WebCore {
@@ -36 +37 @@
     String target() const final { return m_target->currentValue(); }
     Ref<SVGAnimatedString>& targetAnimated() { return m_target; }
+
+    SharedStringHash visitedLinkHash() const;
 
 private:
@@ -64 +67 @@
     PropertyRegistry m_propertyRegistry { *this };
     Ref<SVGAnimatedString> m_target { SVGAnimatedString::create(this) };
+
+    // This is computed only once and must not be affected by subsequent URL changes.
+    mutable Optional<SharedStringHash> m_storedVisitedLinkHash;
 };