[webkit-reviews] review granted: [Bug 205462] Fetch: handle emtpy Location value : [Attachment 386126] Patch

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has granted Rob Buis <rbuis at igalia.com>'s
request for review:
Bug 205462: Fetch: handle emtpy Location value
https://bugs.webkit.org/show_bug.cgi?id=205462

Attachment 386126: Patch

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




--- Comment #2 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 386126
  --> https://bugs.webkit.org/attachment.cgi?id=386126
Patch

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

> Source/WebCore/loader/SubresourceLoader.cpp:405
> +	   if (options().redirect == FetchOptions::Redirect::Follow &&
response.isRedirection()) {

response.isRedirection() is not needed here.

> Source/WebCore/loader/SubresourceLoader.cpp:406
> +	       if
(response.httpHeaderFields().contains(HTTPHeaderName::Location) &&
response.httpHeaderField(HTTPHeaderName::Location).isEmpty()) {

Can we just have one check with something like !isNull() && isEmpty(). or
impl() && impl()->isEmpty() maybe?

> Source/WebCore/loader/SubresourceLoader.cpp:410
> +	       }

Would be nice if we could do something like:
if (options().redirect == FetchOptions::Redirect::Follow &&
isLocationURLFailure(response)) {
    ...
    return;
}