[Webkit-unassigned] [Bug 156334] Initial Link preload support

[bugzilla-daemon at webkit.org]

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

Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #275944|review?                     |review+
              Flags|                            |

--- Comment #26 from Darin Adler <darin at apple.com> ---
Comment on attachment 275944
  --> https://bugs.webkit.org/attachment.cgi?id=275944
Patch

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

Looks OK. No test coverage for the various types of preloading, which seems like a problem to me.

> Source/WebCore/loader/LinkLoader.cpp:104
> +    String lowerAs = as.convertToASCIILowercase();
> +    if (lowerAs == "image")
> +        return CachedResource::ImageResource;
> +    if (lowerAs == "script")
> +        return CachedResource::Script;
> +    if (lowerAs == "style")
> +        return CachedResource::CSSStyleSheet;
> +    if (lowerAs == "media")
> +        return CachedResource::MediaResource;
> +    if (lowerAs == "font")
> +        return CachedResource::FontResource;
> +    if (lowerAs == "track")
> +        return CachedResource::TextTrackResource;

This should use equalLettersIgnoringASCIICase and not allocate a lowercased copy of the string.

> Source/WebCore/loader/LinkLoader.cpp:114
> +    if (!href.isValid() || href.isEmpty()) {

I found other code doing checks like this, but there is no need to check isEmpty after checking isValid. Empty URLs are always also invalid.

> Source/WebCore/loader/LinkLoader.cpp:118
> +    Optional<CachedResource::Type> type = LinkLoader::getResourceTypeFromAsAttribute(as);

I think this reads better with the type "auto" instead of writing out the Optional type.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160411/677f96dd/attachment.html>