<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - Add PerfAutoRun to automate browser based performance benchmarks(e.g. Speedometer and JetStream)"
href="https://bugs.webkit.org/show_bug.cgi?id=144038#c3">Comment # 3</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - Add PerfAutoRun to automate browser based performance benchmarks(e.g. Speedometer and JetStream)"
href="https://bugs.webkit.org/show_bug.cgi?id=144038">bug 144038</a>
from <span class="vcard"><a class="email" href="mailto:cdumez@apple.com" title="Chris Dumez <cdumez@apple.com>"> <span class="fn">Chris Dumez</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=251306&action=diff" name="attach_251306" title="Patch">attachment 251306</a> <a href="attachment.cgi?id=251306&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=251306&action=review">https://bugs.webkit.org/attachment.cgi?id=251306&action=review</a>
Ryosuke is the right person to review this, just making a few initial comments.
<span class="quote">> Tools/Scripts/PerfAutoRun/Patches/JetStream.patch:12
> ++ for (var i = 0; i < benchmarks.length; ++ i) {</span >
no space between ++ and i
<span class="quote">> Tools/Scripts/PerfAutoRun/Patches/JetStream.patch:21
> ++ </span >
Not needed.
<span class="quote">> Tools/Scripts/PerfAutoRun/Patches/JetStream.patch:28
> ++ //submit result to server</span >
Missing space after //. Missing capital letter for Submit and missing '.' at the end of the sentence.
<span class="quote">> Tools/Scripts/PerfAutoRun/Patches/JetStream.patch:30
> ++ var http = new XMLHttpRequest();</span >
We usually use xhr variable name for these rather than http I believe.
<span class="quote">> Tools/Scripts/PerfAutoRun/Patches/JetStream.patch:31
> ++ var url = '/report';</span >
No need for this variable.
<span class="quote">> Tools/Scripts/PerfAutoRun/Patches/JetStream.patch:32
> ++ http.open("POST", url, true);</span >
true can be omitted as async is the default.
<span class="quote">> Tools/Scripts/PerfAutoRun/Patches/JetStream.patch:39
> ++ if(http.readyState == 4 && http.status == 200) {</span >
xhr.readyState == XMLHttpRequest.DONE
<span class="quote">> Tools/Scripts/PerfAutoRun/Patches/JetStream.patch:40
> ++ closeRequest = new XMLHttpRequest();</span >
missing var
<span class="quote">> Tools/Scripts/PerfAutoRun/Patches/JetStream.patch:41
> ++ closeRequest.open("GET", '/shutdown', true);</span >
true can be omitted as async is the default.
<span class="quote">> Tools/Scripts/PerfAutoRun/Patches/JetStream.patch:56
> ++ Copyright (C) 2014 Apple Inc. All rights reserved.</span >
2015?
<span class="quote">> Tools/Scripts/PerfAutoRun/Patches/JetStream.patch:95
> ++ else</span >
else should be on previous line.
<span class="quote">> 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></span >
s/Bigger/Higher ?
<span class="quote">> Tools/Scripts/PerfAutoRun/Utilities/HTTPHandle/SimpleHTTPHandle.py:59
> + 'Http Server is serving at port: %d',</span >
HTTP
<span class="quote">> Tools/Scripts/PerfAutoRun/Utilities/PlatformHandle/OSXSafariHandle.py:60
> + '/Users/Leelour/Code/Webkit/OpenSource/WebKitBuild/Release/',</span >
?</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>