Add a perf test for the CSS parser.
Created attachment 126083 [details] Patch
Attachment 126083 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'PerformanceTests/ChangeLog', u'Performance..." exit_code: 1 Last 3072 characters of output: r/resources/style.css:4189: Line contains tab character. [whitespace/tab] [5] PerformanceTests/Parser/resources/style.css:4190: Line contains tab character. [whitespace/tab] [5] PerformanceTests/Parser/resources/style.css:4191: Line contains tab character. [whitespace/tab] [5] PerformanceTests/Parser/resources/style.css:4192: Line contains tab character. [whitespace/tab] [5] PerformanceTests/Parser/resources/style.css:4193: Line contains tab character. [whitespace/tab] [5] PerformanceTests/Parser/resources/style.css:4194: Line contains tab character. [whitespace/tab] [5] PerformanceTests/Parser/resources/style.css:4195: Line contains tab character. [whitespace/tab] [5] PerformanceTests/Parser/resources/style.css:4196: Line contains tab character. [whitespace/tab] [5] PerformanceTests/Parser/resources/style.css:4197: Line contains tab character. [whitespace/tab] [5] PerformanceTests/Parser/resources/style.css:4198: Line contains tab character. [whitespace/tab] [5] PerformanceTests/Parser/resources/style.css:4199: Line contains tab character. [whitespace/tab] [5] PerformanceTests/Parser/resources/style.css:4200: Line contains tab character. [whitespace/tab] [5] PerformanceTests/Parser/resources/style.css:4201: Line contains tab character. [whitespace/tab] [5] PerformanceTests/Parser/resources/style.css:4202: Line contains tab character. [whitespace/tab] [5] PerformanceTests/Parser/resources/style.css:4203: Line contains tab character. [whitespace/tab] [5] PerformanceTests/Parser/resources/style.css:4206: Line contains tab character. [whitespace/tab] [5] PerformanceTests/Parser/resources/style.css:4207: Line contains tab character. [whitespace/tab] [5] PerformanceTests/Parser/resources/style.css:4208: Line contains tab character. [whitespace/tab] [5] PerformanceTests/Parser/resources/style.css:4212: Line contains tab character. [whitespace/tab] [5] PerformanceTests/Parser/resources/style.css:4213: Line contains tab character. [whitespace/tab] [5] PerformanceTests/Parser/resources/style.css:4214: Line contains tab character. [whitespace/tab] [5] PerformanceTests/Parser/resources/style.css:4218: Line contains tab character. [whitespace/tab] [5] PerformanceTests/Parser/resources/style.css:4219: Line contains tab character. [whitespace/tab] [5] PerformanceTests/Parser/resources/style.css:4220: Line contains tab character. [whitespace/tab] [5] PerformanceTests/Parser/resources/style.css:4397: Line contains tab character. [whitespace/tab] [5] PerformanceTests/Parser/resources/style.css:4433: Line contains tab character. [whitespace/tab] [5] PerformanceTests/Parser/resources/style.css:4485: Line contains tab character. [whitespace/tab] [5] PerformanceTests/Parser/resources/style.css:4557: Line contains tab character. [whitespace/tab] [5] PerformanceTests/Parser/resources/style.css:4638: Line contains tab character. [whitespace/tab] [5] PerformanceTests/Parser/resources/style.css:5401: Line contains tab character. [whitespace/tab] [5] Total errors found: 34 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 126083 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=126083&action=review > PerformanceTests/Parser/resources/style.css:2 > +/* Facebook */ > +/*1328647392,176833978*/ We probably have to get permission from Facebook et al to use their styles. > PerformanceTests/Parser/resources/style.css:2913 > +.view-error .oops-msg .icon{margin-left:13px}.view-error .oops-msg .shadow{background:url(http://l.yimg.com/a/i/ww/met/shadow_icon_32_101508.png) no-repeat;margin-left:8px;height:6px;width:34px;float:left;clear:left} We probably also need to sanitize URLs in the css. We don't want to hit a live page when running the perf tests. > PerformanceTests/Parser/resources/style.css:6147 > +/* lib/yui/2.8.1/container/assets/container-core-min.css */ YUI might be OK. You'd have to check the license.
YUI appears to be BSD license: http://yuilibrary.com/license/
(In reply to comment #4) > YUI appears to be BSD license: http://yuilibrary.com/license/ Yeah I will update the patch on Friday with the Wikipedia CSS and the YUI CSS code, that should be enough.
Created attachment 126747 [details] Patch
Comment on attachment 126747 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=126747&action=review > PerformanceTests/Parser/resources/style.css:3 > +/* Wikipedia */ > + > +#interwiki-completelist{font-weight:bold} Where is the license/copyright notice for the wikipedia CSS file?
(In reply to comment #7) > (From update of attachment 126747 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=126747&action=review > > > PerformanceTests/Parser/resources/style.css:3 > > +/* Wikipedia */ > > + > > +#interwiki-completelist{font-weight:bold} > > Where is the license/copyright notice for the wikipedia CSS file? http://www.mediawiki.org/wiki/Download says it is GPL v2. Is that acceptable?
Comment on attachment 126747 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=126747&action=review >>> PerformanceTests/Parser/resources/style.css:3 >>> +#interwiki-completelist{font-weight:bold} >> >> Where is the license/copyright notice for the wikipedia CSS file? > > http://www.mediawiki.org/wiki/Download says it is GPL v2. Is that acceptable? GPL v2 might be incompatible with LGPL (v2.1).
Created attachment 126811 [details] Patch
(In reply to comment #10) > Created an attachment (id=126811) [details] > Patch Got rid of Wikipedia. Mozilla.org is not an option too. Seems pretty hard to find some big CSS chunk out there that is compatible with BSD licensing.
Apparently slashdot open-sources some of its code. e.g.: http://slashcode.git.sourceforge.net/git/gitweb.cgi?p=slashcode/slashcode;a=tree;f=themes/slashcode/htdocs;h=dedbc999441cb971b9092fbcaeb588aaa45a0289;hb=live
(In reply to comment #12) > Apparently slashdot open-sources some of its code. e.g.: > http://slashcode.git.sourceforge.net/git/gitweb.cgi?p=slashcode/slashcode;a=tree;f=themes/slashcode/htdocs;h=dedbc999441cb971b9092fbcaeb588aaa45a0289;hb=live But http://slashcode.git.sourceforge.net/git/gitweb.cgi?p=slashcode/slashcode;a=blob;f=themes/slashcode/htdocs/index.pl;h=fdc7527f03a936175e1e0c9a0c08b8de65aa6470;hb=live seems to say it is GPL :(.
Comment on attachment 126811 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=126811&action=review > PerformanceTests/Parser/css-parser.html:8 > + var styles = document.getElementById("styles"); > + styles.setAttribute("href", "resources/style.css"); Should we be removing the old element before attaching new one?
Created attachment 127221 [details] Patch
(In reply to comment #15) > Created an attachment (id=127221) [details] > Patch Ten consecutive runs output this : http://paste.kde.org/423104/ results seems pretty close which is good for stability.
(In reply to comment #16) > (In reply to comment #15) > > Created an attachment (id=127221) [details] [details] > > Patch > > Ten consecutive runs output this : > > http://paste.kde.org/423104/ > > results seems pretty close which is good for stability. Yup, that looks like a solid performance test.
(In reply to comment #17) > (In reply to comment #16) > > (In reply to comment #15) > > > Created an attachment (id=127221) [details] [details] [details] > > > Patch > > > > Ten consecutive runs output this : > > > > http://paste.kde.org/423104/ > > > > results seems pretty close which is good for stability. > > Yup, that looks like a solid performance test. Please bless it when you want :D
(In reply to comment #18) > (In reply to comment #17) > > (In reply to comment #16) > > > (In reply to comment #15) > > > > Created an attachment (id=127221) [details] [details] [details] [details] > > > > Patch > > > > > > Ten consecutive runs output this : > > > > > > http://paste.kde.org/423104/ > > > > > > results seems pretty close which is good for stability. > > > > Yup, that looks like a solid performance test. > > Please bless it when you want :D I think we should give this test a more specific name, e.g css-parser-yui. We may want to add more parser test in the future :)
Created attachment 127349 [details] Patch
Comment on attachment 127349 [details] Patch Clearing flags on attachment: 127349 Committed r107964: <http://trac.webkit.org/changeset/107964>
All reviewed patches have been landed. Closing bug.