[webkit-reviews] review granted: [Bug 213342] Add referrerpolicy attribute support for <link> : [Attachment 402282] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Rob Buis <rbuis at igalia.com>'s request
for review:
Bug 213342: Add referrerpolicy attribute support for <link>
https://bugs.webkit.org/show_bug.cgi?id=213342

Attachment 402282: Patch

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




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

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

> Source/WebCore/html/parser/HTMLPreloadScanner.cpp:290
> +	       else if (match(attributeName, referrerpolicyAttr)) {
> +		   m_referrerPolicy = parseReferrerPolicy(attributeValue,
ReferrerPolicySource::ReferrerPolicyAttribute).valueOr(ReferrerPolicy::EmptyStr
ing);
> +		   break;
> +	       }

Doesn't seem necessary to include "break" here. The ones above don’t, and there
is a break after the chained if/else statements. That would also allow us to
omit the braces and this would fit in better with the lines above.

> Source/WebCore/loader/LinkLoader.h:57
> +    ReferrerPolicy referrerPolicy;

Should we set a default here of ReferrerPolicy::EmptyString? The other members
have defaults, except for relAttribute. Generally seems good to not risk this
ever being uninitialized. And maybe we can even omit this in some places if it
has a default?

> LayoutTests/TestExpectations:244
> -imported/w3c/web-platform-tests/referrer-policy [ Skip ]
> +imported/w3c/web-platform-tests/referrer-policy/origin [ Skip ]

Sure could use a comment.