New issue
Advanced search Search tips

Issue 661598 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 24
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task

Blocking:
issue 769774



Sign in to add a comment

Use IdTargetObserver to track SVG*Element 'id' changes

Project Member Reported by f...@opera.com, Nov 2 2016

Issue description

Investigate if IdTargetObserver could serve as a replacement for the SVGDocumentExtensions::*PendingResource infrastructure (in most cases.)

A benefit of this could be better handling of duplicate ids and a "thinning" of the responsibilities of SVGDocumentExtensions. Would probably also help with some TreeScope issues in this area.
 

Comment 1 by f...@opera.com, Nov 2 2016

Components: Blink>SVG

Comment 2 by f...@opera.com, Nov 2 2016

Based on usage of addPendingResource, the following is likely to be affected:

 SVGFEImageElement (href)
 SVGMPathElement (href)
 SVGTextPathElement (href)
 SVGUseElement (href)
 SVGSMILElement (href and event-base)

The following also use the pending resource tracking, but should probably not be migrated in the same way as the above:

 LayoutSVGResourceContainer
 ReferenceFilterBuilder
 SVGResources
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 3 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9365003f3f4f921445eeb94413d7a29f996b4148

commit 9365003f3f4f921445eeb94413d7a29f996b4148
Author: fs <fs@opera.com>
Date: Thu Nov 03 08:41:47 2016

Simplify SVG pending resource (re)validation

The contents of the m_pendingResourcesForRemoval map has a lifespan that
does not extend beyond the scope of SVGElement's
buildPendingResourcesIfNeeded() method.
So instead of passing through the map in SVGDocumentExtensions, just
take the corresponding set for the pending 'id' and iterate that
directly, avoiding indirection and complicated removal sequence.
This also allow SVGDocumentExtensions::removeElementFromPendingResources
to be simplified, so do that, and then remove the
m_pendingResourcesForRemoval map from SVGDocumentExtensions.

BUG= 661598 

