Bug 149510 – accessibility/mac/aria-expanded-notifications.html is flaky

• Component: Accessibility
• Status: RESOLVED FIXED

[Beth Dakin]

https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=accessibility%2Fmac%2Faria-expanded-notifications.html shows that this test has been flaky on the Mavericks and Yosemite bots.

[Radar WebKit Bug Importer]

<rdar://problem/22826005>

[Beth Dakin]

Marked it flaky: http://trac.webkit.org/changeset/190183

[Radar WebKit Bug Importer]

<rdar://problem/22826112>

[Myles C. Maxfield]

Almost certainly caused by http://trac.webkit.org/changeset/189668

[Myles C. Maxfield]

Can reproducing after running a few dozen times in a row with the attached test list.

[Myles C. Maxfield]

Created attachment 262106 [details]
Test List

[Myles C. Maxfield]

Attaching the failure diff

[Myles C. Maxfield]

Created attachment 262109 [details]
Failure diff

[Myles C. Maxfield]

(In reply to comment #4)
> Almost certainly caused by http://trac.webkit.org/changeset/189668

Verified this.

[Myles C. Maxfield]

(In reply to comment #9)
> (In reply to comment #4)
> > Almost certainly caused by http://trac.webkit.org/changeset/189668
> 
> Verified this.

By "verified," I mean that, when the patch is reverted, I can no longer repro the issue.

[Myles C. Maxfield]

Breaking in AccessibilityController::addNotificationListener() seems to make this reproduce 100%. Seems like a race.

[Myles C. Maxfield]

The problem is triggered by our resumable parser.

            if (elapsedTime > m_parserTimeLimit)
                session.needsYield = true;

AXLoadComplete gets called synchronously from DocumentLoader::finishedLoading(). This means that the order of the AXLoadComplete message isn't FIFO:

If the parser yielded previously, the AXLoadComplete will be handled after some AX messages had been handled. If the parser hadn't yielded, the AXLoadComplete will be the first message processed (no matter what had been queued up before it).

I assume that making AXLoadComplete asynchronous is not possible. Therefore, it seems the test should be relaxed to handle the case of various orderings.

[Myles C. Maxfield]

Created attachment 262236 [details]
Patch

[Alexey Proskuryakov]

> I assume that making AXLoadComplete asynchronous is not possible.

This actually sounds like it should be good. Loading is asynchronous in nature, so queueing the notification should not make an observable difference other than in this edge case.

Chris, does that make sense?

[Alexey Proskuryakov]

Comment on attachment 262236 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=262236&action=review

> LayoutTests/accessibility/mac/aria-expanded-notifications-expected.txt:12
>  TEST COMPLETE
> -Notification: AXLoadComplete
>  Notification: AXRowCountChanged

This should be fixed by properly using js-test functionality - TEST COMPLETE is supposed to be last.

> LayoutTests/platform/mac/TestExpectations:-1334
> -webkit.org/b/149510 accessibility/mac/aria-expanded-notifications.html [ Pass Failure ]

It seems like we would need a code change anyway for the other test that asserts.

[chris fleizach]

Yea this sounds good. The test is not dependent on the order of these notifications
If we didn't get to the bottom of this I was going to suggest filtering out the load notification 

Thanks!

[Myles C. Maxfield]

Comment on attachment 262236 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=262236&action=review

>> LayoutTests/platform/mac/TestExpectations:-1334
>> -webkit.org/b/149510 accessibility/mac/aria-expanded-notifications.html [ Pass Failure ]
> 
> It seems like we would need a code change anyway for the other test that asserts.

Unfortunately, it seems that other test is a separate issue :(

[Myles C. Maxfield]

Committed r190417: <http://trac.webkit.org/changeset/190417>