Ah, whoops. https://chromium-review.googlesource.com/c/chromium/src/+/1060693/ was not meant to remove that test as well, apparently I slightly mis-edited the LeakExpectations file.

Will send out a CL to re-add to LeakExpectations until it can be actually fixed.

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

commit bc61471975bdb827ce495195b56b88fe05f7785d
Author: Stephen McGruer <smcgruer@chromium.org>
Date: Mon Aug 27 21:04:34 2018

Remark svg/animations/additive-type-by-animation.html as leaky

This was accidentally removed in
https://chromium-review.googlesource.com/c/chromium/src/+/1060693/, which fixed
another set of leaks.

Bug:  878032 
Change-Id: I70c6cf10bfb1f3d7e1552a1f637878a8322c7a39
Reviewed-on: https://chromium-review.googlesource.com/1191672
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586414}
[modify] https://crrev.com/bc61471975bdb827ce495195b56b88fe05f7785d/third_party/WebKit/LayoutTests/LeakExpectations

Keeping this bug around to track the leak, but it has once more been suppressed so shouldn't be on the sheriff queue.

Appears to be caused by the following line in third_party/WebKit/LayoutTests/svg/animations/resources/additive-type-by-animation.svg:

<!-- AnimatedColor -->
<animate xlink:href="#rect" attributeName="fill" begin="0s" dur="4s" by="green" fill="freeze"/>


Without this animation, there is no leak I believe.

The target is suspicious, as it has a filter in it (which has previously caused leaks, see  issue 821547 

Minimal repro attached

Deleted: additive-type-by-animation.svg 477 bytes

additive-type-by-animation.svg 477 bytes Download

This is definitely related to the filter and SVGLocalResource[0], but I cannot figure out where we jump out of Oilpan to make the cycle uncollectable :/

The animation is also necessary to make it leak, without the animation a FilterOperations is created but is also cleaned up. 

[0] Overriding StyleBuilderConverter::ConvertFilterOperations to just return a blank FilterOperations makes the leak go away.

I've learnt a small amount of perhaps useful, perhaps not useful information.

FilterOperations is stored inside a StyleFilterData (https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/style/style_filter_data.h) which is GC'd but which is stored as a Persistent<> inside ComputedStyleBase. ComputedStyle (subclass of ComputedStyleBase) is stored in a scoped_refptr<>, so if something GC'd managed to get ahold of a ComputedStyle reference (possibly indirectly) then it seems quite trivial for a leak to happen.

This is my current theory. It may be wrong.

The animation in the repro is animating the 'fill' property:

<animate xlink:href="#rect" attributeName="fill" begin="0s" dur="4s" by="green" fill="freeze"/>

'fill' is not a registered property for SVGRectElement (https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/svg/svg_rect_element.cc?l=60&rcl=4c826ea1e53dbdaeb9c9c399c6c123087901b853). As such, SVGAnimateElement::ResolveTargetProperty (https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/svg/svg_animate_element.cc?l=171&rcl=3ef2ffbb5222d9074728217c0c6db9b4c07a69ca) will set target_property_ to nullptr, and IsAnimatingSVGDom() will return false.

We now move onto SVGAnimateElement::ResetAnimatedType. Since IsAnimatingSVGDom() is false, we call into ComputeCSSPropertyValue (https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/svg/svg_animate_element.cc?l=49&rcl=0314bf927d3f487c0e290d2e50eaacd86d0f6f56). This then calls into:

CSSComputedStyleDeclaration::GetPropertyValue
  --> CSSComputedStyleDeclaration::GetPropertyCSSValue
    --> CSSComputedStyleDeclaration::ComputeComputedStyle
      --> Node::EnsureComputedStyle
        --> SVGElement::VirtualEnsureComputedStyle
          --> SVGElement::EnsureComputedStyle

Ok, so what does SVGElement::EnsureComputedStyle do? Well, under certain circumstances it calls into SVGElementRareData::OverrideComputedStyle. This creates a ComputedStyle and stores it in a scoped_refptr<ComputedStyle>. The SVGElementRareData is stored in a Member<SVGElementRareData> in the SVGElement.

So if we combine this with #10, we have a theory of:

i. Document holds a GC path to SVGElement (this seems reasonable, though unproven) and thus to SVGElementRareData
ii. SVGElementRareData has a scoped_refptr<ComputedStyle> (jumping out of GC)
iii. ComputedStyle has a Persistent<StyleFilterData>.
iv. StyleFilterData has a FilterOperations, which has a HeapVector<Member<FilterOperation>>
v. ReferenceFilterOperation is a subclass of FilterOperation, and has a Member<SVGResource>
vi. LocalSVGResource is a subclass of SVGResource and has a Member<TreeScope>, which eventually leads back to the document.

And thus an uncollectable cycle is born.

**If** this theory is correct, fixing it is going to be... painful. I can't imagine just making ComputedStyle GC'd, for a start ;).

I think the first step will be to confirm that I'm not just crazy, and find some experts in SVG and ComputedStyle to pull in.

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

commit 9a16de1e8297267503c4573f57f84f443a6c0449
Author: Stephen McGruer <smcgruer@chromium.org>
Date: Fri Sep 07 22:32:58 2018

Clear ComputedStyle in SVGElement::DetachLayoutTree to avoid reference cycle

Storing a scoped_refptr<ComputedStyle> in SVGElementRareData can cause a
noncollectable Blink GC reference cycle, as there can be a path from the
Document --> SVGElementRareData, and a path from ComputedStyle -->
SVGLocalResource --> Document. The scoped_refptr in the center stops
Oilpan from being able to detect the cycle.

To fix this we clear the overridden ComputedStyle when the SVGElement is
detached from the layout tree. Doing so breaks the cycle and allows
Oilpan to collect everything as normal

Bug:  878032 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ib1095a9ae63b0499332e0e7369621ec6b8f961c0
Reviewed-on: https://chromium-review.googlesource.com/1213367
Reviewed-by: Anders Ruud <andruud@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589692}
[modify] https://crrev.com/9a16de1e8297267503c4573f57f84f443a6c0449/third_party/WebKit/LayoutTests/LeakExpectations
[modify] https://crrev.com/9a16de1e8297267503c4573f57f84f443a6c0449/third_party/blink/renderer/core/svg/svg_element.cc
[modify] https://crrev.com/9a16de1e8297267503c4573f57f84f443a6c0449/third_party/blink/renderer/core/svg/svg_element_rare_data.cc
[modify] https://crrev.com/9a16de1e8297267503c4573f57f84f443a6c0449/third_party/blink/renderer/core/svg/svg_element_rare_data.h