[Webkit-unassigned] [Bug 148128] Decode data URLs in web process

[bugzilla-daemon at webkit.org]

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

--- Comment #36 from Antti Koivisto <koivisto at iki.fi> ---

> I don’t think we need this constructor. We should just be able to write:
> 
>     std::make_unique<DecodeTask>({ ... });

That wouldn't compile. More awkward version does:

    std::make_unique<DecodeTask>(DecodeTask { ... });


> > Source/WebCore/platform/network/DataURLDecoder.cpp:68
> > +    ASSERT(urlString.startsWith(dataString));
> 
> This check needs to ignore ASCII case.

I believe protocol part of URL is always converted to lowercase

> > Source/WebCore/platform/network/DataURLDecoder.cpp:72
> > +    if (headerEnd == notFound)
> > +        return nullptr;
> 
> Do we have a test case that covers this?

Probably not. We have some explicit data url test coverage and a ton of incidental coverage. It would be good to add a bunch of more systematic cases covering all the edges.

> > Source/WebCore/platform/network/DataURLDecoder.h:49
> > +void decode(const URL&, DecodeCompletionHandler);
> 
> Don’t functions sometimes have state and are thus expensive to copy? If so,
> it might be nicer to take ownership of the completion handler rather than
> passing it by value.

std::function has efficient move constructor. I think we just need to use WTF::move in a few places? (instead of say DecodeCompletionHandler&&)

> > Source/WebCore/platform/text/DecodeEscapeSequences.h:170
> > +            encodedRunPosition = length;
> 
> I’m not sure this line of code is needed. We intentionally made
> encodedRunPosition a very large value so it doesn’t need to be explicitly
> checked for. Using StringView::substring with it should work because it
> clamps its arguments down to the length of the string. It’s even possible,
> depending on how URLEscapeSequence::findEndOfRun is written, that we could
> remove the if statement entirely.

Ok. I didn't know that notFound value was intentionally chosen for this purpose.

-- 
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/20150821/cac0a7fd/attachment.html>