Bug 173192 – URL::host should return a StringView

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: New Bugs

[Alex Christensen]

URL::host should return a StringView

[Alex Christensen]

Created attachment 312507 [details]
Patch

[Alex Christensen]

Created attachment 312511 [details]
Patch

[Chris Dumez]

Comment on attachment 312511 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        URL::host should return a StringView

This adds quite a bit of toString() at call sites, which is unfortunate. Couldn't we add a new method which returns a StringView for call sites that actually need a view rather than a string?

[Alex Christensen]

We should work towards removing those .toString() calls by doing things like making functions in HashSet<String> that take StringViews as arguments, etc.  This is a step in the right direction.

[Chris Dumez]

(In reply to Alex Christensen from comment #4)
> We should work towards removing those .toString() calls by doing things like
> making functions in HashSet<String> that take StringViews as arguments, etc.
> This is a step in the right direction.

I also worry about the safety of this. People should be very careful when using StringViews.

e.g.
auto host = url.host();
// modify url
// host is now potentially bad.

[Alex Christensen]

That's true for all StringViews.  That shouldn't prevent this patch.
If anything we should get rid of StringView.toStringWithoutCopying, but that's completely unrelated to this patch.

[Alex Christensen]

Created attachment 312513 [details]
Patch

[Build Bot]

Attachment 312513 [details] did not pass style-queue:


ERROR: Source/WebCore/page/SecurityOrigin.cpp:148:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 48 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alex Christensen]

*** This bug has been marked as a duplicate of bug 185986 ***