[webkit-reviews] review requested: [Bug 199632] Remove support for beforeload on link=prefetch : [Attachment 373911] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jul 12 08:45:53 PDT 2019

Rob Buis <rbuis at igalia.com> has asked  for review:
Bug 199632: Remove support for beforeload on link=prefetch

Attachment 373911: Patch


--- Comment #20 from Rob Buis <rbuis at igalia.com> ---
Comment on attachment 373911
  --> https://bugs.webkit.org/attachment.cgi?id=373911

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

>> Source/WebCore/ChangeLog:13
>> +	    (WebCore::LinkLoader::loadLink):
> It is not clear these changes remove support for beforeload for prefetch.
> I understand it that we remove the call to shouldLoadLink for prefetch which
thus removes the call to before load.
> Can you give some more description in the change log?

Sure! Done.

>> Source/WebCore/loader/LinkLoader.cpp:286
>> +	if (!params.href.isValid() || !document.frame())
> It is initially intriguing to not call shouldLoadLink for prefetch.
> It might be clearer to call m_client.shouldLoadLink() in every case in
LinkLoader::loadLink and return true early if the link is prefetch.


>> Source/WebCore/loader/LinkLoader.cpp:332
>> +	    return true;
> It seems loadLink is always returning true.
> Maybe loadLink should be void.

Well spotted, done.

>> LayoutTests/http/wpt/prefetch/beforeload-dynamic.html:6
>> +</script>
> Can we create it in the script element below?


>> LayoutTests/http/wpt/prefetch/beforeload.html:5
>> +	var t = async_test('Makes sure that prefetch does not call
> Ditto.


More information about the webkit-reviews mailing list