[webkit-reviews] review granted: [Bug 210431] Move URL to use StringView when returning substrings of the URL : [Attachment 397474] Patch

[bugzilla-daemon at webkit.org]

Anders Carlsson <andersca at apple.com> has granted Darin Adler
<darin at apple.com>'s request for review:
Bug 210431: Move URL to use StringView when returning substrings of the URL
https://bugs.webkit.org/show_bug.cgi?id=210431

Attachment 397474: Patch

https://bugs.webkit.org/attachment.cgi?id=397474&action=review




--- Comment #13 from Anders Carlsson <andersca at apple.com> ---
Comment on attachment 397474
  --> https://bugs.webkit.org/attachment.cgi?id=397474
Patch

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

> Source/WTF/wtf/URL.cpp:169
> +    Vector<LChar, 512> percentDecoded;

512 seems kinda arbitrary and a little high.

> Source/WTF/wtf/text/StringView.cpp:315
> +// FIXME: Should this be named parseNumber<uint16_t> instead?

I like this idea.

> Source/WebCore/loader/appcache/ManifestParser.cpp:38
> +static StringView manifestPath(const URL& manifestURL)

Need to be careful about passing temporaries to this function!

> Source/WebCore/platform/network/curl/CurlProxySettings.cpp:40
> +constexpr uint16_t SocksProxyPort = 1080;

I'd keep this static.

> Source/WebCore/platform/win/PasteboardWin.cpp:-582
> -	   String lastComponent = kurl.lastPathComponent();

kurl!

> Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp:1127
> +	   ||
isPublicSuffix(m_currentRequest.url().host().toStringWithoutCopying());

Surprised isPublicSuffic doesn't take a StringView.

> Source/WebKitLegacy/win/Plugins/PluginDatabase.cpp:2
> + * Copyright (C) 2006, 2007 Apple Inc. All rights reserved.

Maybe change the copyright years here too.