REGRESSION(r227983): fast/dom/adopt-node-crash-2.html is flaky

[Ryosuke Niwa]

Ryan Haddad 2018-02-07 16:57:14 PST
(In reply to Ryosuke Niwa from comment #20)
> Committed r227983: <https://trac.webkit.org/changeset/227983>

This change has caused LayoutTest fast/dom/adopt-node-crash-2.html to become a flaky failure on High Sierra Release WK2 and Sierra Release WK2:
https://build.webkit.org/results/Apple%20Sierra%20Release%20WK2%20(Tests)/r228225%20(7494)/results.html

I am able to reproduce the failure with a build of r227984 with the following:
run-webkit-tests fast/dom/adopt-node-crash-2.html -fg --iter 100 --exit-after-n-failures 1

I cannot reproduce with a build of r227982.

[Ryan Haddad]

Ryosuke, is this something you intend to fix shortly or do you want to mark the test as failing?

[Ryosuke Niwa]

(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.

[Ryan Haddad]

(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.

[Ryosuke Niwa]

Thanks!

[Radar WebKit Bug Importer]

<rdar://problem/37517904>

[Alexey Proskuryakov]

--- /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.
 
-

[Ryosuke Niwa]

Created attachment 340165 [details]
Speculative fix

[Ryosuke Niwa]

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.

[Wenson Hsieh]

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?

[Ryosuke Niwa]

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.

[Ryosuke Niwa]

Committed r231690: <https://trac.webkit.org/changeset/231690>

[Ryosuke Niwa]

Oops, the test is still failing because I forgot to update the expected result & remove the flaky expectation.

[Ryosuke Niwa]

Committed r234485: <https://trac.webkit.org/changeset/234485>

[Ryosuke Niwa]

*** Bug 132183 has been marked as a duplicate of this bug. ***

[Ryosuke Niwa]

The test appears to be passing consistently now.