Bug 126427 – [CSS Regions] Add performance tests for selection with mixed content

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Manuel Rego Casasnovas]

As agreed with Mihnea it would be great to have new performance tests for selection in CSS Regions but mixing content inside and outside regions. On top of that, it seems a good idea to have a test for "select all" command too.

[Manuel Rego Casasnovas]

Created attachment 220295 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 220295 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=220295&action=review

> PerformanceTests/Layout/RegionsSelectAllMixedContent.html:4
> +        <link rel="stylesheet" href="resources/regions.css" TYPE="text/css"></link>

Why is test capitalized?

> PerformanceTests/Layout/RegionsSelectAllMixedContent.html:16
> +            PerfTestRunner.measureTime(createRegionsSelectAllMixedContentTest(1000));

Why can't I see the actual test here?  It's really annoying that the test code is written elsewhere.

> PerformanceTests/Layout/RegionsSelectionMixedContent.html:4
> +        <link rel="stylesheet" href="resources/regions.css" TYPE="text/css"></link>

Ditto.

> PerformanceTests/Layout/RegionsSelectionMixedContent.html:16
> +            PerfTestRunner.measureTime(createRegionsSelectionMixedContentTest(100));

Ditto.

> PerformanceTests/Layout/resources/regions.js:165
> +    function createRegionsSelectionMixedContentTest(regionCount) {

This should be in .html fie so that we can see the test code in the single file.
Also, this function and the test should be named something like "ExtendingSelection".
"Selection" doesn't convey enough semantics.

> PerformanceTests/Layout/resources/regions.js:188
> +    function createRegionsSelectAllMixedContentTest(regionCount) {

Ditto about moving this into the html file.

> PerformanceTests/Layout/resources/regions.js:212
>      window.createRegionsTest = createRegionsTest;
>      window.createRegionsSelectionTest = createRegionsSelectionTest;

These other test functions should really be in the respective html files.

[Manuel Rego Casasnovas]

(In reply to comment #2)
> (From update of attachment 220295 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=220295&action=review
> 
> > PerformanceTests/Layout/RegionsSelectAllMixedContent.html:4
> > +        <link rel="stylesheet" href="resources/regions.css" TYPE="text/css"></link>
> 
> Why is test capitalized?
> 
> > PerformanceTests/Layout/RegionsSelectAllMixedContent.html:16
> > +            PerfTestRunner.measureTime(createRegionsSelectAllMixedContentTest(1000));
> 
> Why can't I see the actual test here?  It's really annoying that the test code is written elsewhere.
> 
> > PerformanceTests/Layout/RegionsSelectionMixedContent.html:4
> > +        <link rel="stylesheet" href="resources/regions.css" TYPE="text/css"></link>
> 
> Ditto.
> 
> > PerformanceTests/Layout/RegionsSelectionMixedContent.html:16
> > +            PerfTestRunner.measureTime(createRegionsSelectionMixedContentTest(100));
> 
> Ditto.
> 
> > PerformanceTests/Layout/resources/regions.js:165
> > +    function createRegionsSelectionMixedContentTest(regionCount) {
> 
> This should be in .html fie so that we can see the test code in the single file.
> Also, this function and the test should be named something like "ExtendingSelection".
> "Selection" doesn't convey enough semantics.
> 
> > PerformanceTests/Layout/resources/regions.js:188
> > +    function createRegionsSelectAllMixedContentTest(regionCount) {
> 
> Ditto about moving this into the html file.

Attaching patch for landing fixing these issues.

> > PerformanceTests/Layout/resources/regions.js:212
> >      window.createRegionsTest = createRegionsTest;
> >      window.createRegionsSelectionTest = createRegionsSelectionTest;
> 
> These other test functions should really be in the respective html files.

Like this is related to other tests, it'll be fixed in a separated bug.

[Manuel Rego Casasnovas]

Created attachment 221252 [details]
Patch

Patch for landing.

[WebKit Commit Bot]

Comment on attachment 221252 [details]
Patch

Clearing flags on attachment: 221252

Committed r162065: <http://trac.webkit.org/changeset/162065>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Manuel Rego Casasnovas]

(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 220295 [details] [details])
> > > PerformanceTests/Layout/resources/regions.js:212
> > >      window.createRegionsTest = createRegionsTest;
> > >      window.createRegionsSelectionTest = createRegionsSelectionTest;
> > 
> > These other test functions should really be in the respective html files.
> 
> Like this is related to other tests, it'll be fixed in a separated bug.

This has been done in bug #127041.