Bug 31376 - WebInspector: Adds test for Timeline EventDispatch record
Summary: WebInspector: Adds test for Timeline EventDispatch record
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-11-11 14:17 PST by Eric Ayers
Modified: 2009-12-02 11:11 PST (History)
7 users (show)

See Also:


Attachments
WebInspector: Adds EventDispatch test for Timeline records. (3.75 KB, patch)
2009-11-11 14:30 PST, Eric Ayers
pfeldman: review-
Details | Formatted Diff | Diff
WebInspector: Adds test for DOM dispatch timeline record (2.53 KB, patch)
2009-11-30 07:18 PST, Eric Ayers
pfeldman: review+
Details | Formatted Diff | Diff
WebInspector:Adds test for Timeline EventDispatch record (Alternative to patch #2) (2.53 KB, patch)
2009-12-01 10:15 PST, Eric Ayers
no flags Details | Formatted Diff | Diff
Web Inspector: Adds a test for event dispatch timeline records (2.43 KB, patch)
2009-12-02 09:40 PST, Eric Ayers
no flags Details | Formatted Diff | Diff
Web Inspector: Adds a test for event dispatch timeline records (2.43 KB, patch)
2009-12-02 09:44 PST, Eric Ayers
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Ayers 2009-11-11 14:17:54 PST
Simulates a mousedown event and dumps the corresponding EventDispatch record.
Comment 1 Eric Ayers 2009-11-11 14:30:39 PST
Created attachment 43004 [details]
WebInspector: Adds EventDispatch test for Timeline records.
Comment 2 Pavel Feldman 2009-11-11 23:55:02 PST
Comment on attachment 43004 [details]
WebInspector: Adds EventDispatch test for Timeline records.


> +function findMarkTimeline(record) 
> +{
> +    if (record.type === timelineAgentRecordType.MarkTimeline && record.data.message === timelineMark)
> +        return true;
> +    var numChildren = record.children ? record.children.length : 0;
> +    for (var i = 0; i < numChildren; ++i)
> +        if (findMarkTimeline(record.children[i]))
> +            return true;
> +    return false;
> +}
> +
> +function findEventDispatch(record, dispatchType)
> +{
> +    if (record.type === timelineAgentRecordType.EventDispatch && record.data.type === "mousedown")
> +        if (findMarkTimeline(record)) {
> +            printTimelineRecordProperties(record);
> +            return true;
> +        }
> +
> +    var numChildren = record.children ? record.children.length : 0;
> +    for (var i = 0; i < numChildren; ++i) {
> +        var child = record.children[i];
> +        if (findEventDispatch(child, dispatchType))
> +            return true;
> +    }
> +    return false;
> +}
> +

See my comments to the previous patch. The ones above are again code duplication.

> +
> +function analyzeTimelineData(timelineRecords)
> +{
> +    // Uncomment to debugging the list of data returned.
> +    // dumpTimelineRecords(timelineRecords);
> +
> +    var found = false;
> +    var numRecords = timelineRecords.length;
> +
> +    // Look for a mousedown EventDispatch that follows the mark
> +    for (var i = 0; i < numRecords; ++i) {
> +        var record = timelineRecords[i];
> +        found = findEventDispatch(record, "mousedown");
> +        if (found)
> +            break;
> +    }
> +    if (!found)
> +        output("Did not find EventDispatch for mousedown");
> +}

And again.
Comment 3 Eric Ayers 2009-11-30 07:18:41 PST
Created attachment 44020 [details]
WebInspector: Adds test for DOM dispatch timeline record

I'm not sure if the repeated test for the event firing with the timer is necessary, but I thought it best to avoid flakiness.
Comment 4 Adam Barth 2009-11-30 12:50:55 PST
style-queue ran check-webkit-style on attachment 44020 [details] without any errors.
Comment 5 Pavel Feldman 2009-12-01 08:00:53 PST
Comment on attachment 44020 [details]
WebInspector: Adds test for DOM dispatch timeline record

> +    function step() 
> +    {
> +        if (window.eventHandled) {
> +            printTimelineRecords(null, "EventDispatch");
> +        } else {
> +            setTimeout(step, 100);
> +        }
> +    }
> +    setTimeout(step, 100);

Can we avoid timeout here?
Comment 6 Eric Ayers 2009-12-01 08:20:36 PST
(In reply to comment #5)
> (From update of attachment 44020 [details])
> > +    function step() 
> > +    {
> > +        if (window.eventHandled) {
> > +            printTimelineRecords(null, "EventDispatch");
> > +        } else {
> > +            setTimeout(step, 100);
> > +        }
> > +    }
> > +    setTimeout(step, 100);
> 
> Can we avoid timeout here?

I didn't peek under the covers to see how simulating events was implemented (do they get fired immediately or put on the event queue?)

I could certainly set the timeout down to a smaller number ('0' ?).   The worst that could happen is that it would have to try again and end up executing the way I have it written now.
Comment 7 Eric Ayers 2009-12-01 10:15:26 PST
Created attachment 44086 [details]
WebInspector:Adds test for Timeline EventDispatch record (Alternative to patch #2)

alternative to patch #2 where the timeout for step() is set to 0 for the first attempt to get timeline data.
Comment 8 WebKit Review Bot 2009-12-01 10:19:43 PST
style-queue ran check-webkit-style on attachment 44086 [details] without any errors.
Comment 9 Eric Ayers 2009-12-02 09:40:34 PST
Created attachment 44157 [details]
Web Inspector: Adds a test for event dispatch timeline records
Comment 10 Eric Ayers 2009-12-02 09:44:51 PST
Created attachment 44160 [details]
Web Inspector: Adds a test for event dispatch timeline records

(last patch accidentally left the timeout set at 100ms.)
Comment 11 WebKit Commit Bot 2009-12-02 11:10:55 PST
Comment on attachment 44160 [details]
Web Inspector: Adds a test for event dispatch timeline records

Clearing flags on attachment: 44160

Committed r51604: <http://trac.webkit.org/changeset/51604>
Comment 12 WebKit Commit Bot 2009-12-02 11:11:00 PST
All reviewed patches have been landed.  Closing bug.