Summary: | REGRESSION(r227983): fast/dom/adopt-node-crash-2.html is flaky | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ryosuke Niwa <rniwa> | ||||
Component: | UI Events | Assignee: | Ryosuke Niwa <rniwa> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | ap, cdumez, eocanha, jlewis3, koivisto, ryanhaddad, simon.fraser, webkit-bug-importer, wenson_hsieh | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Ryosuke Niwa
2018-02-07 17:08:33 PST
Ryosuke, is this something you intend to fix shortly or do you want to mark the test as failing? (In reply to Ryan Haddad from comment #1) > Ryosuke, is this something you intend to fix shortly or do you want to mark > the test as failing? Sorry, adding a test expectation short terms is a good fix. Will look into it ASAP but I have another bug I need to urgently address first. (In reply to Ryosuke Niwa from comment #2) > (In reply to Ryan Haddad from comment #1) > > Ryosuke, is this something you intend to fix shortly or do you want to mark > > the test as failing? > > Sorry, adding a test expectation short terms is a good fix. Will look into > it ASAP but I have another bug I need to urgently address first. Marked test as flaky in https://trac.webkit.org/r228288. Thanks! --- /Volumes/Data/slave/sierra-release-tests-wk2/build/layout-test-results/fast/dom/adopt-node-crash-2-expected.txt +++ /Volumes/Data/slave/sierra-release-tests-wk2/build/layout-test-results/fast/dom/adopt-node-crash-2-actual.txt @@ -1,3 +1,2 @@ Tests for a crash due to adopting a DOM node during DOMFocusOut event. Test passes if it doesn't crash. - Created attachment 340165 [details]
Speculative fix
I can't reproduce this flakiness locally, and we haven't seen the flakiness since April but GTK+ bots are still occasionally seeing it. I think r227983 introducing a flakiness to this test sort of makes sense since it delays the layout update upon focus so I've crafted a speculative fix for it based on my observation. Comment on attachment 340165 [details] Speculative fix View in context: https://bugs.webkit.org/attachment.cgi?id=340165&action=review > LayoutTests/fast/dom/adopt-node-crash-2.html:39 > setTimeout("doit()", 1); The `1` is an interesting choice here. Does it not work if we schedule this on a 0-delay timer? Comment on attachment 340165 [details] Speculative fix View in context: https://bugs.webkit.org/attachment.cgi?id=340165&action=review >> LayoutTests/fast/dom/adopt-node-crash-2.html:39 >> setTimeout("doit()", 1); > > The `1` is an interesting choice here. Does it not work if we schedule this on a 0-delay timer? Indeed... I think it probably would but I couldn't figure out if this test would catch the bug after that change since reverting https://trac.webkit.org/changeset/135914 is kind of hard at this point. Committed r231690: <https://trac.webkit.org/changeset/231690> Oops, the test is still failing because I forgot to update the expected result & remove the flaky expectation. Committed r234485: <https://trac.webkit.org/changeset/234485> *** Bug 132183 has been marked as a duplicate of this bug. *** The test appears to be passing consistently now. |