Update speedometer patch to provide better UI and fix merge result bug.
Created attachment 252132 [details] Patch
Comment on attachment 252132 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=252132&action=review > Tools/Scripts/webkitpy/benchmark_runner/data/patches/Speedometer.patch:80 > ++ var enabledSuites = Suites.filter(function (suite) { return !suite.disabled }); > ++ var values = timeValues.map(function (time) { return 60 * 1000 * enabledSuites.length / time}); Alternatively, we can just use measuredValues.length. That's probably better here. > Tools/Scripts/webkitpy/benchmark_runner/data/patches/Speedometer.patch:104 > ++ console.log(results); Superflous logging. > Tools/Scripts/webkitpy/benchmark_runner/data/patches/Speedometer.patch:116 > -+ > ++ What's up with this change? > Tools/Scripts/webkitpy/benchmark_runner/data/patches/Speedometer.patch:120 > -+ > ++ Ditto.
Created attachment 252136 [details] Patch
Comment on attachment 252132 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=252132&action=review >> Tools/Scripts/webkitpy/benchmark_runner/data/patches/Speedometer.patch:116 >> ++ > > What's up with this change? Delete unnecessary space for blank line.
Comment on attachment 252136 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=252136&action=review > Tools/Scripts/webkitpy/benchmark_runner/data/patches/Speedometer.patch:80 > ++ var values = timeValues.map(function (time) { return 60 * 1000 * enabledSuites.length / time}); I really don't think we should be duplicating the logic to compute the score in this file. Please modify Performance/Speedometer/resources/main.js to expose this function as a global function and use it here.
Created attachment 252140 [details] Patch
Created attachment 252142 [details] Patch
Comment on attachment 252142 [details] Patch Clearing flags on attachment: 252142 Committed r183685: <http://trac.webkit.org/changeset/183685>
All reviewed patches have been landed. Closing bug.
Comment on attachment 252142 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=252142&action=review > PerformanceTests/Speedometer/resources/main.js:55 > + return computerScore(time); this says computerScore > PerformanceTests/Speedometer/resources/main.js:161 > +function computeScore(time) { this says computeScore Since they don’t match, does this work? How did we test?
Comment on attachment 252142 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=252142&action=review >> PerformanceTests/Speedometer/resources/main.js:161 >> +function computeScore(time) { > > this says computeScore > > Since they don’t match, does this work? How did we test? Sorry for the typo. I should have tested for both automatic test as well as manual test. Since automatic test will override the benchmarkClient, this error is not trigger. Sorry again. I will fix it immediately.
Reopening to attach new patch.
Created attachment 252173 [details] Patch
Comment on attachment 252173 [details] Patch This patch can't be applied because the previous patch has already been landed. Please rebase your patch against ToT.