Bug 95546 - [DRT] Make simulating a web click on a notification a queued task
Summary: [DRT] Make simulating a web click on a notification a queued task
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.8
: P2 Normal
Assignee: Jon Lee
URL:
Keywords: InRadar
Depends on:
Blocks: 77969
  Show dependency treegraph
 
Reported: 2012-08-31 02:54 PDT by Jon Lee
Modified: 2012-08-31 11:55 PDT (History)
3 users (show)

See Also:


Attachments
Patch (4.07 KB, patch)
2012-08-31 10:29 PDT, Jon Lee
ap: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jon Lee 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.
Comment 1 Radar WebKit Bug Importer 2012-08-31 02:55:24 PDT
<rdar://problem/12214170>
Comment 2 Jon Lee 2012-08-31 10:29:51 PDT
Created attachment 161730 [details]
Patch
Comment 3 Alexey Proskuryakov 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.
Comment 4 Alexey Proskuryakov 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
Comment 5 Jon Lee 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().
Comment 6 Jon Lee 2012-08-31 11:55:23 PDT
Committed r127298: <http://trac.webkit.org/changeset/127298>