open uri, wait around 5 seconds
Created attachment 56787 [details] crash log
Created attachment 56788 [details] copy of URI file at time of crash
Created attachment 56818 [details] Test case - crashes WebKit! This is a simple test case for the crash. It seems not possible, that a parent and a child element take the same filter: <g filter="url(#filter)"> <rect width="100" height="100" filter="url(#filter)"/> </g> Not absolutely sure, what causes this crash, but I guess it's the following line: http://trac.webkit.org/browser/trunk/WebCore/rendering/RenderSVGResourceFilter.cpp#L253 This overwrites the reference to the temporary context. The problem is maybe, that we overwrite the saved context twice by calling it twice and so loose the reference to the original context. We might think about storing the sourceGraphicBuffer and the saved context in FilterData. This should fix this issue, but I did not test it yet.
Created attachment 56876 [details] Patch
Comment on attachment 56876 [details] Patch LGTM, r=me.
(In reply to comment #3) > > This overwrites the reference to the temporary context. The problem is maybe, that we overwrite the saved context twice by calling it twice and so loose the reference to the original context. We might think about storing the sourceGraphicBuffer and the saved context in FilterData. This should fix this issue, but I did not test it yet. I've been looking at this bug from the chrome point of view, and agree about Dirk's suggested fix (I was getting ready to submit a similar patch). Can I please ask that the patch also be tested on http://upload.wikimedia.org/wikipedia/commons/b/b5/Cloud_computing.svg as it has a reasonably complicated nesting of parents/children in the filter structure?
Comment on attachment 56876 [details] Patch Clearing flags on attachment: 56876 Committed r60439: <http://trac.webkit.org/changeset/60439>
All reviewed patches have been landed. Closing bug.
There appears to be fairly identical context-saving code in RenderSVGResourceGradient{.h|.cpp}. Can this same problem arise with gradients also?
(In reply to comment #6) > (In reply to comment #3) > > > > This overwrites the reference to the temporary context. The problem is maybe, that we overwrite the saved context twice by calling it twice and so loose the reference to the original context. We might think about storing the sourceGraphicBuffer and the saved context in FilterData. This should fix this issue, but I did not test it yet. > > I've been looking at this bug from the chrome point of view, and agree about Dirk's suggested fix (I was getting ready to submit a similar patch). > > Can I please ask that the patch also be tested on > > http://upload.wikimedia.org/wikipedia/commons/b/b5/Cloud_computing.svg > > as it has a reasonably complicated nesting of parents/children in the filter structure? I checked the page on QtWebkit WebKitGtk and Chromium, all without the patch but filters enabled. Only Chromium crashed on this page. Are you sure, that this patch can fix the issue? Do you have a backtrace?
(In reply to comment #9) > There appears to be fairly identical context-saving code in RenderSVGResourceGradient{.h|.cpp}. > > Can this same problem arise with gradients also? Can you explain your fears a bit more? Filters can be applied to a parent as well as a child. This is not possible on gradients or patterns, since only RenderPath takes care about fill and stroke properties. But RenderPath can't have childs.
(In reply to comment #10) > > I checked the page on QtWebkit WebKitGtk and Chromium, all without the patch but filters enabled. Only Chromium crashed on this page. Are you sure, that this patch can fix the issue? Do you have a backtrace? I tried the patch this morning, and it does appear to fix the problem for Chromium.
(In reply to comment #11) > > Can you explain your fears a bit more? Filters can be applied to a parent as well as a child. This is not possible on gradients or patterns, since only RenderPath takes care about fill and stroke properties. But RenderPath can't have childs. My fear was that perhaps gradients could be applied to children as well, but if that's not the case then it's not an issue. Thanks!
(In reply to comment #13) > (In reply to comment #11) > > > > Can you explain your fears a bit more? Filters can be applied to a parent as well as a child. This is not possible on gradients or patterns, since only RenderPath takes care about fill and stroke properties. But RenderPath can't have childs. > > My fear was that perhaps gradients could be applied to children as well, but if that's not the case then it's not an issue. Thanks! Yes, only clipper and masker can be applied to parent and children beside filters. But I don't see a issue there. Only the boundries are used by all instances of the same resource and this is wanted and used to speed up the rendering.