Bug 40802 - Add CSS scanning to HTML5PreloadScanner
Summary: Add CSS scanning to HTML5PreloadScanner
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Adam Barth
URL:
Keywords:
Depends on:
Blocks: 39259
  Show dependency treegraph
 
Reported: 2010-06-17 16:43 PDT by Adam Barth
Modified: 2010-06-18 02:36 PDT (History)
5 users (show)

See Also:


Attachments
Patch (17.72 KB, patch)
2010-06-17 16:44 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
Patch (24.60 KB, patch)
2010-06-17 18:20 PDT, Adam Barth
eric: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Barth 2010-06-17 16:43:40 PDT
Add CSS scanning to HTML5PreloadScanner
Comment 1 Adam Barth 2010-06-17 16:44:45 PDT
Created attachment 59046 [details]
Patch
Comment 2 WebKit Review Bot 2010-06-17 16:45:24 PDT
Attachment 59046 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/html/CSSPreloadScanner.cpp:93:  Line contains only semicolon. If this should be an empty statement, use { } instead.  [whitespace/semicolon] [5]
WebCore/html/CSSPreloadScanner.cpp:116:  Line contains only semicolon. If this should be an empty statement, use { } instead.  [whitespace/semicolon] [5]
WebCore/html/CSSPreloadScanner.cpp:135:  Line contains only semicolon. If this should be an empty statement, use { } instead.  [whitespace/semicolon] [5]
Total errors found: 3 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Eric Seidel (no email) 2010-06-17 16:53:26 PDT
Comment on attachment 59046 [details]
Patch

Can we share the code with the legacy parser?  That would make this diff easier to verify correctness because we'd see the moved code.  (Ideally we'd split CSSPreloadScanner out of the old parser first before doing this patch.)
Comment 4 Adam Barth 2010-06-17 17:03:34 PDT
I was just going to delete the old preload scanner.
Comment 5 WebKit Review Bot 2010-06-17 17:20:21 PDT
Attachment 59046 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/3270341
Comment 6 Eric Seidel (no email) 2010-06-17 17:23:14 PDT
Comment on attachment 59046 [details]
Patch

We came up with a testing strategy in person using:
1.  a slow script which document.write('<plaintext>')
2.  A docuemnt with:
<script src="slow_script_which_document_write_plaintext.js"></script>
<style>
@import "foo.css"
</style>

and look for a loader callback for foo.css
Comment 7 Adam Barth 2010-06-17 18:20:39 PDT
Created attachment 59057 [details]
Patch
Comment 8 Eric Seidel (no email) 2010-06-17 18:32:24 PDT
Comment on attachment 59057 [details]
Patch

Thank you for adding testing.
Comment 9 Adam Barth 2010-06-17 19:01:16 PDT
Committed r61366: <http://trac.webkit.org/changeset/61366>
Comment 10 WebKit Review Bot 2010-06-17 19:17:52 PDT
http://trac.webkit.org/changeset/61366 might have broken Qt Linux Release
Comment 11 Adam Barth 2010-06-17 19:23:15 PDT
Qt and GTK don't support dumpResourceResponseMIMETypes, so we can't run these tests.
Comment 12 Antti Koivisto 2010-06-18 02:36:51 PDT
Nice, thanks!