[webkit-reviews] review denied: [Bug 234543] Archived subresource loads fail if m_allowedNetworkHosts doesn't include the remote URL : [Attachment 447746] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Dec 21 16:03:39 PST 2021


Alex Christensen <achristensen at apple.com> has denied Matt Woodrow
<m_woodrow at apple.com>'s request for review:
Bug 234543: Archived subresource loads fail if m_allowedNetworkHosts doesn't
include the remote URL
https://bugs.webkit.org/show_bug.cgi?id=234543

Attachment 447746: Patch

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




--- Comment #4 from Alex Christensen <achristensen at apple.com> ---
Comment on attachment 447746
  --> https://bugs.webkit.org/attachment.cgi?id=447746
Patch

This is problematic for many reasons:
1. It doesn't contain a unit test to verify we won't break it in the future.
2. It does not contain a good description of what is going on or under what
circumstances this changes behavior.
3. Being an "archive load" is not an inherent property of a resource request,
which ought to be basically just a URL and header fields
4.  It substantially changes the behavior of when scheduleArchiveLoad is
called, which almost certainly breaks web archive loading somehow.
5. There is no link to a radar or something motivating this change.

I believe the intent of this is to make allowsLoadFromURL return true if we
know it will only load from a web archive and not from the network when
_allowedNetworkHosts is being used.  Such a change should clearly not affect
anything else.


More information about the webkit-reviews mailing list