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.
Created attachment 220295 [details] Patch
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.
(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.
Created attachment 221252 [details] Patch Patch for landing.
Comment on attachment 221252 [details] Patch Clearing flags on attachment: 221252 Committed r162065: <http://trac.webkit.org/changeset/162065>
All reviewed patches have been landed. Closing bug.
(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.