| Summary: | accessibility/mac/aria-expanded-notifications.html is flaky | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Beth Dakin <bdakin> | ||||||||
| Component: | Accessibility | Assignee: | Myles C. Maxfield <mmaxfield> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | aboxhall, apinheiro, ap, cfleizach, commit-queue, dmazzoni, jcraig, jdiggs, mario, mmaxfield, samuel_white, webkit-bug-importer | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=149218 | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Beth Dakin
2015-09-23 13:29:27 PDT
Marked it flaky: http://trac.webkit.org/changeset/190183 Almost certainly caused by http://trac.webkit.org/changeset/189668 Can reproducing after running a few dozen times in a row with the attached test list. Created attachment 262106 [details]
Test List
Attaching the failure diff Created attachment 262109 [details]
Failure diff
(In reply to comment #4) > Almost certainly caused by http://trac.webkit.org/changeset/189668 Verified this. (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. Breaking in AccessibilityController::addNotificationListener() seems to make this reproduce 100%. Seems like a race. 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.
Created attachment 262236 [details]
Patch
> 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?
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. 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! 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 :( Committed r190417: <http://trac.webkit.org/changeset/190417> |