Summary: | [CSS Regions] Add performance tests for selection with mixed content | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Manuel Rego Casasnovas <rego> | ||||||
Component: | Tools / Tests | Assignee: | Manuel Rego Casasnovas <rego> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, jfernandez, mihnea, rniwa, WebkitBugTracker | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 118668 | ||||||||
Attachments: |
|
Description
Manuel Rego Casasnovas
2014-01-03 02:32:46 PST
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. |