[webkit-reviews] review granted: [Bug 209049] Unique origins should not be Potentially Trustworthy : [Attachment 393501] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted youenn fablet <youennf at gmail.com>'s
request for review:
Bug 209049: Unique origins should not be Potentially Trustworthy
https://bugs.webkit.org/show_bug.cgi?id=209049

Attachment 393501: Patch

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




--- Comment #6 from Darin Adler <darin at apple.com> ---
Comment on attachment 393501
  --> https://bugs.webkit.org/attachment.cgi?id=393501
Patch

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

> Source/WebCore/dom/Document.cpp:6025
> +static inline bool isURLPotentiallyTrustworthy(const URL& url)

Seems overkill to write inline here.

> Source/WebCore/dom/Document.cpp:6027
> +    if (url.protocol() == "about"_s)

Should use URL::protocolIsAbout.

If not, then no need to use ASCIILiteral (the _s suffix) with ==; in fact I
think it currently makes the == operator less efficient, although we could fix
that. In general, to check protocols, I think we should use URL::protocolIs
rather than URL::protocol with ==.

> Source/WebCore/dom/Document.cpp:6028
> +	   return url.path() == "blank"_s || url.path() == "srcdoc"_s;

For about:blank we already have URL::isBlankURL. I suggest consider adding
something similar for "srcdoc" instead of writing out like this.

Is it correct for this to be case-sensitive or do we want case folding? If we
want case folding we should use equalLettersIgnoringASCIICase.

Same comment about using ASCIILiteral (the _s suffix).

Also, should change URL::path to return a StringView for efficiency.

> Source/WebCore/dom/Document.cpp:6029
> +    if (url.protocol() == "data"_s)

URL::protocolIsData