[Webkit-unassigned] [Bug 106198] PreloadScanner preloads external CSS with non-matching media attribute

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=106198





--- Comment #9 from Adam Barth <abarth at webkit.org>  2013-04-01 10:54:58 PST ---
(From update of attachment 195963)
View in context: https://bugs.webkit.org/attachment.cgi?id=195963&action=review

The general approach looks reasonable.

You're also missing a ChangeLog entry.  :)

> Source/WebCore/html/parser/HTMLResourcePreloader.h:41
>      bool isSafeToSendToAnotherThread() const;

Please add m_mediaAttr to this function so that we can detect threading bug.

> Source/WebCore/html/parser/HTMLResourcePreloader.h:57
> +        , m_mediaAttr(mediaAttr)

You need to make a copy of this string here because this object is sent across threads.  See m_resourceURL above.

> Source/WebCore/html/parser/HTMLResourcePreloader.h:69
> +    String m_mediaAttr;

m_mediaAttr -> m_mediaAttribute (please use complete words in variable names)

> Source/WebCore/html/parser/HTMLResourcePreloader.h:92
> +    static bool mediaAttributeMatches(Frame* frame, RenderStyle* renderStyle, const String& attributeValue);

There's no reason to expose this function in the header.  It can just be a static function in the implementation.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.