Review-Url: https://codereview.chromium.org/2473483004
Cr-Commit-Position: refs/heads/master@{#429539}

[modify] https://crrev.com/9365003f3f4f921445eeb94413d7a29f996b4148/third_party/WebKit/Source/core/svg/SVGDocumentExtensions.cpp
[modify] https://crrev.com/9365003f3f4f921445eeb94413d7a29f996b4148/third_party/WebKit/Source/core/svg/SVGDocumentExtensions.h
[modify] https://crrev.com/9365003f3f4f921445eeb94413d7a29f996b4148/third_party/WebKit/Source/core/svg/SVGElement.cpp

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/43f91d7f998095673e151be50bf17fe1b011f080

commit 43f91d7f998095673e151be50bf17fe1b011f080
Author: fs <fs@opera.com>
Date: Wed Mar 08 23:14:25 2017

Add a new mechanism for watching SVGElement 'href' targets

This CL adds a new mechanism for watching elements referenced via 'href'
attributes. It uses IdTargetObserver as the basis adding a callback for
notification.
This is a step away from relying on SVGTreeScopeResources for tracking
of "pending" elements for these use cases. Each element is instead does
its own tracking via the relevant TreeScope. This will eventually mean
that the buildPendingResourcesIfNeeded mechanism can be removed.

SVGTextPathElement and SVGMPathElement are updated to use this new
scheme. Other uses will be replaced in future CLs.

BUG= 661598 

Review-Url: https://codereview.chromium.org/2737653006
Cr-Commit-Position: refs/heads/master@{#455584}

[modify] https://crrev.com/43f91d7f998095673e151be50bf17fe1b011f080/third_party/WebKit/Source/core/svg/SVGMPathElement.cpp
[modify] https://crrev.com/43f91d7f998095673e151be50bf17fe1b011f080/third_party/WebKit/Source/core/svg/SVGMPathElement.h
[modify] https://crrev.com/43f91d7f998095673e151be50bf17fe1b011f080/third_party/WebKit/Source/core/svg/SVGTextPathElement.cpp
[modify] https://crrev.com/43f91d7f998095673e151be50bf17fe1b011f080/third_party/WebKit/Source/core/svg/SVGTextPathElement.h
[modify] https://crrev.com/43f91d7f998095673e151be50bf17fe1b011f080/third_party/WebKit/Source/core/svg/SVGURIReference.cpp
[modify] https://crrev.com/43f91d7f998095673e151be50bf17fe1b011f080/third_party/WebKit/Source/core/svg/SVGURIReference.h

Project Member

Comment 5 by bugdroid1@chromium.org, Mar 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/32a58c7e9cf5f455eb07420587efec657b1409ba

commit 32a58c7e9cf5f455eb07420587efec657b1409ba
Author: fs <fs@opera.com>
Date: Thu Mar 09 16:37:29 2017

Use IdTargetObserver in SVGFEImageElement

Change SVGFEImageElement to use the observeTarget() helper from
SVGURIReference.
A slight change in behavior for when a load is initiated for a potential
image resource is made. Instead of using a "failed element lookup and a
non-existing id" as the condition, use "failed element lookup and non-
local resource reference".
Also add a new method clearImageResource() and put the tear-down for
the image resource, and change a use of ownerDocument() to just
document().

BUG= 661598 

Review-Url: https://codereview.chromium.org/2740003003
Cr-Commit-Position: refs/heads/master@{#455765}

[modify] https://crrev.com/32a58c7e9cf5f455eb07420587efec657b1409ba/third_party/WebKit/Source/core/svg/SVGFEImageElement.cpp
[modify] https://crrev.com/32a58c7e9cf5f455eb07420587efec657b1409ba/third_party/WebKit/Source/core/svg/SVGFEImageElement.h

Project Member

Comment 6 by bugdroid1@chromium.org, Mar 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9b88406d1a87dfeef593c6d1a91beb3a2b696391

commit 9b88406d1a87dfeef593c6d1a91beb3a2b696391
Author: fs <fs@opera.com>
Date: Thu Mar 09 16:47:37 2017

Use IdTargetObserver in SVGUseElement

Change SVGUseElement to use the observeTarget() helper from
SVGURIReference. Since SVGUseElement has some additional requirements
for its reference management, a more low-level observeTarget() variant
is exposed.
To facilitate this change, clearShadowTree() is renamed to
clearResourceReference(), and the shadow tree tear-down is hoisted out
of it.

BUG= 661598 

Review-Url: https://codereview.chromium.org/2744613002
Cr-Commit-Position: refs/heads/master@{#455769}

[modify] https://crrev.com/9b88406d1a87dfeef593c6d1a91beb3a2b696391/third_party/WebKit/Source/core/svg/SVGURIReference.cpp
[modify] https://crrev.com/9b88406d1a87dfeef593c6d1a91beb3a2b696391/third_party/WebKit/Source/core/svg/SVGURIReference.h
[modify] https://crrev.com/9b88406d1a87dfeef593c6d1a91beb3a2b696391/third_party/WebKit/Source/core/svg/SVGUseElement.cpp
[modify] https://crrev.com/9b88406d1a87dfeef593c6d1a91beb3a2b696391/third_party/WebKit/Source/core/svg/SVGUseElement.h

Comment 7 by f...@opera.com, Mar 10 2017

I'm looking at moving SVG{Linear,Radial}GradientElement and SVGPatternElement over to this mechanism as well, and removing the "linked resource" from SVGResources. (This tightens the relationship between ComputedStyle and SVGResources.)
Project Member

Comment 8 by bugdroid1@chromium.org, Mar 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ef3b9727c297214c9a7093cdf51f92200d78140c

commit ef3b9727c297214c9a7093cdf51f92200d78140c
Author: fs <fs@opera.com>
Date: Mon Mar 13 13:56:59 2017

Use IdTargetObserver in SVGSMILElement

Move SVGSMILElement (target reference) and SVGSMILElement::Condition to
use IdTargetObserver via SVGURIReference::observeTarget.
Simplify the result to avoid needing the lookupEventBase helper.

BUG= 661598 

Review-Url: https://codereview.chromium.org/2737403003
Cr-Commit-Position: refs/heads/master@{#456367}

[modify] https://crrev.com/ef3b9727c297214c9a7093cdf51f92200d78140c/third_party/WebKit/Source/core/svg/SVGURIReference.cpp
[modify] https://crrev.com/ef3b9727c297214c9a7093cdf51f92200d78140c/third_party/WebKit/Source/core/svg/SVGURIReference.h
[modify] https://crrev.com/ef3b9727c297214c9a7093cdf51f92200d78140c/third_party/WebKit/Source/core/svg/animation/SVGSMILElement.cpp
[modify] https://crrev.com/ef3b9727c297214c9a7093cdf51f92200d78140c/third_party/WebKit/Source/core/svg/animation/SVGSMILElement.h

Project Member

Comment 9 by bugdroid1@chromium.org, Mar 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6729d1ecf49a910999920ee6102bc48821127607

commit 6729d1ecf49a910999920ee6102bc48821127607
Author: fs <fs@opera.com>
Date: Mon Mar 13 14:23:27 2017

Move common gradient attribute collection to SVGGradientElement

Move the collection of attributes shared by both gradient types to the
base class (collectCommonAttributes.) Introduce a new helper for looking
up a potential element to inherit attributes from. Restructure the
"collection loop" a bit after the common code has been broken out.
This also means that buildStops() can be made private.

BUG= 661598 

Review-Url: https://codereview.chromium.org/2741993002
Cr-Commit-Position: refs/heads/master@{#456371}

[modify] https://crrev.com/6729d1ecf49a910999920ee6102bc48821127607/third_party/WebKit/Source/core/svg/SVGGradientElement.cpp
[modify] https://crrev.com/6729d1ecf49a910999920ee6102bc48821127607/third_party/WebKit/Source/core/svg/SVGGradientElement.h
[modify] https://crrev.com/6729d1ecf49a910999920ee6102bc48821127607/third_party/WebKit/Source/core/svg/SVGLinearGradientElement.cpp
[modify] https://crrev.com/6729d1ecf49a910999920ee6102bc48821127607/third_party/WebKit/Source/core/svg/SVGRadialGradientElement.cpp

Project Member

Comment 10 by bugdroid1@chromium.org, Mar 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/de33d7047da8e6cac634efa0d4160eb8aa7716e0

commit de33d7047da8e6cac634efa0d4160eb8aa7716e0
Author: fs <fs@opera.com>
Date: Mon Mar 13 15:11:49 2017

Remove <filter> from the chainableResourceTags set

We don't actually support inheritance for <filter>, so having it in the
set is of no use.

BUG= 661598 

Review-Url: https://codereview.chromium.org/2743293002
Cr-Commit-Position: refs/heads/master@{#456380}

[modify] https://crrev.com/de33d7047da8e6cac634efa0d4160eb8aa7716e0/third_party/WebKit/Source/core/layout/svg/SVGResources.cpp

Project Member

Comment 11 by bugdroid1@chromium.org, Mar 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c9632b65e4048becc539dd103028a01a8d60e55c

commit c9632b65e4048becc539dd103028a01a8d60e55c
Author: fs <fs@opera.com>
Date: Tue Mar 14 09:58:10 2017

Remove argument to LayoutSVGResourceGradient::collectGradientAttributes

We can assume that element() is non-null, and just cast it in the
overriding implementations.
Move the synchronizeAnimatedSVGAttribute(...) calls into actual
attribute collection, so that it applies to all elements in the
inheritance chain.
Also rewrite the lengthy comment, because gradient building has changed
significantly from what it describes, and attribute collection now
precedes the actual Gradient construction. Replicate the new comment to
the similar place for <pattern>s.

BUG= 661598 

Review-Url: https://codereview.chromium.org/2749593002
Cr-Commit-Position: refs/heads/master@{#456662}

[modify] https://crrev.com/c9632b65e4048becc539dd103028a01a8d60e55c/third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceGradient.cpp
[modify] https://crrev.com/c9632b65e4048becc539dd103028a01a8d60e55c/third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceGradient.h
[modify] https://crrev.com/c9632b65e4048becc539dd103028a01a8d60e55c/third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceLinearGradient.cpp
[modify] https://crrev.com/c9632b65e4048becc539dd103028a01a8d60e55c/third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceLinearGradient.h
[modify] https://crrev.com/c9632b65e4048becc539dd103028a01a8d60e55c/third_party/WebKit/Source/core/layout/svg/LayoutSVGResourcePattern.cpp
[modify] https://crrev.com/c9632b65e4048becc539dd103028a01a8d60e55c/third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceRadialGradient.cpp
[modify] https://crrev.com/c9632b65e4048becc539dd103028a01a8d60e55c/third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceRadialGradient.h
[modify] https://crrev.com/c9632b65e4048becc539dd103028a01a8d60e55c/third_party/WebKit/Source/core/svg/SVGLinearGradientElement.cpp
[modify] https://crrev.com/c9632b65e4048becc539dd103028a01a8d60e55c/third_party/WebKit/Source/core/svg/SVGRadialGradientElement.cpp

Comment 12 by f...@opera.com, Jan 16 2018

Blocking: 769774
Project Member

Comment 13 by bugdroid1@chromium.org, Jan 22 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c36508a349b4bff18c73f2e2e2b46805ad091598

commit c36508a349b4bff18c73f2e2e2b46805ad091598
Author: Fredrik Söderquist <fs@opera.com>
Date: Mon Jan 22 19:01:26 2018

[CI] DOM-based SVG resource tracking (1/n)

This starts the transition away from tracking SVG resources based on LayoutObjects.
In the new (probably transitional) model, the resources are track via a Resource
object (inner class) on SVGTreeScopeResources. These Resource objects are
IdTargetObservers, meaning that id mutations no longer are tracked via hooks in
SVGElement. The "pending resource handling" mechanism is also moved to the Resource
objects in the form of a "pending clients" set. The old mechanism (i.e
NeedsPendingResourceHandling() and BuildPendingResourcesIfNeeded()) are no longer
needed and hence removed.

Next step will be to eliminate all remaining tracking logic from
LayoutSVGResourceContainer.

Somewhat independently, a new entry point ResourceReferenceChanged is added to
SVGResourcesCache, codifying the sequence that previously involved calls to
ClientStyleChanged and MarkForLayoutAndParentResourceInvalidation.

Bug:  454767 ,  661598 , 769774, 784435
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I268f2e2a2a5e2e01f077a6b0a5b6270f668a2a6f
Reviewed-on: https://chromium-review.googlesource.com/689998
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#530930}
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/LayoutTests/paint/invalidation/svg/js-late-marker-and-object-creation-expected.txt
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/LayoutTests/paint/invalidation/svg/js-late-marker-creation-expected.txt
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/LayoutTests/paint/invalidation/svg/js-late-mask-and-object-creation-expected.txt
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/LayoutTests/paint/invalidation/svg/js-late-mask-creation-expected.txt
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/LayoutTests/platform/mac/paint/invalidation/svg/js-late-clipPath-and-object-creation-expected.txt
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/LayoutTests/platform/mac/paint/invalidation/svg/js-late-clipPath-creation-expected.txt
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/LayoutTests/platform/mac/paint/invalidation/svg/js-late-gradient-creation-expected.txt
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/LayoutTests/platform/mac/paint/invalidation/svg/js-late-pattern-creation-expected.txt
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/LayoutTests/platform/win/paint/invalidation/svg/js-late-clipPath-and-object-creation-expected.txt
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/LayoutTests/platform/win/paint/invalidation/svg/js-late-clipPath-creation-expected.txt
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/LayoutTests/platform/win/paint/invalidation/svg/js-late-gradient-creation-expected.txt
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/LayoutTests/platform/win/paint/invalidation/svg/js-late-pattern-creation-expected.txt
[add] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/LayoutTests/svg/custom/duplicate-ids-remove-first-expected.html
[add] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/LayoutTests/svg/custom/duplicate-ids-remove-first.html
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/LayoutTests/virtual/spv175/paint/invalidation/svg/js-late-gradient-creation-expected.txt
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/LayoutTests/virtual/spv175/paint/invalidation/svg/js-late-marker-and-object-creation-expected.txt
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/LayoutTests/virtual/spv175/paint/invalidation/svg/js-late-marker-creation-expected.txt
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/LayoutTests/virtual/spv175/paint/invalidation/svg/js-late-mask-and-object-creation-expected.txt
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/LayoutTests/virtual/spv175/paint/invalidation/svg/js-late-mask-creation-expected.txt
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/LayoutTests/virtual/spv175/paint/invalidation/svg/js-late-pattern-creation-expected.txt
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/Source/core/dom/Element.cpp
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/Source/core/dom/IdTargetObserver.h
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.cpp
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.h
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/Source/core/layout/svg/SVGResources.cpp
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/Source/core/layout/svg/SVGResources.h
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/Source/core/layout/svg/SVGResourcesCache.cpp
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/Source/core/layout/svg/SVGResourcesCache.h
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/Source/core/svg/SVGClipPathElement.h
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/Source/core/svg/SVGElement.cpp
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/Source/core/svg/SVGElement.h
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/Source/core/svg/SVGFilterElement.h
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/Source/core/svg/SVGGradientElement.h
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/Source/core/svg/SVGMarkerElement.h
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/Source/core/svg/SVGMaskElement.h
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/Source/core/svg/SVGPatternElement.h
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/Source/core/svg/SVGTreeScopeResources.cpp
[modify] https://crrev.com/c36508a349b4bff18c73f2e2e2b46805ad091598/third_party/WebKit/Source/core/svg/SVGTreeScopeResources.h

Project Member

Comment 14 by bugdroid1@chromium.org, Jan 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4984b61ecda63c05f602bbebabdb841ec2603bc0

commit 4984b61ecda63c05f602bbebabdb841ec2603bc0
Author: Fredrik Söderquist <fs@opera.com>
Date: Tue Jan 23 18:35:43 2018

[CI] Move SVG dependency graph notification into SVGElement

This moves a chunk of code pertaining to notifications of dependent
elements from a helper LayoutSVGResourceContainer.cpp, thus collection
all such operations within the realms of the SVGElement DOM.

Bug:  454767 ,  661598 , 769774
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I18b9a982b43767f2d8f0b8f458ea93e5bdd5e786
Reviewed-on: https://chromium-review.googlesource.com/881502
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#531285}
[modify] https://crrev.com/4984b61ecda63c05f602bbebabdb841ec2603bc0/third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.cpp
[modify] https://crrev.com/4984b61ecda63c05f602bbebabdb841ec2603bc0/third_party/WebKit/Source/core/svg/SVGElement.cpp
[modify] https://crrev.com/4984b61ecda63c05f602bbebabdb841ec2603bc0/third_party/WebKit/Source/core/svg/SVGElement.h

Project Member

Comment 15 by bugdroid1@chromium.org, Feb 12 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/945b08784087ba5df9ed3f504a3eb5986f6bcbce

commit 945b08784087ba5df9ed3f504a3eb5986f6bcbce
Author: Fredrik Söderquist <fs@opera.com>
Date: Mon Feb 12 16:57:19 2018

[CI] Add SVGGradientElement::InvalidateGradient

Add a new helper method on SVGGradientElement that encapsulates the code
that invalidates the gradient data/cache. Use this method where
appropriate.

Add an InvalidateCacheAndMarkForLayout overload taking a
LayoutInvalidationReason to LayoutSVGResourceContainer, and use it in
SVGElement::InvalidateRelativeLengthClients and the new method, which
also takes a LayoutInvalidationReason to improve fidelity of reporting.

The change to use SVGGradientElement::InvalidateGradient in
ChildrenChanged constitutes a minor change in behavior since it
previously didn't have the SelfNeedsLayout() check. Said check should be
reasonable to have in this code-path as well though.

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: Ia5c318a4df958960aabb56f71ad3c384791a46b0
Reviewed-on: https://chromium-review.googlesource.com/913688
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#536107}
[modify] https://crrev.com/945b08784087ba5df9ed3f504a3eb5986f6bcbce/third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.cpp
[modify] https://crrev.com/945b08784087ba5df9ed3f504a3eb5986f6bcbce/third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.h
[modify] https://crrev.com/945b08784087ba5df9ed3f504a3eb5986f6bcbce/third_party/WebKit/Source/core/svg/SVGElement.cpp
[modify] https://crrev.com/945b08784087ba5df9ed3f504a3eb5986f6bcbce/third_party/WebKit/Source/core/svg/SVGGradientElement.cpp
[modify] https://crrev.com/945b08784087ba5df9ed3f504a3eb5986f6bcbce/third_party/WebKit/Source/core/svg/SVGGradientElement.h
[modify] https://crrev.com/945b08784087ba5df9ed3f504a3eb5986f6bcbce/third_party/WebKit/Source/core/svg/SVGLinearGradientElement.cpp
[modify] https://crrev.com/945b08784087ba5df9ed3f504a3eb5986f6bcbce/third_party/WebKit/Source/core/svg/SVGRadialGradientElement.cpp
[modify] https://crrev.com/945b08784087ba5df9ed3f504a3eb5986f6bcbce/third_party/WebKit/Source/core/svg/SVGStopElement.cpp

Comment 16 by f...@opera.com, Feb 12 2018

Labels: -Type-Bug Type-Task
Project Member

Comment 17 by bugdroid1@chromium.org, Mar 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ce851e47bd77773970a805acd62ec1627b0aaa76

commit ce851e47bd77773970a805acd62ec1627b0aaa76
Author: Fredrik Söderquist <fs@opera.com>
Date: Fri Mar 16 13:43:42 2018

[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: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543685}
[modify] https://crrev.com/ce851e47bd77773970a805acd62ec1627b0aaa76/third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.cpp
[modify] https://crrev.com/ce851e47bd77773970a805acd62ec1627b0aaa76/third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceGradient.cpp
[modify] https://crrev.com/ce851e47bd77773970a805acd62ec1627b0aaa76/third_party/WebKit/Source/core/layout/svg/SVGResources.cpp
[modify] https://crrev.com/ce851e47bd77773970a805acd62ec1627b0aaa76/third_party/WebKit/Source/core/svg/SVGElement.cpp
[modify] https://crrev.com/ce851e47bd77773970a805acd62ec1627b0aaa76/third_party/WebKit/Source/core/svg/SVGElement.h
[modify] https://crrev.com/ce851e47bd77773970a805acd62ec1627b0aaa76/third_party/WebKit/Source/core/svg/SVGGradientElement.cpp
[modify] https://crrev.com/ce851e47bd77773970a805acd62ec1627b0aaa76/third_party/WebKit/Source/core/svg/SVGGradientElement.h

Project Member

Comment 18 by bugdroid1@chromium.org, Mar 20 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2df427946b090559b8a9a66c03a3b032e28f6511

commit 2df427946b090559b8a9a66c03a3b032e28f6511
Author: Fredrik Söderquist <fs@opera.com>
Date: Tue Mar 20 17:49:33 2018

[PE] Don't use callbacks for SVGElement::NotifyIncomingReferences

Dispatch overhead is high.

Bug:  661598 , 769774,  823473 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I4b200d0076c209584a033be810f95c504e81c102
Reviewed-on: https://chromium-review.googlesource.com/970422
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#544431}
[modify] https://crrev.com/2df427946b090559b8a9a66c03a3b032e28f6511/third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.cpp
[modify] https://crrev.com/2df427946b090559b8a9a66c03a3b032e28f6511/third_party/WebKit/Source/core/svg/SVGElement.cpp
[modify] https://crrev.com/2df427946b090559b8a9a66c03a3b032e28f6511/third_party/WebKit/Source/core/svg/SVGElement.h
[modify] https://crrev.com/2df427946b090559b8a9a66c03a3b032e28f6511/third_party/WebKit/Source/core/svg/SVGGradientElement.cpp

Project Member

Comment 19 by bugdroid1@chromium.org, Mar 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2e4b433f9dac35b965e797247b8d15f287cad05c

commit 2e4b433f9dac35b965e797247b8d15f287cad05c
Author: Fredrik Söderquist <fs@opera.com>
Date: Fri Mar 23 16:03:25 2018

[PE] Restore LayoutObject check in SVGElement::NotifyIncomingReferences

When an element has incoming references that are not in the layout tree
(like an element animated by [multiple] SMIL elements), the overhead of
the hash set insert/erase can be noticable (like for instance in
balls_svg_animation.html.)

Bug:  661598 , 769774,  823473 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I8a5879186dabd5e071f708ac953b8f1e91b62847
Reviewed-on: https://chromium-review.googlesource.com/978213
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#545467}
[modify] https://crrev.com/2e4b433f9dac35b965e797247b8d15f287cad05c/third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceContainer.cpp
[modify] https://crrev.com/2e4b433f9dac35b965e797247b8d15f287cad05c/third_party/WebKit/Source/core/svg/SVGElement.h

Status: Fixed (was: Assigned)
I think this can be considered complete now. (The old-style notifier is gone.)

Sign in to add a comment