Like what we did for SVGAnimatedInteger in <https://trac.webkit.org/changeset/243036>, we are going to do similar transform for SVGAnimatedBoolean. Instead of saving the raw boolean data in the SVGElement and use the tear off object to provide the animation and the DOM objects, the SVGElement will hold a Ref<SVGAnimatedBoolean>. This will eliminate the need to create any other objects.
Created attachment 364965 [details] Patch
Created attachment 364966 [details] Patch for review
Comment on attachment 364966 [details] Patch for review View in context: https://bugs.webkit.org/attachment.cgi?id=364966&action=review > Source/WebCore/svg/SVGAnimatorFactory.h:52 > - return std::make_unique<SVGAnimatedBooleanAnimator>(animationElement, contextElement); > + return nullptr; This is legacy code that will be removed? > Source/WebCore/svg/SVGExternalResourcesRequired.h:77 > + Ref<SVGAnimatedBoolean> m_externalResourcesRequired; https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/externalResourcesRequired says that "externalResourcesRequired" is not animatable (I could not find it in the spec). So why do we make a animated boolean for it?
Comment on attachment 364966 [details] Patch for review View in context: https://bugs.webkit.org/attachment.cgi?id=364966&action=review >> Source/WebCore/svg/SVGAnimatorFactory.h:52 >> + return nullptr; > > This is legacy code that will be removed? Yes. >> Source/WebCore/svg/SVGExternalResourcesRequired.h:77 >> + Ref<SVGAnimatedBoolean> m_externalResourcesRequired; > > https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/externalResourcesRequired says that "externalResourcesRequired" is not animatable (I could not find it in the spec). So why do we make a animated boolean for it? This comment at the top of Source/WebCore/svg/SVGExternalResourcesRequired.h might explain why it was defined as SVGAnimatedBoolean. But I did not understand what is the difference between the SVG DOM and the SVG language definition especially there is no links. // Notes on a SVG 1.1 spec discrepancy: // The SVG DOM defines the attribute externalResourcesRequired as being of type SVGAnimatedBoolean, whereas the // SVG language definition says that externalResourcesRequired is not animated. Because the SVG language definition // states that externalResourcesRequired cannot be animated, the animVal will always be the same as the baseVal. // FIXME: When implementing animVal support, make sure that animVal==baseVal for externalResourcesRequired I am not sure what we should do here. According to https://www.w3.org/TR/SVG/changes.html, SVGExternalResourcesRequired was removed from SVG2. Should we remove it? If we remove it, what is the backward compatibility story here? We still need the SVGAnimatedBoolean because it is used by SVGFEConvolveMatrixElement according to https://drafts.fxtf.org/filter-effects/#element-attrdef-feconvolvematrix-preservealpha.
Let's just keep existing behavior for now.
Created attachment 365092 [details] Patch
Comment on attachment 365092 [details] Patch Clearing flags on attachment: 365092 Committed r243121: <https://trac.webkit.org/changeset/243121>
All reviewed patches have been landed. Closing bug.
<rdar://problem/49001748>