Update focus test
<rdar://problem/27070007>
Created attachment 282309 [details] Patch
Comment on attachment 282309 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=282309&action=review Looks good to me. Unofficial r=me. > PerformanceTests/Animometer/tests/master/resources/focus.js:12 > +var movementDuration = 2500; Should not these be const? > PerformanceTests/Animometer/tests/master/resources/focus.js:27 > + this.particle = document.createElement('div'); Don't we need to append this.particle the childList of some parent? stage.element.appendChild(obj.particle); > PerformanceTests/Animometer/tests/master/resources/focus.js:38 > + this.animate(stage, 0, 0); Why do we need this call here? The FocusElement.animate() function will be called anyway from FocusStage.animate(). It is strange also we call this function before appending this.particle to any parent. > PerformanceTests/Animometer/tests/master/resources/focus.js:-97 > - Utilities.setElementPrefixedProperty(this._centerElement, "filter", "blur(" + blur + "px)"); Why did we remove the center element? Is this because we want more accurate results?
Comment on attachment 282309 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=282309&action=review >> PerformanceTests/Animometer/tests/master/resources/focus.js:12 >> +var movementDuration = 2500; > > Should not these be const? I avoided it to keep things simple. >> PerformanceTests/Animometer/tests/master/resources/focus.js:27 >> + this.particle = document.createElement('div'); > > Don't we need to append this.particle the childList of some parent? stage.element.appendChild(obj.particle); That's done in tune(). >> PerformanceTests/Animometer/tests/master/resources/focus.js:38 >> + this.animate(stage, 0, 0); > > Why do we need this call here? The FocusElement.animate() function will be called anyway from FocusStage.animate(). It is strange also we call this function before appending this.particle to any parent. The order is that we create the particle, add it, and then style it. It is possible that before we style it in animate() we paint after adding, in which case you get a flash of opaque circles. >> PerformanceTests/Animometer/tests/master/resources/focus.js:-97 >> - Utilities.setElementPrefixedProperty(this._centerElement, "filter", "blur(" + blur + "px)"); > > Why did we remove the center element? Is this because we want more accurate results? Remove some variability.
Comment on attachment 282309 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=282309&action=review > PerformanceTests/Animometer/tests/dom/resources/focus.js:16 > + // size and blurring are a function of depth Nit: We do comments with sentence case and periods. > PerformanceTests/Animometer/tests/dom/resources/focus.js:137 > + // update center element before loop Here too. >>> PerformanceTests/Animometer/tests/master/resources/focus.js:12 >>> +var movementDuration = 2500; >> >> Should not these be const? > > I avoided it to keep things simple. I'm not sure if all other browsers support const. >>> PerformanceTests/Animometer/tests/master/resources/focus.js:27 >>> + this.particle = document.createElement('div'); >> >> Don't we need to append this.particle the childList of some parent? stage.element.appendChild(obj.particle); > > That's done in tune(). Nit: Use " not '
Comment on attachment 282309 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=282309&action=review >> PerformanceTests/Animometer/tests/dom/resources/focus.js:16 >> + // size and blurring are a function of depth > > Nit: We do comments with sentence case and periods. Done. >> PerformanceTests/Animometer/tests/dom/resources/focus.js:137 >> + // update center element before loop > > Here too. Removed this instead. >>>> PerformanceTests/Animometer/tests/master/resources/focus.js:27 >>>> + this.particle = document.createElement('div'); >>> >>> Don't we need to append this.particle the childList of some parent? stage.element.appendChild(obj.particle); >> >> That's done in tune(). > > Nit: Use " not ' Done.
Committed r202601: <http://trac.webkit.org/changeset/202601>