In SVGFEImageFrame::Init we call FrameCreated and then IncrementVisibleCount on the associated nsImageLoadingContent. However in Init the primary frame pointer hasn't been setup yet. FrameCreated correctly works around this by passing the SKIP_FRAME_CHECK to TrackImage. TrackImage won't track unless the visible count is positive and there is a primary frame. So if we call FrameCreated first the TrackImage call won't track because the visible count is 0 (even though we correctly skip the primary frame check). We want to call IncrementVisibleCount first so the visible count is positive when FrameCreated calls TrackImage. This is kind of fragile, I'm open to a better solution. This came up while testing my patches for 847223.
Attachment #742131 - Flags: review?(matspal)
Comment on attachment 742131 [details] [diff] [review] patch Looks good, r=mats I find the comment is a little confusing though; couldn't you just say: + // Increment the visible count before calling FrameCreated so that + // tracks the image. I think the primary frame / SKIP_FRAME_CHECK part would be clearer as a separate comment before the FrameCreated() call, if needed. (it's already documented in FrameCreated)
Attachment #742131 - Flags: review?(matspal) → review+
Ok, I changed the comment to be basically what you suggested.
Comment on attachment 742131 [details] [diff] [review] patch [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 689623 User impact if declined: dynamically added SVGFEImage's won't animate Testing completed (on m-c, etc.): just landed on inbound now Risk to taking this patch (and alternatives if risky): this should be a safe patch, I think the risk of not taking it is higher than the risk of taking it String or IDL/UUID changes made by this patch: none
Attachment #742131 - Flags: approval-mozilla-aurora?
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Attachment #742131 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Component: Layout: Misc Code → Layout
Product: Core Graveyard → Core
You need to log in before you can comment on or make changes to this bug.