Bug 158636

Summary: handleDataURL is only used by curl
Product: WebKit Reporter: Alex Christensen <achristensen>
Component: New BugsAssignee: Alex Christensen <achristensen>
Status: RESOLVED FIXED    
Severity: Normal CC: Basuke.Suzuki, commit-queue, Hironori.Fujii
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch thorton: review+

Description Alex Christensen 2016-06-10 15:07:11 PDT
handleDataURL is only used by curl
Comment 1 Alex Christensen 2016-06-10 15:07:29 PDT
Created attachment 281051 [details]
Patch
Comment 2 WebKit Commit Bot 2016-06-10 15:09:45 PDT
Attachment 281051 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Darin Adler 2016-06-10 15:38:09 PDT
Comment on attachment 281051 [details]
Patch

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

> Source/WebCore/platform/network/curl/ResourceHandleManager.cpp:916
> +static void handleDataURL(ResourceHandle* handle)

This should take a reference, not a pointer.

> Source/WebCore/platform/network/curl/ResourceHandleManager.cpp:921
> +    ASSERT(handle);

Not all that valuable to assert this after first using it in a second assertion above.

> Source/WebCore/platform/network/curl/ResourceHandleManager.cpp:924
> +    int index = url.find(',');

Type here needs to be auto, not int.

> Source/WebCore/platform/network/curl/ResourceHandleManager.cpp:925
> +    if (index == -1) {

Needs to be "notFound", not -1.

> Source/WebCore/platform/network/curl/ResourceHandleManager.cpp:930
> +    String mediaType = url.substring(5, index - 5);

Function should use a lot more StringView and a lot less String to be more efficient.

> Source/WebCore/platform/network/curl/ResourceHandleManager.cpp:933
> +    bool base64 = mediaType.endsWith(";base64", false);

Would be nice for this to use endsWithIgnoringASCIICase instead of passing "false" here, although it looks like that one is not overloaded for string literals so it would allocate a temporary string unless we add that overload.

> Source/WebCore/platform/network/curl/ResourceHandleManager.cpp:938
> +        mediaType = "text/plain";

Should use ASCIILiteral here.

> Source/WebCore/platform/network/curl/ResourceHandleManager.cpp:944
> +        charset = "US-ASCII";

Should use ASCIILiteral here.
Comment 4 Basuke Suzuki 2017-02-23 15:14:26 PST
This bug seems to be resolved.
https://trac.webkit.org/changeset/201946
Comment 5 Basuke Suzuki 2017-03-02 11:06:11 PST
This bug seems to be resolved.
https://trac.webkit.org/changeset/201946