1090936 - Make invalidation of rendering observers async

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Robert Longson [:longsonr]]

Attachment: async.txt

Builds on bug 975757 and converts the remaining InvalidateRenderingObservers calls so all tree crawling is done by the restyle manager

Comment 1

[Jonathan Watt [:jwatt]]

The SVGFE*Frame and SVGStopFrame changes seem wrong, and I don't think they will pass Try. We only add observers to elements that we obtain via nsSVGRenderingObserver::GetReferencedElement(), right? So observers are only added to the <filter> and <linear/radialGradient> elements.

Comment 2

[Robert Longson [:longsonr]]

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=476a6d1a457c

Comment 3

[Robert Longson [:longsonr]]

In nsSVGStopFrame I've got nsSVGEffects::InvalidateDirectRenderingObservers(GetParent()); The parent of the stop frame is the gradient on which we have observers. Similar for the FE*Frame changes.

Comment 4

[Jonathan Watt [:jwatt]]

Ah, sorry, I failed to notice that part of the change.

Comment 5

[Jonathan Watt [:jwatt]]

Comment on attachment 8513449 [details] [diff] [review]
async.txt

Review of attachment 8513449 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great, thank you Robert.

::: layout/svg/SVGFEContainerFrame.cpp
@@ +100,5 @@
>                                        int32_t  aModType)
>  {
>    nsSVGFE *element = static_cast<nsSVGFE*>(mContent);
>    if (element->AttributeAffectsRendering(aNameSpaceID, aAttribute)) {
> +    nsSVGEffects::InvalidateDirectRenderingObservers(GetParent());

I think it's worth adding something like:

  MOZ_ASSERT(GetParent()->GetType() == nsGkAtoms::svgFilterFrame,
             "Observers observe the filter, so that's what we must invalidate");

here and equivalents to the other SVGFE* changes in case the spec/implementation changes to make it possible to fail in this way.

::: layout/svg/nsSVGStopFrame.cpp
@@ +97,5 @@
>                                   int32_t         aModType)
>  {
>    if (aNameSpaceID == kNameSpaceID_None &&
>        aAttribute == nsGkAtoms::offset) {
> +    nsSVGEffects::InvalidateDirectRenderingObservers(GetParent());

Add a similar assert here for nsGkAtoms::svgLinearGradientFrame and nsGkAtoms::svgRadialGradientFrame.

Comment 6

[Robert Longson [:longsonr]]

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=2fd36892ad6d

Comment 7

[Robert Longson [:longsonr]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7822514901e6

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7822514901e6