[CSS Regions] Add performance test for selection

[CSS Regions] Add performance test for selection

Patch already landed in WebKit. See http://trac.webkit.org/changeset/159488

Add new performance test for selection in CSS Regions. It checks a selection
from the first region to the last one, passing through all the regions.

Test is skipped for now while implementation of selection in CSS Regions is
still evolving.

BUG=319305
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=162418

[Julien - ping for review, 2013-11-20 05:31:54]

The test looks fine but why land all these tests disabled? How are they helping
to catch regressions while the region code evolves?

https://codereview.chromium.org/76293002/diff/1/PerformanceTests/Layout/resou...
File PerformanceTests/Layout/resources/regions.js (right):

https://codereview.chromium.org/76293002/diff/1/PerformanceTests/Layout/resou...
PerformanceTests/Layout/resources/regions.js:110: function
createRegionsSelectionTest(regionCount) {
Why don't you directly do the following?

window.createRegionsSelectionTest = function(regionCount) {

[Manuel Rego, 2013-11-20 15:23:39]

On 2013/11/20 05:31:54, Julien Chaffraix - AEDT wrote:
> The test looks fine but why land all these tests disabled? How are they
helping
> to catch regressions while the region code evolves?

Regions implementation is not complete and regions performance is an ongoing
development. So having the perftests is usseful as reference to check results
when somone provide a patch improving performance, however performance
regressions might be needed to implement some features. At some point in the
future we'll enable those tests by default.

>
https://codereview.chromium.org/76293002/diff/1/PerformanceTests/Layout/resou...
> File PerformanceTests/Layout/resources/regions.js (right):
> 
>
https://codereview.chromium.org/76293002/diff/1/PerformanceTests/Layout/resou...
> PerformanceTests/Layout/resources/regions.js:110: function
> createRegionsSelectionTest(regionCount) {
> Why don't you directly do the following?
> 
> window.createRegionsSelectionTest = function(regionCount) {

Yeah, that would work too. I was just following the style of the file as it's
already doing something very similar in:
    window.createRegionsTest = createRegionsTest;

Anyway, I can change it if you think it's needed.

[Julien - ping for review, 2013-11-20 21:08:26]

lgtm

On 2013/11/20 15:23:39, Manuel Rego wrote:
> On 2013/11/20 05:31:54, Julien Chaffraix - AEDT wrote:
> > The test looks fine but why land all these tests disabled? How are they
> helping
> > to catch regressions while the region code evolves?
> 
> Regions implementation is not complete and regions performance is an ongoing
> development. So having the perftests is usseful as reference to check results
> when somone provide a patch improving performance, however performance
> regressions might be needed to implement some features. At some point in the
> future we'll enable those tests by default.

AFAICT the region code is fairly stable so there would be some values in having
these enabled to avoid regressions as a by-product of the ongoing work / other
changes. This is all the more true if you are in the performance tuning phase :)

>
https://codereview.chromium.org/76293002/diff/1/PerformanceTests/Layout/resou...
> > File PerformanceTests/Layout/resources/regions.js (right):
> > 
> >
>
https://codereview.chromium.org/76293002/diff/1/PerformanceTests/Layout/resou...
> > PerformanceTests/Layout/resources/regions.js:110: function
> > createRegionsSelectionTest(regionCount) {
> > Why don't you directly do the following?
> > 
> > window.createRegionsSelectionTest = function(regionCount) {
> 
> Yeah, that would work too. I was just following the style of the file as it's
> already doing something very similar in:
>     window.createRegionsTest = createRegionsTest;
> 
> Anyway, I can change it if you think it's needed.

I don't feel strongly about this.

[Manuel Rego, 2013-11-20 22:24:34]

On 2013/11/20 21:08:26, Julien Chaffraix - AEDT wrote:
> On 2013/11/20 15:23:39, Manuel Rego wrote:
> > On 2013/11/20 05:31:54, Julien Chaffraix - AEDT wrote:
> > > The test looks fine but why land all these tests disabled? How are they
> > helping
> > > to catch regressions while the region code evolves?
> > 
> > Regions implementation is not complete and regions performance is an ongoing
> > development. So having the perftests is usseful as reference to check
results
> > when somone provide a patch improving performance, however performance
> > regressions might be needed to implement some features. At some point in the
> > future we'll enable those tests by default.
> 
> AFAICT the region code is fairly stable so there would be some values in
having
> these enabled to avoid regressions as a by-product of the ongoing work / other
> changes. This is all the more true if you are in the performance tuning phase
:)

I'll talk with the rest of the people developing CSS Regions implementation and
I hope we'll be able soon to provide a patch to enable all those perftests.
Thanks for the feedback.

[commit-bot: I haz the power, 2013-11-20 22:25:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rego@igalia.com/76293002/1

[commit-bot: I haz the power, 2013-11-20 23:52:17]

Change committed as 162418