756462 - Stop unnecessary propagating COORD_CONTEXT_CHANGED notifications to descendants of nsSVGInnerSVGFrame

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Jonathan Watt [:jwatt]]

We should stop unnecessary and wastefully propagating COORD_CONTEXT_CHANGED notifications to descendants of nsSVGInnerSVGFrame. Doing so has the potential to cause us to do much more work than necessary, mainly in the form of repainting a much larger area of the screen than we need to.

Comment 1

[Jonathan Watt [:jwatt]]

Attachment: patch

Comment 2

[Jonathan Watt [:jwatt]]

Actually I've changed the comment to the following locally:

      // Remove COORD_CONTEXT_CHANGED, since we establish the coordinate
      // context for our descendants and this notification won't change its
      // dimensions:

Comment 3

[Robert Longson [:longsonr]]

If the inner svg originally had a width or height of 0px and we change to a non-zero px width/height or we have a viewBox and a width/height of 0% and we change to non-zero% width/height then we do need a notification I think as our children won't have had valid user units at all before. Can you add reftests to check each of these cases please and additional logic if necessary.

Comment 4

[Robert Longson [:longsonr]]

You might want to clone and adjust the reftests that went wrong when you sent your original patch for bug 413960 to try.

Comment 5

[Jonathan Watt [:jwatt]]

NotifySVGChanged is never called on a frame for changes to itself. It's called on descendants of the frame that changed. So in your examples, when the width/height/viewBox of the nsSVGInnerSVGFrame change, then NotifySVGChanged will be called on all its descendants without the opportunity for nsSVGInnerSVGFrame::NotifySVGChanged to stop that, since nsSVGInnerSVGFrame::AttributeChanged calls nsSVGUtils::NotifyChildrenOfSVGChange.

Comment 6

[Robert Longson [:longsonr]]

(In reply to Jonathan Watt [:jwatt] from comment #5)
> NotifySVGChanged is never called on a frame for changes to itself. It's
> called on descendants of the frame that changed. So in your examples, when
> the width/height/viewBox of the nsSVGInnerSVGFrame change, then
> NotifySVGChanged will be called on all its descendants without the
> opportunity for nsSVGInnerSVGFrame::NotifySVGChanged to stop that, since
> nsSVGInnerSVGFrame::AttributeChanged calls
> nsSVGUtils::NotifyChildrenOfSVGChange.

OK, I guess I buy that.

Comment 7

[Jonathan Watt [:jwatt]]

Pushed https://hg.mozilla.org/integration/mozilla-inbound/rev/17cad7e07453

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/17cad7e07453