| Summary: | [DRT] Make simulating a web click on a notification a queued task | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Jon Lee <jonlee> | ||||
| Component: | Tools / Tests | Assignee: | Jon Lee <jonlee> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | ap, jberlin, webkit-bug-importer | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Mac | ||||||
| OS: | OS X 10.8 | ||||||
| Bug Depends on: | |||||||
| Bug Blocks: | 77969 | ||||||
| Attachments: |
|
||||||
|
Description
Jon Lee
2012-08-31 02:54:52 PDT
Created attachment 161730 [details]
Patch
Comment on attachment 161730 [details]
Patch
r=me
I suggest adding an ASSERT(!m_hasPendingWebNotificationClick) when DRT is dumping test results - the m_hasPendingWebNotificationClick check in dispatched block is insufficient for safety. If one test calls simulateWebNotificationClick, finishes, and then another one does the same, we'll end up dispatching original notificationID, breaking the second test.
To be clear, r+ is conditional on fixing this. A test that leaves an outstanding notification clearly has a bug, and an assertion would go a longer way towards preventing flakiness than silently passing the error condition to the next test. The dispatched block is going to be executed anyway during the next test, and there is no guarantee that m_hasPendingWebNotificationClick will still be false by that time Comment on attachment 161730 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=161730&action=review > Tools/DumpRenderTree/mac/TestRunnerMac.mm:331 > + m_hasPendingWebNotificationClick = false; Per ap's comments, removing this reset, and instead add an assert in dump(). Committed r127298: <http://trac.webkit.org/changeset/127298> |