Commit ce851e47 authored by Fredrik Söderquist's avatar Fredrik Söderquist Committed by Commit Bot

[CI] Stop tracking <*Gradient> 'href' references through SVGResources

Gradient resources are currently only using the 'linked' resource in the
SVGResources object for cycle breaking and change notifications. The
SVG*GradientElements also perform cycle breaking on their own,
disregarding what SVGResourcesCycleSolver has done. The 'href's
themselves are what risk introducing cycles for gradients at the moment,
so handling them explicitly (as is already done) doesn't add any
additional complexity. Mixing resources defined by the DOM with those
defined by style does add complexity though.

This CL stops tracking <*Gradient> href's using SVGResources, and starts
tracking them using an IdTargetObserver in SVGGradientElement, much like
how similar 'href's are tracked.

Bug: 661598, 769774
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I5538ede4170c7181098fe2308dbfa640506334f4
Reviewed-on: https://chromium-review.googlesource.com/880965
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Reviewed-by: default avatarPhilip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543685}
parent 1ab087d8
...@@ -190,6 +190,16 @@ void LayoutSVGResourceContainer::InvalidateCacheAndMarkForLayout( ...@@ -190,6 +190,16 @@ void LayoutSVGResourceContainer::InvalidateCacheAndMarkForLayout(
LayoutInvalidationReason::kSvgResourceInvalidated, layout_scope); LayoutInvalidationReason::kSvgResourceInvalidated, layout_scope);
} }
static void MarkForLayoutAndParentResourceInvalidationCallback(
bool needs_layout,
SVGElement& element) {
LayoutObject* layout_object = element.GetLayoutObject();
if (!layout_object)
return;
LayoutSVGResourceContainer::MarkForLayoutAndParentResourceInvalidation(
*layout_object, needs_layout);
}
static inline void RemoveFromCacheAndInvalidateDependencies( static inline void RemoveFromCacheAndInvalidateDependencies(
LayoutObject& object, LayoutObject& object,
bool needs_layout) { bool needs_layout) {
...@@ -205,7 +215,9 @@ static inline void RemoveFromCacheAndInvalidateDependencies( ...@@ -205,7 +215,9 @@ static inline void RemoveFromCacheAndInvalidateDependencies(
if (!object.GetNode() || !object.GetNode()->IsSVGElement()) if (!object.GetNode() || !object.GetNode()->IsSVGElement())
return; return;
ToSVGElement(object.GetNode())->NotifyIncomingReferences(needs_layout); auto callback = WTF::BindRepeating(
&MarkForLayoutAndParentResourceInvalidationCallback, needs_layout);
ToSVGElement(object.GetNode())->NotifyIncomingReferences(callback);
} }
void LayoutSVGResourceContainer::MarkForLayoutAndParentResourceInvalidation( void LayoutSVGResourceContainer::MarkForLayoutAndParentResourceInvalidation(
......
...@@ -34,6 +34,7 @@ void LayoutSVGResourceGradient::RemoveAllClientsFromCache( ...@@ -34,6 +34,7 @@ void LayoutSVGResourceGradient::RemoveAllClientsFromCache(
bool mark_for_invalidation) { bool mark_for_invalidation) {
gradient_map_.clear(); gradient_map_.clear();
should_collect_gradient_attributes_ = true; should_collect_gradient_attributes_ = true;
ToSVGGradientElement(*GetElement()).InvalidateDependentGradients();
MarkAllClientsForInvalidation( MarkAllClientsForInvalidation(
mark_for_invalidation ? kPaintInvalidation : kParentOnlyInvalidation); mark_for_invalidation ? kPaintInvalidation : kParentOnlyInvalidation);
} }
......
...@@ -28,7 +28,6 @@ ...@@ -28,7 +28,6 @@
#include "core/layout/svg/LayoutSVGResourceMasker.h" #include "core/layout/svg/LayoutSVGResourceMasker.h"
#include "core/layout/svg/LayoutSVGResourcePaintServer.h" #include "core/layout/svg/LayoutSVGResourcePaintServer.h"
#include "core/style/ComputedStyle.h" #include "core/style/ComputedStyle.h"
#include "core/svg/SVGGradientElement.h"
#include "core/svg/SVGPatternElement.h" #include "core/svg/SVGPatternElement.h"
#include "core/svg/SVGResource.h" #include "core/svg/SVGResource.h"
#include "core/svg/SVGTreeScopeResources.h" #include "core/svg/SVGTreeScopeResources.h"
...@@ -101,28 +100,6 @@ static HashSet<AtomicString>& FillAndStrokeTags() { ...@@ -101,28 +100,6 @@ static HashSet<AtomicString>& FillAndStrokeTags() {
return tag_list; return tag_list;
} }
static HashSet<AtomicString>& ChainableResourceTags() {
DEFINE_STATIC_LOCAL(HashSet<AtomicString>, tag_list,
({
linearGradientTag.LocalName(), patternTag.LocalName(),
radialGradientTag.LocalName(),
}));
return tag_list;
}
static inline AtomicString TargetReferenceFromResource(SVGElement& element) {
String target;
if (auto* pattern = ToSVGPatternElementOrNull(element))
target = pattern->href()->CurrentValue()->Value();
else if (auto* gradient = ToSVGGradientElementOrNull(element))
target = gradient->href()->CurrentValue()->Value();
else
NOTREACHED();
return SVGURIReference::FragmentIdentifierFromIRIString(
target, element.GetTreeScope());
}
namespace { namespace {
template <typename ContainerType> template <typename ContainerType>
...@@ -257,8 +234,9 @@ std::unique_ptr<SVGResources> SVGResources::BuildResources( ...@@ -257,8 +234,9 @@ std::unique_ptr<SVGResources> SVGResources::BuildResources(
} }
} }
if (ChainableResourceTags().Contains(tag_name)) { if (auto* pattern = ToSVGPatternElementOrNull(element)) {
AtomicString id = TargetReferenceFromResource(element); AtomicString id = SVGURIReference::FragmentIdentifierFromIRIString(
pattern->HrefString(), tree_scope);
EnsureResources(resources).SetLinkedResource( EnsureResources(resources).SetLinkedResource(
AttachToResource<LayoutSVGResourceContainer>(tree_scope_resources, id, AttachToResource<LayoutSVGResourceContainer>(tree_scope_resources, id,
element)); element));
......
...@@ -1257,7 +1257,8 @@ void SVGElement::AddReferenceTo(SVGElement* target_element) { ...@@ -1257,7 +1257,8 @@ void SVGElement::AddReferenceTo(SVGElement* target_element) {
target_element->EnsureSVGRareData()->IncomingReferences().insert(this); target_element->EnsureSVGRareData()->IncomingReferences().insert(this);
} }
void SVGElement::NotifyIncomingReferences(bool needs_layout) { void SVGElement::NotifyIncomingReferences(
const InvalidationCallback& invalidation_callback) {
if (!HasSVGRareData()) if (!HasSVGRareData())
return; return;
...@@ -1273,16 +1274,12 @@ void SVGElement::NotifyIncomingReferences(bool needs_layout) { ...@@ -1273,16 +1274,12 @@ void SVGElement::NotifyIncomingReferences(bool needs_layout) {
(new SVGElementSet)); (new SVGElementSet));
for (SVGElement* element : dependencies) { for (SVGElement* element : dependencies) {
if (LayoutObject* layout_object = element->GetLayoutObject()) { if (UNLIKELY(!invalidating_dependencies.insert(element).is_new_entry)) {
if (UNLIKELY(!invalidating_dependencies.insert(element).is_new_entry)) { // Reference cycle: we are in process of invalidating this dependant.
// Reference cycle: we are in process of invalidating this dependant. continue;
continue;
}
LayoutSVGResourceContainer::MarkForLayoutAndParentResourceInvalidation(
*layout_object, needs_layout);
invalidating_dependencies.erase(element);
} }
invalidation_callback.Run(*element);
invalidating_dependencies.erase(element);
} }
} }
......
...@@ -171,7 +171,8 @@ class CORE_EXPORT SVGElement : public Element { ...@@ -171,7 +171,8 @@ class CORE_EXPORT SVGElement : public Element {
bool InUseShadowTree() const; bool InUseShadowTree() const;
void AddReferenceTo(SVGElement*); void AddReferenceTo(SVGElement*);
void NotifyIncomingReferences(bool needs_layout); using InvalidationCallback = base::RepeatingCallback<void(SVGElement&)>;
void NotifyIncomingReferences(const InvalidationCallback&);
void RebuildAllIncomingReferences(); void RebuildAllIncomingReferences();
void RemoveAllIncomingReferences(); void RemoveAllIncomingReferences();
void RemoveAllOutgoingReferences(); void RemoveAllOutgoingReferences();
......
...@@ -24,6 +24,7 @@ ...@@ -24,6 +24,7 @@
#include "core/css/StyleChangeReason.h" #include "core/css/StyleChangeReason.h"
#include "core/dom/Attribute.h" #include "core/dom/Attribute.h"
#include "core/dom/ElementTraversal.h" #include "core/dom/ElementTraversal.h"
#include "core/dom/IdTargetObserver.h"
#include "core/layout/svg/LayoutSVGResourceContainer.h" #include "core/layout/svg/LayoutSVGResourceContainer.h"
#include "core/svg/GradientAttributes.h" #include "core/svg/GradientAttributes.h"
#include "core/svg/SVGStopElement.h" #include "core/svg/SVGStopElement.h"
...@@ -68,10 +69,27 @@ void SVGGradientElement::Trace(blink::Visitor* visitor) { ...@@ -68,10 +69,27 @@ void SVGGradientElement::Trace(blink::Visitor* visitor) {
visitor->Trace(gradient_transform_); visitor->Trace(gradient_transform_);
visitor->Trace(spread_method_); visitor->Trace(spread_method_);
visitor->Trace(gradient_units_); visitor->Trace(gradient_units_);
visitor->Trace(target_id_observer_);
SVGElement::Trace(visitor); SVGElement::Trace(visitor);
SVGURIReference::Trace(visitor); SVGURIReference::Trace(visitor);
} }
void SVGGradientElement::BuildPendingResource() {
ClearResourceReferences();
if (!isConnected())
return;
Element* target = ObserveTarget(target_id_observer_, *this);
if (auto* gradient = ToSVGGradientElementOrNull(target))
AddReferenceTo(gradient);
InvalidateGradient(LayoutInvalidationReason::kSvgResourceInvalidated);
}
void SVGGradientElement::ClearResourceReferences() {
UnobserveTarget(target_id_observer_);
RemoveAllOutgoingReferences();
}
void SVGGradientElement::CollectStyleForPresentationAttribute( void SVGGradientElement::CollectStyleForPresentationAttribute(
const QualifiedName& name, const QualifiedName& name,
const AtomicString& value, const AtomicString& value,
...@@ -94,16 +112,35 @@ void SVGGradientElement::SvgAttributeChanged(const QualifiedName& attr_name) { ...@@ -94,16 +112,35 @@ void SVGGradientElement::SvgAttributeChanged(const QualifiedName& attr_name) {
if (attr_name == SVGNames::gradientUnitsAttr || if (attr_name == SVGNames::gradientUnitsAttr ||
attr_name == SVGNames::gradientTransformAttr || attr_name == SVGNames::gradientTransformAttr ||
attr_name == SVGNames::spreadMethodAttr || attr_name == SVGNames::spreadMethodAttr) {
SVGURIReference::IsKnownAttribute(attr_name)) {
SVGElement::InvalidationGuard invalidation_guard(this); SVGElement::InvalidationGuard invalidation_guard(this);
InvalidateGradient(LayoutInvalidationReason::kAttributeChanged); InvalidateGradient(LayoutInvalidationReason::kAttributeChanged);
return; return;
} }
if (SVGURIReference::IsKnownAttribute(attr_name)) {
SVGElement::InvalidationGuard invalidation_guard(this);
BuildPendingResource();
return;
}
SVGElement::SvgAttributeChanged(attr_name); SVGElement::SvgAttributeChanged(attr_name);
} }
Node::InsertionNotificationRequest SVGGradientElement::InsertedInto(
ContainerNode* root_parent) {
SVGElement::InsertedInto(root_parent);
if (root_parent->isConnected())
BuildPendingResource();
return kInsertionDone;
}
void SVGGradientElement::RemovedFrom(ContainerNode* root_parent) {
SVGElement::RemovedFrom(root_parent);
if (root_parent->isConnected())
ClearResourceReferences();
}
void SVGGradientElement::ChildrenChanged(const ChildrenChange& change) { void SVGGradientElement::ChildrenChanged(const ChildrenChange& change) {
SVGElement::ChildrenChanged(change); SVGElement::ChildrenChanged(change);
...@@ -119,6 +156,15 @@ void SVGGradientElement::InvalidateGradient( ...@@ -119,6 +156,15 @@ void SVGGradientElement::InvalidateGradient(
layout_object->InvalidateCacheAndMarkForLayout(reason); layout_object->InvalidateCacheAndMarkForLayout(reason);
} }
void SVGGradientElement::InvalidateDependentGradients() {
NotifyIncomingReferences(WTF::BindRepeating([](SVGElement& element) {
if (auto* gradient = ToSVGGradientElementOrNull(element)) {
gradient->InvalidateGradient(
LayoutInvalidationReason::kSvgResourceInvalidated);
}
}));
}
void SVGGradientElement::CollectCommonAttributes( void SVGGradientElement::CollectCommonAttributes(
GradientAttributes& attributes) const { GradientAttributes& attributes) const {
if (!attributes.HasSpreadMethod() && spreadMethod()->IsSpecified()) if (!attributes.HasSpreadMethod() && spreadMethod()->IsSpecified())
......
...@@ -61,6 +61,8 @@ class SVGGradientElement : public SVGElement, public SVGURIReference { ...@@ -61,6 +61,8 @@ class SVGGradientElement : public SVGElement, public SVGURIReference {
} }
void InvalidateGradient(LayoutInvalidationReasonForTracing); void InvalidateGradient(LayoutInvalidationReasonForTracing);
void InvalidateDependentGradients();
const SVGGradientElement* ReferencedElement() const; const SVGGradientElement* ReferencedElement() const;
void CollectCommonAttributes(GradientAttributes&) const; void CollectCommonAttributes(GradientAttributes&) const;
...@@ -79,13 +81,19 @@ class SVGGradientElement : public SVGElement, public SVGURIReference { ...@@ -79,13 +81,19 @@ class SVGGradientElement : public SVGElement, public SVGURIReference {
const AtomicString&, const AtomicString&,
MutableCSSPropertyValueSet*) override; MutableCSSPropertyValueSet*) override;
InsertionNotificationRequest InsertedInto(ContainerNode*) final;
void RemovedFrom(ContainerNode*) final;
void ChildrenChanged(const ChildrenChange&) final; void ChildrenChanged(const ChildrenChange&) final;
void BuildPendingResource();
void ClearResourceReferences();
Vector<Gradient::ColorStop> BuildStops() const; Vector<Gradient::ColorStop> BuildStops() const;
Member<SVGAnimatedTransformList> gradient_transform_; Member<SVGAnimatedTransformList> gradient_transform_;
Member<SVGAnimatedEnumeration<SVGSpreadMethodType>> spread_method_; Member<SVGAnimatedEnumeration<SVGSpreadMethodType>> spread_method_;
Member<SVGAnimatedEnumeration<SVGUnitTypes::SVGUnitType>> gradient_units_; Member<SVGAnimatedEnumeration<SVGUnitTypes::SVGUnitType>> gradient_units_;
Member<IdTargetObserver> target_id_observer_;
}; };
inline bool IsSVGGradientElement(const SVGElement& element) { inline bool IsSVGGradientElement(const SVGElement& element) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment