Created attachment 153859 [details] Fill animation not working Animation is activated from Document::implicitClose(), but deferred element insertion doesn't seem to hit that path.
Created attachment 155829 [details] Patch
Comment on attachment 155829 [details] Patch Looks sane, r=me.
Dicsussing this patch on IRC now. At least the layout test needs an update.
Comment on attachment 155829 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=155829&action=review > LayoutTests/svg/animations/deferred-insertion.html:16 > + if (window.layoutTestController) > + layoutTestController.waitUntilDone(); We discussed the patch online during your review Niko. I am fine with the patch as well, but window.layoutTestController needs to be replaced.
Created attachment 155847 [details] Patch for landing
Thanks for reviewing this Niko & Dirk.
(In reply to comment #4) > We discussed the patch online during your review Niko. I am fine with the patch as well, but window.layoutTestController needs to be replaced. Good catch!
Comment on attachment 155847 [details] Patch for landing Clearing flags on attachment: 155847 Committed r124369: <http://trac.webkit.org/changeset/124369>
All reviewed patches have been landed. Closing bug.
I think this change may have introduced a performance regression: http://webkit-perf.appspot.com/graph.html#tests=[[1610803,2001,173262]]&sel=none&displayrange=7&datatype=running Pageload time went from 88ms to 92ms average. None of the boolean checks added in this patch are more than just local variable reads, so I don't know what's up.
(In reply to comment #10) > I think this change may have introduced a performance regression: > http://webkit-perf.appspot.com/graph.html#tests=[[1610803,2001,173262]]&sel=none&displayrange=7&datatype=running > > Pageload time went from 88ms to 92ms average. None of the boolean checks added in this patch are more than just local variable reads, so I don't know what's up. This seems unlikely not only because it's a lightweight test, but also because that code path is only hit on <svg> element insertion. And for this particular perftest, the conditional gets shortcut on the first test (document()->parsing() is true) as the SVG element is inserted during parsing. Have you ruled out all the other changes in range?
Created attachment 156402 [details] crawfish2_ganson.svg perftest run I ran the perftest locally, see attached graph. First 40 runs are without this patch, last 40 with it - I cannot see a difference.