Attachment 346224 Details for Bug 186751 – Register SVGTests attributes into SVGAttributeRegistry

[patch] Register SVGTests attributes into SVGAttributeRegistry

186751-26.patch (text/plain), 12.92 KB, created by Said Abou-Hallawa on 2018-07-31 16:49:20 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Said Abou-Hallawa

Created: 2018-07-31 16:49:20 PDT

Size: 12.92 KB

patch

obsolete

>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index da54c907386..050710796dc 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,44 @@
>+2018-07-31  Said Abou-Hallawa  <sabouhallawa@apple.com>
>+
>+        [26] Remove the SVG elements' attributes macros
>+        https://bugs.webkit.org/show_bug.cgi?id=186751
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Register SVGTests attributes into SVGAttributeRegistry
>+
>+        -- Change the type of m_requiredFeatures, m_requiredExtensions and 
>+           m_systemLanguage to be non animateable SVGStringListValuesAttribute.
>+           This is how they are defined in SVGTests.idl.
>+        -- Add m_attributeOwnerProxy but make it a std::unique_ptr to avoid including
>+           SVGAttributeOwnerProxy.h in SVGTests.h. If we did all the header files
>+           SVGAnimatedxxx.h have to added to WebCore project and made private.
>+        -- 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.
>+
>+        * svg/SVGTests.cpp:
>+        (WebCore::SVGTests::SVGTests):
>+        (WebCore::SVGTests::registerAttributes):
>+        (WebCore::SVGTests::attributeRegistry):
>+        (WebCore::SVGTests::isKnownAttribute):
>+        (WebCore::SVGTests::isValid const):
>+        (WebCore::SVGTests::parseAttribute):
>+        (WebCore::SVGTests::svgAttributeChanged):
>+        (WebCore::SVGTests::requiredFeatures):
>+        (WebCore::SVGTests::requiredExtensions):
>+        (WebCore::SVGTests::systemLanguage):
>+        (WebCore::createSVGTestPropertyInfo): Deleted.
>+        (WebCore::createSVGTextAttributeToPropertyMap): Deleted.
>+        (WebCore::SVGTests::attributeToPropertyMap): Deleted.
>+        (WebCore::SVGTests::handleAttributeChange): Deleted.
>+        (WebCore::SVGTests::synchronizeAttribute): Deleted.
>+        (WebCore::SVGTests::synchronizeRequiredFeatures): Deleted.
>+        (WebCore::SVGTests::synchronizeRequiredExtensions): Deleted.
>+        (WebCore::SVGTests::synchronizeSystemLanguage): Deleted.
>+        * svg/SVGTests.h:
>+
> 2018-07-31  Said Abou-Hallawa  <sabouhallawa@apple.com>
> 
>         [25] Remove the SVG elements' attributes macros
>diff --git a/Source/WebCore/svg/SVGTests.cpp b/Source/WebCore/svg/SVGTests.cpp
>index 597cedd7c0a..70624827538 100644
>--- a/Source/WebCore/svg/SVGTests.cpp
>+++ b/Source/WebCore/svg/SVGTests.cpp
>@@ -1,7 +1,7 @@
> /*
>  * Copyright (C) 2004, 2005, 2008 Nikolas Zimmermann <zimmermann@kde.org>
>  * Copyright (C) 2004, 2005, 2006, 2007 Rob Buis <buis@kde.org>
>- * Copyright (C) 2015-2016 Apple Inc. All right reserved.
>+ * Copyright (C) 2015-2018 Apple Inc. All right reserved.
>  *
>  * This library is free software; you can redistribute it and/or
>  * modify it under the terms of the GNU Library General Public
>@@ -24,6 +24,7 @@
> 
> #include "DOMImplementation.h"
> #include "HTMLNames.h"
>+#include "SVGAttributeOwnerProxy.h"
> #include "SVGElement.h"
> #include "SVGNames.h"
> #include "SVGStringList.h"
>@@ -106,40 +107,34 @@ static const HashSet<String, ASCIICaseInsensitiveHash>& supportedSVGFeatures()
>     return features;
> }
> 
>-SVGTests::SVGTests()
>-    : m_requiredFeatures(requiredFeaturesAttr)
>-    , m_requiredExtensions(requiredExtensionsAttr)
>-    , m_systemLanguage(systemLanguageAttr)
>+SVGTests::SVGTests(SVGElement* contextElement)
>+    : m_contextElement(*contextElement)
>+    , m_attributeOwnerProxy(std::make_unique<AttributeOwnerProxy>(*this, *contextElement))
>+    , m_requiredFeatures(*m_attributeOwnerProxy, SVGNames::requiredFeaturesAttr)
>+    , m_requiredExtensions(*m_attributeOwnerProxy, SVGNames::requiredExtensionsAttr)
>+    , m_systemLanguage(*m_attributeOwnerProxy, SVGNames::systemLanguageAttr)
> {
>+    registerAttributes();
> }
> 
>-static SVGPropertyInfo createSVGTestPropertyInfo(const QualifiedName& attributeName, SVGPropertyInfo::SynchronizeProperty synchronizeFunction)
>+void SVGTests::registerAttributes()
> {
>-    return { AnimatedUnknown, PropertyIsReadWrite, attributeName, attributeName.localName(), synchronizeFunction, nullptr };
>+    auto& registry = attributeRegistry();
>+    if (!registry.isEmpty())
>+        return;
>+    registry.registerAttribute<SVGNames::requiredFeaturesAttr, &SVGTests::m_requiredFeatures>();
>+    registry.registerAttribute<SVGNames::requiredExtensionsAttr, &SVGTests::m_requiredExtensions>();
>+    registry.registerAttribute<SVGNames::systemLanguageAttr, &SVGTests::m_systemLanguage>();
> }
> 
>-static SVGAttributeToPropertyMap createSVGTextAttributeToPropertyMap()
>+SVGTests::AttributeRegistry& SVGTests::attributeRegistry()
> {
>-    typedef NeverDestroyed<const SVGPropertyInfo> Info;
>-
>-    SVGAttributeToPropertyMap map;
>-
>-    static Info requiredFeatures = createSVGTestPropertyInfo(requiredFeaturesAttr, SVGElement::synchronizeRequiredFeatures);
>-    map.addProperty(requiredFeatures.get());
>-
>-    static Info requiredExtensions = createSVGTestPropertyInfo(requiredExtensionsAttr, SVGElement::synchronizeRequiredExtensions);
>-    map.addProperty(requiredExtensions.get());
>-
>-    static Info systemLanguage = createSVGTestPropertyInfo(systemLanguageAttr, SVGElement::synchronizeSystemLanguage);
>-    map.addProperty(systemLanguage.get());
>-
>-    return map;
>+    return AttributeOwnerProxy::attributeRegistry();
> }
> 
>-const SVGAttributeToPropertyMap& SVGTests::attributeToPropertyMap()
>+bool SVGTests::isKnownAttribute(const QualifiedName& attributeName)
> {
>-    static NeverDestroyed<SVGAttributeToPropertyMap> map = createSVGTextAttributeToPropertyMap();
>-    return map;
>+    return AttributeOwnerProxy::isKnownAttribute(attributeName);
> }
> 
> bool SVGTests::hasExtension(const String& extension)
>@@ -154,15 +149,15 @@ bool SVGTests::hasExtension(const String& extension)
> 
> bool SVGTests::isValid() const
> {
>-    for (auto& feature : m_requiredFeatures.value) {
>+    for (auto& feature : m_requiredFeatures.value()) {
>         if (feature.isEmpty() || !supportedSVGFeatures().contains(feature))
>             return false;
>     }
>-    for (auto& language : m_systemLanguage.value) {
>+    for (auto& language : m_systemLanguage.value()) {
>         if (language != defaultLanguage().substring(0, 2))
>             return false;
>     }
>-    for (auto& extension : m_requiredExtensions.value) {
>+    for (auto& extension : m_requiredExtensions.value()) {
>         if (!hasExtension(extension))
>             return false;
>     }
>@@ -172,29 +167,21 @@ bool SVGTests::isValid() const
> void SVGTests::parseAttribute(const QualifiedName& attributeName, const AtomicString& value)
> {
>     if (attributeName == requiredFeaturesAttr)
>-        m_requiredFeatures.value.reset(value);
>+        m_requiredFeatures.value().reset(value);
>     if (attributeName == requiredExtensionsAttr)
>-        m_requiredExtensions.value.reset(value);
>+        m_requiredExtensions.value().reset(value);
>     if (attributeName == systemLanguageAttr)
>-        m_systemLanguage.value.reset(value);
>+        m_systemLanguage.value().reset(value);
> }
> 
>-bool SVGTests::isKnownAttribute(const QualifiedName& attributeName)
>+void SVGTests::svgAttributeChanged(const QualifiedName& attrName)
> {
>-    return attributeName == requiredFeaturesAttr
>-        || attributeName == requiredExtensionsAttr
>-        || attributeName == systemLanguageAttr;
>-}
>+    if (!isKnownAttribute(attrName))
>+        return;
> 
>-bool SVGTests::handleAttributeChange(SVGElement* targetElement, const QualifiedName& attributeName)
>-{
>-    ASSERT(targetElement);
>-    if (!isKnownAttribute(attributeName))
>-        return false;
>-    if (!targetElement->isConnected())
>-        return true;
>-    targetElement->invalidateStyleAndRenderersForSubtree();
>-    return true;
>+    if (!m_contextElement.isConnected())
>+        return;
>+    m_contextElement.invalidateStyleAndRenderersForSubtree();
> }
> 
> void SVGTests::addSupportedAttributes(HashSet<QualifiedName>& supportedAttributes)
>@@ -204,44 +191,22 @@ void SVGTests::addSupportedAttributes(HashSet<QualifiedName>& supportedAttribute
>     supportedAttributes.add(systemLanguageAttr);
> }
> 
>-void SVGTests::synchronizeAttribute(SVGElement& contextElement, SVGSynchronizableAnimatedProperty<SVGStringListValues>& property, const QualifiedName& attributeName)
>-{
>-    if (!property.shouldSynchronize)
>-        return;
>-    m_requiredFeatures.synchronize(&contextElement, attributeName, property.value.valueAsString());
>-}
>-
>-void SVGTests::synchronizeRequiredFeatures(SVGElement& contextElement)
>-{
>-    synchronizeAttribute(contextElement, m_requiredFeatures, requiredFeaturesAttr);
>-}
>-
>-void SVGTests::synchronizeRequiredExtensions(SVGElement& contextElement)
>-{
>-    synchronizeAttribute(contextElement, m_requiredExtensions, requiredExtensionsAttr);
>-}
>-
>-void SVGTests::synchronizeSystemLanguage(SVGElement& contextElement)
>-{
>-    synchronizeAttribute(contextElement, m_systemLanguage, systemLanguageAttr);
>-}
>-
> Ref<SVGStringList> SVGTests::requiredFeatures(SVGElement& contextElement)
> {
>-    m_requiredFeatures.shouldSynchronize = true;
>-    return SVGStringList::create(contextElement, m_requiredFeatures.value);
>+    m_requiredFeatures.setShouldSynchronize(true);
>+    return SVGStringList::create(m_contextElement, m_requiredFeatures.value());
> }
> 
> Ref<SVGStringList> SVGTests::requiredExtensions(SVGElement& contextElement)
> {
>-    m_requiredExtensions.shouldSynchronize = true;    
>-    return SVGStringList::create(contextElement, m_requiredExtensions.value);
>+    m_requiredExtensions.setShouldSynchronize(true);
>+    return SVGStringList::create(m_contextElement, m_requiredExtensions.value());
> }
> 
> Ref<SVGStringList> SVGTests::systemLanguage(SVGElement& contextElement)
> {
>-    m_systemLanguage.shouldSynchronize = true;
>-    return SVGStringList::create(contextElement, m_systemLanguage.value);
>+    m_systemLanguage.setShouldSynchronize(true);
>+    return SVGStringList::create(m_contextElement, m_systemLanguage.value());
> }
> 
> bool SVGTests::hasFeatureForLegacyBindings(const String& feature, const String& version)
>diff --git a/Source/WebCore/svg/SVGTests.h b/Source/WebCore/svg/SVGTests.h
>index d99ac22401a..e1ecdedb24c 100644
>--- a/Source/WebCore/svg/SVGTests.h
>+++ b/Source/WebCore/svg/SVGTests.h
>@@ -1,6 +1,7 @@
> /*
>  * Copyright (C) 2004, 2005, 2008 Nikolas Zimmermann <zimmermann@kde.org>
>  * Copyright (C) 2004, 2005, 2006, 2010 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
>@@ -20,7 +21,7 @@
> 
> #pragma once
> 
>-#include "SVGAnimatedPropertyMacros.h"
>+#include "SVGAttribute.h"
> #include "SVGStringListValues.h"
> 
> namespace WebCore {
>@@ -28,39 +29,47 @@ namespace WebCore {
> class SVGElement;
> class SVGStringList;
> 
>+template<typename OwnerType, typename... BaseTypes>
>+class SVGAttributeRegistry;
>+
>+template<typename OwnerType, typename... BaseTypes>
>+class SVGAttributeOwnerProxyImpl;
>+
> class SVGTests {
> public:
>     static bool hasExtension(const String&);
>     bool isValid() const;
> 
>+    using AttributeRegistry = SVGAttributeRegistry<SVGTests>;
>+    static AttributeRegistry& attributeRegistry();
>+    static bool isKnownAttribute(const QualifiedName& attributeName);
>+
>     void parseAttribute(const QualifiedName&, const AtomicString&);
>+    void svgAttributeChanged(const QualifiedName&);
> 
>-    static bool isKnownAttribute(const QualifiedName&);
>     static void addSupportedAttributes(HashSet<QualifiedName>&);
> 
>-    static bool handleAttributeChange(SVGElement*, const QualifiedName&);
>-
>-    static const SVGAttributeToPropertyMap& attributeToPropertyMap();
>-
>     WEBCORE_EXPORT static bool hasFeatureForLegacyBindings(const String& feature, const String& version);
> 
>+    // These methods are called from DOM through the super classes.
>+    Ref<SVGStringList> requiredFeatures();
>+    Ref<SVGStringList> requiredExtensions();
>+    Ref<SVGStringList> systemLanguage();
>+
> protected:
>     SVGTests();
> 
>-    Ref<SVGStringList> requiredFeatures(SVGElement&);
>-    Ref<SVGStringList> requiredExtensions(SVGElement&);
>-    Ref<SVGStringList> systemLanguage(SVGElement&);
>+private:
>+    SVGElement& m_contextElement;
> 
>-    void synchronizeRequiredFeatures(SVGElement&);
>-    void synchronizeRequiredExtensions(SVGElement&);
>-    void synchronizeSystemLanguage(SVGElement&);
>+    using AttributeOwnerProxy = SVGAttributeOwnerProxyImpl<SVGTests>;
>+    static void registerAttributes();
> 
>-private:
>-    void synchronizeAttribute(SVGElement& contextElement, SVGSynchronizableAnimatedProperty<SVGStringListValues>&, const QualifiedName& attributeName);
>+    std::unique_ptr<AttributeOwnerProxy> m_attributeOwnerProxy;
> 
>-    SVGSynchronizableAnimatedProperty<SVGStringListValues> m_requiredFeatures;
>-    SVGSynchronizableAnimatedProperty<SVGStringListValues> m_requiredExtensions;
>-    SVGSynchronizableAnimatedProperty<SVGStringListValues> m_systemLanguage;
>+    SVGStringListValuesAttribute m_requiredFeatures;
>+    SVGStringListValuesAttribute m_requiredExtensions;
>+    SVGStringListValuesAttribute m_systemLanguage;
> };
> 
> } // 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