Stop unnecessary propagating COORD_CONTEXT_CHANGED notifications to descendants of nsSVGInnerSVGFrame

RESOLVED FIXED in mozilla15

Status

()

RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: jwatt, Assigned: jwatt)

Tracking

({perf})

Trunk
mozilla15
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

7 years ago
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.
(Assignee)

Updated

7 years ago
Keywords: perf
(Assignee)

Comment 1

7 years ago
Created attachment 625103 [details] [diff] [review]
patch
Attachment #625103 - Flags: review?(longsonr)
(Assignee)

Comment 2

7 years ago
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:
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.
You might want to clone and adjust the reftests that went wrong when you sent your original patch for bug 413960 to try.
(Assignee)

Comment 5

7 years ago
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.
Attachment #625103 - Flags: review?(longsonr) → review+
(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.
(Assignee)

Comment 7

7 years ago
Pushed https://hg.mozilla.org/integration/mozilla-inbound/rev/17cad7e07453
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/17cad7e07453
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.