[Webkit-unassigned] [Bug 31372] WebInspector: Adds Timer record tests for Timeline agent records

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=31372


Pavel Feldman <pfeldman at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #43001|review?                     |review-
               Flag|                            |




--- Comment #2 from Pavel Feldman <pfeldman at chromium.org>  2009-11-11 23:53:15 PST ---
(From update of attachment 43001)
> +// Look for the TimerFire event that correlates to the 
> +// to the <script> tag below.

to the is used twice


> +   function analyzeTimerFire(record) 

I think I've seen this pattern before. Can we extract something receiving
timelineMark and a printcallback or something?

> +        var numChildren = record.children ? record.children.length : 0;
> +        // Look for a MarkTimeline as a direct child.
> +        for (var i = 0; i < numChildren; ++i) {
> +            var child = record.children[i];
> +            if (child.type === timelineAgentRecordType.MarkTimeline && child.data.message === timelineMark) {
> +                return record;
> +            }
> +        }

This snippet should really be reused.


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

ditto


> +    // Look for the install that corresponds to the timer fire.
> +    for (var i = 0 ; i < numRecords; ++i) {
> +        var record = timelineRecords[i];
> +        found = findTimerRecord(timelineAgentRecordType.TimerInstall, record, timerId);
> +        if (found)
> +            break;

This pattern is also happening all the time.

JavaScript is a dynamic language allowing you to pass functions as parameters,
etc. 
- Implementing forEachRecord would do some code reuse for you.
- findRecord(type, goldenData) where golden is a set of properties that should
be set in data would also cover lots of stuff.
like the snipet above would convert into
findRecord(timelineAgentRecordType.MarkTimeline, { message : timelineMark }); 
or printTimelineRecordProperties(timelineAgentRecordType.MarkTimeline, {
message : timelineMark }) to reuse even more.
- the approach above would convert findTimerInstall into
findRecort(timelineAgentRecordType.TimerInstall, { timerId : timerId }); and
should work for eval as well.

In other words, please extract some methods. As you create more tests we get
more and more code duplication.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.