Attachment 346241 Details for Bug 186751 – Register SVGExternalResourcesRequired attributes into SVGAttributeRegistry

[patch] Register SVGExternalResourcesRequired attributes into SVGAttributeRegistry

186751-29.patch (text/plain), 11.02 KB, created by Said Abou-Hallawa on 2018-07-31 18:07:59 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Said Abou-Hallawa

Created: 2018-07-31 18:07:59 PDT

Size: 11.02 KB

patch

obsolete

>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index b6d17a0652e..23b271723cf 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,41 @@
>+2018-07-31  Said Abou-Hallawa  <sabouhallawa@apple.com>
>+
>+        [29] Remove the SVG elements' attributes macros
>+        https://bugs.webkit.org/show_bug.cgi?id=186751
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Register SVGExternalResourcesRequired attributes into SVGAttributeRegistry
>+
>+        -- Add a constructor and pass a reference to an SVGElement to it. This
>+           is a reference to the owner element. We will keep a reference to this
>+           SVGElement in the owner proxy.
>+        -- Add registerAttributes() which registers the attributes and call it from
>+           the constructor.
>+        -- Add svgAttributeChanged() which will be called from the super classes
>+           when an attribute changes.
>+        -- Remove the SVGElement parameters from dispatchLoadEvent() and 
>+           insertedIntoDocument().
>+
>+        * svg/SVGExternalResourcesRequired.cpp:
>+        (WebCore::SVGExternalResourcesRequired::SVGExternalResourcesRequired):
>+        (WebCore::SVGExternalResourcesRequired::registerAttributes):
>+        (WebCore::SVGExternalResourcesRequired::parseAttribute):
>+        (WebCore::SVGExternalResourcesRequired::svgAttributeChanged):
>+        (WebCore::SVGExternalResourcesRequired::addSupportedAttributes):
>+        (WebCore::SVGExternalResourcesRequired::dispatchLoadEvent):
>+        (WebCore::SVGExternalResourcesRequired::insertedIntoDocument):
>+        (WebCore::SVGExternalResourcesRequired::finishParsingChildren):
>+        (WebCore::SVGExternalResourcesRequired::haveLoadedRequiredResources const):
>+        (WebCore::SVGExternalResourcesRequired::isKnownAttribute): Deleted.
>+        (WebCore::SVGExternalResourcesRequired::handleAttributeChange): Deleted.
>+        * svg/SVGExternalResourcesRequired.h:
>+        (WebCore::SVGExternalResourcesRequired::attributeRegistry):
>+        (WebCore::SVGExternalResourcesRequired::externalResourcesRequiredAnimated):
>+        (WebCore::SVGExternalResourcesRequired::externalResourcesRequired const):
>+        (WebCore::SVGExternalResourcesRequired::setExternalResourcesRequired):
>+        (WebCore::SVGExternalResourcesRequired::isKnownAttribute):
>+
> 2018-07-31  Said Abou-Hallawa  <sabouhallawa@apple.com>
> 
>         [28] Remove the SVG elements' attributes macros
>diff --git a/Source/WebCore/svg/SVGExternalResourcesRequired.cpp b/Source/WebCore/svg/SVGExternalResourcesRequired.cpp
>index b0d38ecaa92..ff75654ca61 100644
>--- a/Source/WebCore/svg/SVGExternalResourcesRequired.cpp
>+++ b/Source/WebCore/svg/SVGExternalResourcesRequired.cpp
>@@ -1,6 +1,7 @@
> /*
>  * Copyright (C) 2004, 2005, 2008 Nikolas Zimmermann <zimmermann@kde.org>
>  * Copyright (C) 2004, 2005, 2007 Rob Buis <buis@kde.org>
>+ * Copyright (C) 2018 Apple Inc. All rights reserved.
>  *
>  * This library is free software; you can redistribute it and/or
>  * modify it under the terms of the GNU Library General Public
>@@ -26,49 +27,62 @@
> 
> namespace WebCore {
> 
>-void SVGExternalResourcesRequired::parseAttribute(const QualifiedName& name, const AtomicString& value)
>+SVGExternalResourcesRequired::SVGExternalResourcesRequired(SVGElement* contextElement)
>+    : m_contextElement(*contextElement)
>+    , m_attributeOwnerProxy(*this, *contextElement)
> {
>-    if (name == SVGNames::externalResourcesRequiredAttr)
>-        setExternalResourcesRequiredBaseValue(value == "true");
>+    registerAttributes();
> }
> 
>-bool SVGExternalResourcesRequired::isKnownAttribute(const QualifiedName& attrName)
>+void SVGExternalResourcesRequired::registerAttributes()
> {
>-    return attrName == SVGNames::externalResourcesRequiredAttr;
>+    auto& registry = attributeRegistry();
>+    if (!registry.isEmpty())
>+        return;
>+    registry.registerAttribute<SVGNames::externalResourcesRequiredAttr, &SVGEllipseElement::m_externalResourcesRequired>();
> }
> 
>-void SVGExternalResourcesRequired::addSupportedAttributes(HashSet<QualifiedName>& supportedAttributes)
>+void SVGExternalResourcesRequired::parseAttribute(const QualifiedName& name, const AtomicString& value)
> {
>-    supportedAttributes.add(SVGNames::externalResourcesRequiredAttr);
>+    if (name == SVGNames::externalResourcesRequiredAttr)
>+        setExternalResourcesRequired(value == "true");
> }
> 
>-bool SVGExternalResourcesRequired::handleAttributeChange(SVGElement* targetElement, const QualifiedName& attrName)
>+void SVGExternalResourcesRequired::svgAttributeChanged(const QualifiedName& attrName)
> {
>-    ASSERT(targetElement);
>     if (!isKnownAttribute(attrName))
>-        return false;
>-    if (!targetElement->isConnected())
>-        return true;
>+        return;
>+    if (!m_contextElement.isConnected())
>+        return;
> 
>     // Handle dynamic updates of the 'externalResourcesRequired' attribute. Only possible case: changing from 'true' to 'false'
>     // causes an immediate dispatch of the SVGLoad event. If the attribute value was 'false' before inserting the script element
>     // in the document, the SVGLoad event has already been dispatched.
>-    if (!externalResourcesRequiredBaseValue() && !haveFiredLoadEvent() && !isParserInserted()) {
>+    if (!externalResourcesRequired() && !haveFiredLoadEvent() && !isParserInserted()) {
>         setHaveFiredLoadEvent(true);
>-        ASSERT(targetElement->haveLoadedRequiredResources());
> 
>-        targetElement->sendSVGLoadEventIfPossible();
>+        ASSERT(m_contextElement.haveLoadedRequiredResources());
>+        m_contextElement.sendSVGLoadEventIfPossible()
>+    }
>+
>+    auto* renderer = m_contextElement.renderer();
>+    if (renderer && is<RenderSVGShape>(renderer)) {
>+        SVGElement::InstanceInvalidationGuard guard(m_contextElement);
>+        RenderSVGResource::markForLayoutAndParentResourceInvalidation(*renderer);
>     }
>+}
> 
>-    return true;
>+void SVGExternalResourcesRequired::addSupportedAttributes(HashSet<QualifiedName>& supportedAttributes)
>+{
>+    supportedAttributes.add(SVGNames::externalResourcesRequiredAttr);
> }
> 
>-void SVGExternalResourcesRequired::dispatchLoadEvent(SVGElement* targetElement)
>+void SVGExternalResourcesRequired::dispatchLoadEvent()
> {
>     bool externalResourcesRequired = externalResourcesRequiredBaseValue();
> 
>     if (isParserInserted())
>-        ASSERT(externalResourcesRequired != haveFiredLoadEvent());
>+        ASSERT(externalResourcesRequired() != haveFiredLoadEvent());
>     else if (haveFiredLoadEvent())
>         return;
> 
>@@ -76,40 +90,40 @@ void SVGExternalResourcesRequired::dispatchLoadEvent(SVGElement* targetElement)
>     // HTML fires the 'load' event after it sucessfully loaded a remote resource, otherwise an error event.
>     // SVG fires the SVGLoad event immediately after parsing the <script> element, if externalResourcesRequired
>     // is set to 'false', otherwise it dispatches the 'SVGLoad' event just after loading the remote resource.
>-    if (!externalResourcesRequired)
>+    if (!externalResourcesRequired())
>         return;
> 
>     ASSERT(!haveFiredLoadEvent());
> 
>     // Dispatch SVGLoad event
>     setHaveFiredLoadEvent(true);
>-    ASSERT(targetElement->haveLoadedRequiredResources());
>+    ASSERT(m_contextElement.haveLoadedRequiredResources());
> 
>-    targetElement->sendSVGLoadEventIfPossible();
>+    m_contextElement.sendSVGLoadEventIfPossible();
> }
> 
>-void SVGExternalResourcesRequired::insertedIntoDocument(SVGElement* targetElement)
>+void SVGExternalResourcesRequired::insertedIntoDocument()
> {
>     if (isParserInserted())
>         return;
> 
>     // Eventually send SVGLoad event now for the dynamically inserted script element.
>-    if (externalResourcesRequiredBaseValue())
>+    if (externalResourcesRequired())
>         return;
>     setHaveFiredLoadEvent(true);
>-    targetElement->sendSVGLoadEventIfPossibleAsynchronously();
>+    m_contextElement.sendSVGLoadEventIfPossibleAsynchronously();
> }
> 
> void SVGExternalResourcesRequired::finishParsingChildren()
> {
>     // A SVGLoad event has been fired by SVGElement::finishParsingChildren.
>-    if (!externalResourcesRequiredBaseValue())
>+    if (!externalResourcesRequired())
>         setHaveFiredLoadEvent(true);
> }
> 
> bool SVGExternalResourcesRequired::haveLoadedRequiredResources() const
> {
>-    return !externalResourcesRequiredBaseValue() || haveFiredLoadEvent();
>+    return !externalResourcesRequired() || haveFiredLoadEvent();
> }
> 
> }
>diff --git a/Source/WebCore/svg/SVGExternalResourcesRequired.h b/Source/WebCore/svg/SVGExternalResourcesRequired.h
>index 675c5450cfe..9fc22773637 100644
>--- a/Source/WebCore/svg/SVGExternalResourcesRequired.h
>+++ b/Source/WebCore/svg/SVGExternalResourcesRequired.h
>@@ -1,6 +1,7 @@
> /*
>  * Copyright (C) 2004, 2005, 2008 Nikolas Zimmermann <zimmermann@kde.org>
>  * Copyright (C) 2004, 2005 Rob Buis <buis@kde.org>
>+ * Copyright (C) 2018 Apple Inc. All rights reserved.
>  *
>  * This library is free software; you can redistribute it and/or
>  * modify it under the terms of the GNU Library General Public
>@@ -21,6 +22,7 @@
> #pragma once
> 
> #include "QualifiedName.h"
>+#include "SVGAttributeOwnerProxy.h"
> #include <wtf/HashSet.h>
> 
> namespace WebCore {
>@@ -37,25 +39,38 @@ public:
>     virtual ~SVGExternalResourcesRequired() = default;
> 
>     void parseAttribute(const QualifiedName&, const AtomicString&);
>+    void svgAttributeChanged(const QualifiedName&);
> 
>-    static bool isKnownAttribute(const QualifiedName&);
>     static void addSupportedAttributes(HashSet<QualifiedName>&);
> 
>-    bool handleAttributeChange(SVGElement*, const QualifiedName&);
>+    using AttributeOwnerProxy = SVGAttributeOwnerProxyImpl<SVGExternalResourcesRequired>;
>+    static auto& attributeRegistry() { return AttributeOwnerProxy::attributeRegistry(); }
>+
>+    auto externalResourcesRequiredAnimated() { return m_externalResourcesRequired.animatedProperty(); }
>+
>+    bool externalResourcesRequired() const { return m_externalResourcesRequired.value(); }
>+    void setExternalResourcesRequired(bool externalResourcesRequired) { m_externalResourcesRequired.setValue(externalResourcesRequired); }
> 
> protected:
>-    // These types look a bit awkward, but have to match the generic types of the SVGAnimatedProperty macros.
>-    virtual void setExternalResourcesRequiredBaseValue(const bool&, const bool validValue = true) = 0;
>-    virtual bool& externalResourcesRequiredBaseValue() const = 0;
>+    SVGExternalResourcesRequired(SVGElement* contextElement);
>+
>+    static bool isKnownAttribute(const QualifiedName& attributeName) { return AttributeOwnerProxy::isKnownAttribute(attributeName); }
> 
>     virtual void setHaveFiredLoadEvent(bool) { }
>     virtual bool isParserInserted() const { return false; }
>     virtual bool haveFiredLoadEvent() const { return false; }
> 
>-    void dispatchLoadEvent(SVGElement*);
>-    void insertedIntoDocument(SVGElement*);
>+    void dispatchLoadEvent();
>+    void insertedIntoDocument();
>     void finishParsingChildren();
>     bool haveLoadedRequiredResources() const;
>+
>+private:
>+    static void registerAttributes();
>+
>+    SVGElement& m_contextElement;
>+    AttributeOwnerProxy m_attributeOwnerProxy;
>+    SVGAnimatedBooleanAttribute m_externalResourcesRequired { m_attributeOwnerProxy };
> };
> 
> } // namespace WebCore

Actions: View | Formatted Diff | Diff

Attachments on bug 186751: 342916 | 342917 | 342985 | 343001 | 343114 | 343123 | 343124 | 343127 | 343129 | 343162 | 343167 | 343169 | 343171 | 343174 | 343213 | 343217 | 343327 | 343335 | 343338 | 343339 | 343343 | 343415 | 343979 | 343989 | 344002 | 344513 | 344516 | 344523 | 344534 | 344536 | 344988 | 344990 | 346097 | 346103 | 346124 | 346129 | 346177 | 346179 | 346180 | 346182 | 346185 | 346188 | 346190 | 346192 | 346193 | 346197 | 346199 | 346201 | 346202 | 346203 | 346204 | 346205 | 346206 | 346207 | 346211 | 346212 | 346217 | 346224 | 346230 | 346233 | 346241 | 346244 | 346245 | 346315 | 346316 | 346317 | 346318 | 346319 | 346320 | 346322 | 346323 | 346324 | 346325 | 346326 | 346327 | 346328 | 346329 | 346330 | 346331 | 346332 | 346333 | 346334 | 346335 | 346336 | 346337 | 346338 | 346339 | 346340 | 346341 | 346342 | 346343 | 346344 | 346345 | 346346 | 346347 | 346348 | 346349 | 346350 | 346518 | 346519 | 346520 | 346521 | 346522 | 346524 | 346525 | 346526 | 346527 | 346528 | 346529 | 346530 | 346531 | 346532 | 346533 | 346534 | 346535 | 346536 | 346537 | 346538 | 346539 | 346540 | 346541 | 346545 | 346546 | 346549 | 346551 | 346552 | 346601 | 346602 | 346611 | 346636 | 346642