[Webkit-unassigned] [Bug 144038] Add PerfAutoRun to automate browser based performance benchmarks(e.g. Speedometer and JetStream)

[bugzilla-daemon at webkit.org]

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

--- Comment #3 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 251306
  --> https://bugs.webkit.org/attachment.cgi?id=251306
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=251306&action=review

Ryosuke is the right person to review this, just making a few initial comments.

> Tools/Scripts/PerfAutoRun/Patches/JetStream.patch:12
> ++        for (var i = 0; i < benchmarks.length; ++ i) {

no space between ++ and i

> Tools/Scripts/PerfAutoRun/Patches/JetStream.patch:21
> ++        

Not needed.

> Tools/Scripts/PerfAutoRun/Patches/JetStream.patch:28
> ++        //submit result to server

Missing space after //. Missing capital letter for Submit and missing '.' at the end of the sentence.

> Tools/Scripts/PerfAutoRun/Patches/JetStream.patch:30
> ++        var http = new XMLHttpRequest();

We usually use xhr variable name for these rather than http I believe.

> Tools/Scripts/PerfAutoRun/Patches/JetStream.patch:31
> ++        var url = '/report';

No need for this variable.

> Tools/Scripts/PerfAutoRun/Patches/JetStream.patch:32
> ++        http.open("POST", url, true);

true can be omitted as async is the default.

> Tools/Scripts/PerfAutoRun/Patches/JetStream.patch:39
> ++        if(http.readyState == 4 && http.status == 200) {

xhr.readyState == XMLHttpRequest.DONE

> Tools/Scripts/PerfAutoRun/Patches/JetStream.patch:40
> ++                closeRequest = new XMLHttpRequest();

missing var

> Tools/Scripts/PerfAutoRun/Patches/JetStream.patch:41
> ++                closeRequest.open("GET", '/shutdown', true);

true can be omitted as async is the default.

> Tools/Scripts/PerfAutoRun/Patches/JetStream.patch:56
> ++ Copyright (C) 2014 Apple Inc. All rights reserved.

2015?

> Tools/Scripts/PerfAutoRun/Patches/JetStream.patch:95
> ++        else

else should be on previous line.

> Tools/Scripts/PerfAutoRun/Patches/JetStream.patch:109
> ++    <p class="summary">JetStream is a JavaScript benchmark suite focused on the most advanced web applications. For more information, read the <a href="in-depth.html">in-depth analysis</a>. Bigger scores are better.</p>

s/Bigger/Higher ?

> Tools/Scripts/PerfAutoRun/Utilities/HTTPHandle/SimpleHTTPHandle.py:59
> +                            'Http Server is serving at port: %d',

HTTP

> Tools/Scripts/PerfAutoRun/Utilities/PlatformHandle/OSXSafariHandle.py:60
> +        '/Users/Leelour/Code/Webkit/OpenSource/WebKitBuild/Release/',

?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150422/e2c24f45/attachment.html>