Bug 57664 - Stop preload scanning CSS when it's impossible to have another @import.
Summary: Stop preload scanning CSS when it's impossible to have another @import.
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: James Simonsen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-04-01 14:51 PDT by James Simonsen
Modified: 2011-04-05 03:50 PDT (History)
6 users (show)

See Also:

Attachments
Patch (7.14 KB, patch)
2011-04-01 14:57 PDT, James Simonsen 		no flags Details | Formatted Diff | Diff
Patch for landing (7.20 KB, patch)
2011-04-04 15:06 PDT, James Simonsen 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description James Simonsen 2011-04-01 14:51:51 PDT 
Stop preload scanning CSS when it's impossible to have another @import.
Comment 1 James Simonsen 2011-04-01 14:57:14 PDT 
Created attachment 87920 [details]
Patch
Comment 2 Adam Barth 2011-04-01 15:07:04 PDT 
Comment on attachment 87920 [details]
Patch

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

> Source/WebCore/html/parser/CSSPreloadScanner.cpp:68
> +        if (isHTMLSpace(c))
> +            { }

Woah.  That's how we usually do that.  I'd have a separate if clause that just has a break in it.

> Source/WebCore/html/parser/CSSPreloadScanner.cpp:90
>          else if (c == '*')
> -            ;
> +            { }

I'd also fix all of these.  :)
Comment 3 James Simonsen 2011-04-01 15:23:15 PDT 
webkit-patch upload complained. This is the way it told me to do it. I think it's referring to #4 under Braces in the style guide. I'll defer to your judgement though.
Comment 4 Adam Barth 2011-04-01 15:35:26 PDT 
{ } is preferred to ; but we should avoid both if we can.
Comment 5 Adam Barth 2011-04-01 15:39:19 PDT 
Comment on attachment 87920 [details]
Patch

As for the actual change, this looks great!
Comment 6 James Simonsen 2011-04-04 15:06:25 PDT 
Created attachment 88139 [details]
Patch for landing
Comment 7 WebKit Commit Bot 2011-04-05 02:26:18 PDT 
The commit-queue encountered the following flaky tests while processing attachment 88139 [details]:

fast/workers/storage/use-same-database-in-page-and-workers.html bug 50995 (author: dumi@chromium.org)
The commit-queue is continuing to process your patch.
Comment 8 WebKit Commit Bot 2011-04-05 02:28:15 PDT 
Comment on attachment 88139 [details]
Patch for landing

Clearing flags on attachment: 88139

Committed r82916: <http://trac.webkit.org/changeset/82916>
Comment 9 WebKit Commit Bot 2011-04-05 02:28:20 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 10 WebKit Review Bot 2011-04-05 03:50:45 PDT 
http://trac.webkit.org/changeset/82916 might have broken GTK Linux 32-bit Debug