Update tests.
<rdar://problem/24034507>
Created attachment 268232 [details] 1. Fix tests for other browsers.
Created attachment 268233 [details] 2. Move algorithm.js and sampler.js to tests/ and benchmark-runner.js to runner/.
Created attachment 268234 [details] 3. Add a new test.
Comment on attachment 268233 [details] 2. Move algorithm.js and sampler.js to tests/ and benchmark-runner.js to runner/. View in context: https://bugs.webkit.org/attachment.cgi?id=268233&action=review > PerformanceTests/ChangeLog:16 > + into utilities.js. Did you mean "into extensions.js"?
Comment on attachment 268234 [details] 3. Add a new test. View in context: https://bugs.webkit.org/attachment.cgi?id=268234&action=review > PerformanceTests/Animometer/resources/extensions.js:96 > + return new Point(this.x + other, this.y + other); I am not sure if adding a scalar value to a point/size is something meaningful.
All looks good to me. Unofficial r=me.
Comment on attachment 268233 [details] 2. Move algorithm.js and sampler.js to tests/ and benchmark-runner.js to runner/. View in context: https://bugs.webkit.org/attachment.cgi?id=268233&action=review >> PerformanceTests/ChangeLog:16 >> + into utilities.js. > > Did you mean "into extensions.js"? Yes
(In reply to comment #4) > Created attachment 268234 [details] > 3. Add a new test. I will check in a version with the corrected ChangeLog.
Created attachment 268524 [details] 3. Add new test (Take 2)
Created attachment 268526 [details] 1. Fix tests for other browsers (Take 2)
Created attachment 268527 [details] 2. Move algorithm.js and sampler.js (Take 2)
Comment on attachment 268526 [details] 1. Fix tests for other browsers (Take 2) View in context: https://bugs.webkit.org/attachment.cgi?id=268526&action=review > PerformanceTests/ChangeLog:14 > + (CanvasElectron.prototype._draw): Some browsers don't support ellipse. Which ones?
Comment on attachment 268526 [details] 1. Fix tests for other browsers (Take 2) View in context: https://bugs.webkit.org/attachment.cgi?id=268526&action=review > PerformanceTests/Animometer/resources/extensions.js:70 > + var rect = element.getBoundingClientRect(); > + return new Point(rect.width, rect.height); Are you sure this doesn't cause layout thrashing?
Comment on attachment 268524 [details] 3. Add new test (Take 2) View in context: https://bugs.webkit.org/attachment.cgi?id=268524&action=review > PerformanceTests/Animometer/resources/extensions.js:96 > + if(isNaN(other.x)) > + return new Point(this.x + other, this.y + other); Weird but ok! > PerformanceTests/Animometer/tests/master/resources/dom-particles.js:25 > + this.element.style.backgroundColor = "hsl(" + (((Date.now()/2000)%1)*360) + ", 70%, 45%)"; Are the outer parens around Date.now()... necessary?
(In reply to comment #13) > Comment on attachment 268526 [details] > 1. Fix tests for other browsers (Take 2) > > View in context: > https://bugs.webkit.org/attachment.cgi?id=268526&action=review > > > PerformanceTests/ChangeLog:14 > > + (CanvasElectron.prototype._draw): Some browsers don't support ellipse. > > Which ones? Firefox.
(In reply to comment #14) > Comment on attachment 268526 [details] > 1. Fix tests for other browsers (Take 2) > > View in context: > https://bugs.webkit.org/attachment.cgi?id=268526&action=review > > > PerformanceTests/Animometer/resources/extensions.js:70 > > + var rect = element.getBoundingClientRect(); > > + return new Point(rect.width, rect.height); > > Are you sure this doesn't cause layout thrashing? This function is only called when the stage is initialized, and for graphing. It is not called during the benchmark run. I wouldn't expect any benchmark to make a call to this because it should be caching its size.
(In reply to comment #15) > Comment on attachment 268524 [details] > 3. Add new test (Take 2) > > View in context: > https://bugs.webkit.org/attachment.cgi?id=268524&action=review > > > PerformanceTests/Animometer/resources/extensions.js:96 > > + if(isNaN(other.x)) > > + return new Point(this.x + other, this.y + other); > > Weird but ok! > > > PerformanceTests/Animometer/tests/master/resources/dom-particles.js:25 > > + this.element.style.backgroundColor = "hsl(" + (((Date.now()/2000)%1)*360) + ", 70%, 45%)"; > > Are the outer parens around Date.now()... necessary? No. This calculation is done enough that I'll probably factor it out anyway in a future patch.
Comment on attachment 268526 [details] 1. Fix tests for other browsers (Take 2) Clearing flags on attachment: 268526 Committed r194752: <http://trac.webkit.org/changeset/194752>
Committed r194753: <http://trac.webkit.org/changeset/194753>
(In reply to comment #20) > Committed r194753: <http://trac.webkit.org/changeset/194753> This is patch #2.
Committed r194755: <http://trac.webkit.org/changeset/194755>
fix in r194756