Bug 158636 - handleDataURL is only used by curl
Summary: handleDataURL is only used by curl
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-06-10 15:07 PDT by Alex Christensen
Modified: 2017-03-02 11:06 PST (History)
3 users (show)

See Also:


Attachments
Patch (10.73 KB, patch)
2016-06-10 15:07 PDT, Alex Christensen
thorton: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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