Bug 31376

Summary: WebInspector: Adds test for Timeline EventDispatch record
Product: WebKit Reporter: Eric Ayers <zundel>
Component: Web Inspector (Deprecated)Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, commit-queue, knorton, pfeldman, timothy, webkit.review.bot, zundel
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
WebInspector: Adds EventDispatch test for Timeline records.
pfeldman: review-
WebInspector: Adds test for DOM dispatch timeline record
pfeldman: review+
WebInspector:Adds test for Timeline EventDispatch record (Alternative to patch #2)
none
Web Inspector: Adds a test for event dispatch timeline records
none
Web Inspector: Adds a test for event dispatch timeline records none

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.