Bug 95546

Summary: [DRT] Make simulating a web click on a notification a queued task
Product: WebKit Reporter: Jon Lee <jonlee>
Component: Tools / TestsAssignee: 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 Flags
Patch ap: review+

Jon Lee
Reported 2012-08-31 02:54:52 PDT
When simulating a user click on a web notification, make sure that the click is treated like a 0-timer task, rather than clicking synchronously. It mimics reality more accurately.
Attachments
Patch (4.07 KB, patch)
2012-08-31 10:29 PDT, Jon Lee
ap: review+
Radar WebKit Bug Importer
Comment 1 2012-08-31 02:55:24 PDT
Jon Lee
Comment 2 2012-08-31 10:29:51 PDT
Alexey Proskuryakov
Comment 3 2012-08-31 10:53:02 PDT
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.
Alexey Proskuryakov
Comment 4 2012-08-31 11:08:54 PDT
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
Jon Lee
Comment 5 2012-08-31 11:51:26 PDT
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().
Jon Lee
Comment 6 2012-08-31 11:55:23 PDT
Note You need to log in before you can comment on or make changes to this bug.