Bug 33835

Summary: [Gtk] svg/custom/use-instanceRoot-event-bubbling.xhtml fails
Product: WebKit Reporter: Xan Lopez <xan.lopez>
Component: SVGAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, krit, ojan, robert, xan.lopez, zan, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Initial patch
none
Updated patch
none
Updated patch v2
none
Updated patch v3 none

Description Xan Lopez 2010-01-19 01:01:19 PST
Parts of its state machine are constructed with very quick timeouts (100ms), which is a good way of making a test flakey :) (and, for instance, fails in the GTK+ bots).
Comment 1 Eric Seidel (no email) 2010-01-19 12:18:22 PST
http://trac.webkit.org/browser/trunk/LayoutTests/svg/custom/use-instanceRoot-event-bubbling.xhtml
Added by WildFox 15 months ago.

Next time I see it fail, I'll link to the failure diff.
Comment 2 Nikolas Zimmermann 2010-01-19 19:27:36 PST
This is true, the test is flakey as all <use> tests are, because of a problem with instanceRoot() - if the renderer is not attached yet, we're just returning 0, instead of asking the document to update style.

This is only one part of the issue, there is a race condition with event listeners, that currently still cause shadow tree reclones, I've fixed all these issues, and stability seems fine:

run-webkit-tests --repeat-each 50 -p `ls -l use-*.svg | sed -e "s/.*\ use/svg\/custom\/use/" | xargs`
Testing 49 test cases, repeating each test 50 times.
svg/custom ..................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................
100.25s total testing time

all 2450 test cases succeeded
Comment 3 Eric Seidel (no email) 2010-01-19 19:39:38 PST
So you have a patch to fix this? Or one was already landed?  In which case, the revision link would be useful. :)
Comment 4 Nikolas Zimmermann 2010-01-19 20:02:37 PST
Created attachment 46969 [details]
Initial patch

Ran run-webkit-tests -p svg --tolerance 0, no new regressions. Ran all use* testcases with --repeat-each 50 -p, saw no leaks and/or timing issues.
Reenabling all affected testcases on all platforms. Let's hope <use> is stable now, when scripted from JS.

This is also a huge performance benefit when working with <use> + event listeners.
Comment 5 Nikolas Zimmermann 2010-01-19 20:03:25 PST
*** Bug 32519 has been marked as a duplicate of this bug. ***
Comment 6 Eric Seidel (no email) 2010-01-19 20:08:35 PST
Attachment 46969 [details] did not build on mac:
Build output: http://webkit-commit-queue.appspot.com/results/200841
Comment 7 Nikolas Zimmermann 2010-01-19 20:15:13 PST
Created attachment 46970 [details]
Updated patch

Fix release builds, thanks to ews :-)
Comment 8 Eric Seidel (no email) 2010-01-19 20:20:21 PST
Attachment 46970 [details] did not build on mac:
Build output: http://webkit-commit-queue.appspot.com/results/201057
Comment 9 Nikolas Zimmermann 2010-01-19 20:40:19 PST
Created attachment 46973 [details]
Updated patch v2

Damn, got it wrong - ASSERT_UNUSED requires 2 params - fixed.
Comment 10 Nikolas Zimmermann 2010-01-20 12:53:54 PST
Created attachment 47053 [details]
Updated patch v3

Fix issues ap mentioned on IRC.
Comment 11 Alexey Proskuryakov 2010-01-20 13:08:33 PST
Comment on attachment 47053 [details]
Updated patch v3

r=me. I didn't review SVG parts very deeply.

>     // creation, as we can't wait for the lazy creation to happen if ie. JS wants to

This comment is old - but it sounds like that should be "e.g.", not "i.e."
Comment 12 Nikolas Zimmermann 2010-01-20 14:01:58 PST
Landed in r53564.
Comment 13 Nikolas Zimmermann 2010-01-20 17:15:15 PST
Still fails on Gtk, skipped there - works fine on other platforms so it must be a Gtk problem. The setTimeout(, 100) calls are gone, so it must be another issue. Marking as Gtk specific bug.
Comment 14 Eric Seidel (no email) 2010-02-02 14:14:20 PST
Comment on attachment 47053 [details]
Updated patch v3

Obsoleting this patch since it was landed.
Comment 15 Nikolas Zimmermann 2010-05-06 06:29:54 PDT
Can any gtk person run this test, and produce the actual/diffs.txt files and attach them here?
Comment 16 Nikolas Zimmermann 2010-07-09 07:19:42 PDT
Changed component to SVG, so it shows up in my all-svg-bugs search.