Bug 238265

Created attachment 455507 [details] Patch

Comment on attachment 455507 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=455507&action=review > LayoutTests/imported/w3c/web-platform-tests/fetch/metadata/fetch.sub-expected.txt:10 > +FAIL http->http fetch (non-trustworthy destination => no metadata): sec-fetch-site assert_equals: expected "" but got "same-origin" NOTE: This will always FAIL locally because `localhost` is a trustworthy desintation. > LayoutTests/imported/w3c/web-platform-tests/fetch/metadata/form.https.sub-expected.txt:23 > +FAIL localhost -> www.localhost:9443 iframe: forced: sec-fetch-site assert_equals: expected "same-site" but got "cross-site" NOTE: This FAILs locally because `localhost` and `www.localhost` are not considered same site. This is probably a bug as the Registerable Domain for both of them is `null` in theory. > LayoutTests/imported/w3c/web-platform-tests/fetch/metadata/iframe.sub-expected.txt:12 > +FAIL Non-secure same-origin iframe => No headers: sec-fetch-site assert_equals: expected "" but got "same-origin" NOTE: This will always FAIL locally. `localhost` is trustworthy. > LayoutTests/imported/w3c/web-platform-tests/fetch/metadata/redirect/redirect-https-downgrade.sub-expected.txt:26 > +FAIL Https downgrade iframe: sec-fetch-site assert_equals: expected "" but got "cross-site" NOTE: Same here. Downgrading on `localhost` is still trustworthy.

Created attachment 455524 [details] Patch

(In reply to Patrick Griffis from comment #3) > The `topPrivatelyControlledDomain()` function seems to be platform specific > as it depends on libraries such as libpsl. It is implemented by the soup and > curl platforms but will also be needed on Apple/win ports. Maybe only the win port needs it actually.

<rdar://problem/91049146>

Created attachment 456576 [details] Patch

(In reply to Patrick Griffis from comment #2) > > LayoutTests/imported/w3c/web-platform-tests/fetch/metadata/form.https.sub-expected.txt:23 > > +FAIL localhost -> www.localhost:9443 iframe: forced: sec-fetch-site assert_equals: expected "same-site" but got "cross-site" > > NOTE: This FAILs locally because `localhost` and `www.localhost` are not > considered same site. This is probably a bug. So we can simply change `topPrivatelyControlledDomain()` to return `localhost` for `*.localhost` and it fixes these tests. I can't change the macOS impl of this though and its probably a change worth discussing if it makes sense.

(In reply to Patrick Griffis from comment #9) > So we can simply change `topPrivatelyControlledDomain()` to return > `localhost` for `*.localhost` and it fixes these tests. > > I can't change the macOS impl of this though (I'm blind and missed the macOS impl in obj-c with my search)

Comment on attachment 456576 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=456576&action=review > Source/WebCore/loader/cache/CachedResourceLoader.cpp:719 > + // of Sec-Fetch-Site in these cases. We should be able to get the previous SecFetchSite information from the previous request. If the previous request is cross origin, we can stick to it. Otherwise, we can check for same site/same origin. > Source/WebCore/page/SecurityOrigin.cpp:473 > +bool SecurityOrigin::isSameSiteAs(const SecurityOrigin& other) const We currently implement same site using RegistrableDomain operator ==. Can you check whether there are differences between the two implementations?

Comment on attachment 456576 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=456576&action=review >> Source/WebCore/loader/cache/CachedResourceLoader.cpp:719 >> + // of Sec-Fetch-Site in these cases. > > We should be able to get the previous SecFetchSite information from the previous request. > If the previous request is cross origin, we can stick to it. Otherwise, we can check for same site/same origin. I don't think we have enough information as-is. What we need to defend against is a redirect chain such as: http://origin1 (site=undefined) -> http://origin2 (site=undefined) -> https://origin2 (site=cross-origin) When we get to https://origin2 we need the full chain to know the correct value, otherwise its easy to mistakenly call it same-origin. My comment above mentions the idea of tracking the list with all Requests.

(In reply to Patrick Griffis from comment #12) > Comment on attachment 456576 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=456576&action=review > > >> Source/WebCore/loader/cache/CachedResourceLoader.cpp:719 > >> + // of Sec-Fetch-Site in these cases. > > > > We should be able to get the previous SecFetchSite information from the previous request. > > If the previous request is cross origin, we can stick to it. Otherwise, we can check for same site/same origin. > > I don't think we have enough information as-is. > > What we need to defend against is a redirect chain such as: > > http://origin1 (site=undefined) -> http://origin2 (site=undefined) -> > https://origin2 (site=cross-origin) > > When we get to https://origin2 we need the full chain to know the correct > value, otherwise its easy to mistakenly call it same-origin. It might be easy but we do not need the whole chain, since the last computed value is correct for all the previous redirects. We just need to take into account the last value and the new URL. We do this to compute whether a request is cross origin or not (once it is cross origin, it stays cross origin).

(In reply to youenn fablet from comment #11) > We should be able to get the previous SecFetchSite information from the > previous request. > If the previous request is cross origin, we can stick to it. Otherwise, we > can check for same site/same origin. I've implemented this where it's tracked for every request in the chain. One thing I could use help with is that `www.localhost` requests are blocked, even though it should be considered same-site. I fixed the `www.127.0.0.1` ones failing upstream: https://github.com/web-platform-tests/wpt/pull/33484

Created attachment 457275 [details] Patch

Created attachment 457710 [details] Patch

Created attachment 457749 [details] Patch

(In reply to youenn fablet from comment #11) > > Source/WebCore/page/SecurityOrigin.cpp:473 > > +bool SecurityOrigin::isSameSiteAs(const SecurityOrigin& other) const > > We currently implement same site using RegistrableDomain operator ==. > Can you check whether there are differences between the two implementations? From what I could tell these both compare domains the same and both use topPrivatelyControlledDomain(). However when PSL isn't available RegistrableDomain falls back to full host comparisons. I think a hard requirement on PSL for this feature is a good idea.

Comment on attachment 457749 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=457749&action=review > Source/WebCore/loader/FetchOptions.h:63 > + Site site { Site::SameOrigin }; I don't think site should be within FetchOptions. FetchOptions should more or less remain immutable during a load. FetchOptions values are set by the creator of the load to properly setup the load. Here, site is an internal state of the load, which can be stored directly in SubresourceLoader for instance. > Source/WebCore/loader/SubresourceLoader.cpp:281 > + m_options.site = m_documentLoader->cachedResourceLoader().computeFetchMetadataSite(newRequest, m_resource->type(), options().mode, originalOrigin, options().site); Can we make m_options.site a SubresourceLoader member field on its own? We can then pass it to updateRequestAfterRedirection below as an additional parameter. > Source/WebCore/loader/cache/CachedResourceLoader.cpp:660 > +static void updateRequestFetchMetadataHeaders(ResourceRequest& request, const ResourceLoaderOptions& options) Let's add a FetchOptions::Site parameter to this function. > Source/WebCore/loader/cache/CachedResourceLoader.cpp:881 > + request.setFetchMetadataSite(computeFetchMetadataSite(request.resourceRequest(), type, request.options().mode, frameLoader.frame().document()->securityOrigin(), request.options().site)); We do not need to set the site options here if we just pass it to updateRequestFetchMetadataHeaders below.

Comment on attachment 457749 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=457749&action=review >> Source/WebCore/loader/SubresourceLoader.cpp:281 >> + m_options.site = m_documentLoader->cachedResourceLoader().computeFetchMetadataSite(newRequest, m_resource->type(), options().mode, originalOrigin, options().site); > > Can we make m_options.site a SubresourceLoader member field on its own? > We can then pass it to updateRequestAfterRedirection below as an additional parameter. Maybe I'm missing it. But `CachedResourceLoader::updateHTTPRequestHeaders()` doesn't seem to have easy access to a SubResourceLoader to get this information? `CachedResourceLoader::requestResource()` updates the request headers first, then creates a `CachedResource` with that information but we can't get the SubresourceLoader until after that?

Comment on attachment 457749 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=457749&action=review >>> Source/WebCore/loader/SubresourceLoader.cpp:281 >>> + m_options.site = m_documentLoader->cachedResourceLoader().computeFetchMetadataSite(newRequest, m_resource->type(), options().mode, originalOrigin, options().site); >> >> Can we make m_options.site a SubresourceLoader member field on its own? >> We can then pass it to updateRequestAfterRedirection below as an additional parameter. > > Maybe I'm missing it. But `CachedResourceLoader::updateHTTPRequestHeaders()` doesn't seem to have easy access to a SubResourceLoader to get this information? > > `CachedResourceLoader::requestResource()` updates the request headers first, then creates a `CachedResource` with that information but we can't get the > SubresourceLoader until after that? CachedResourceLoader::updateHTTPRequestHeaders() is called for the initial load. We can compute the value from the request itself. SubresourceLoader will compute this same value as well (we can use the same routine as a free function) and in case of redirection will update this value to a new value. SubresourceLoader can pass this new value to updateRequestAfterRedirection. This way, we keep the model of FetchOptions being 'immutable' load options set by the client, and this site property being an internal state of the loader.

Created attachment 460621 [details] Implement FetchMetadata Site

Comment on attachment 460620 [details] Implement FetchMetadataSite View in context: https://bugs.webkit.org/attachment.cgi?id=460620&action=review I see a number of new failures in the test results. Can we fix more of them as-part-of/before adding this feature? > LayoutTests/imported/w3c/web-platform-tests/fetch/metadata/fetch.sub-expected.txt:10 > -PASS http->http fetch (non-trustworthy destination => no metadata): sec-fetch-site > +FAIL http->http fetch (non-trustworthy destination => no metadata): sec-fetch-site assert_equals: expected "" but got "same-origin" What’s the explanation for this new failure? Can we fix it? > LayoutTests/imported/w3c/web-platform-tests/fetch/metadata/iframe.sub-expected.txt:12 > -PASS Non-secure same-origin iframe => No headers: sec-fetch-dest > -PASS Non-secure same-origin iframe => No headers: sec-fetch-mode > -PASS Non-secure same-origin iframe => No headers: sec-fetch-site > +FAIL Non-secure same-origin iframe => No headers: sec-fetch-dest assert_equals: expected "" but got "iframe" > +FAIL Non-secure same-origin iframe => No headers: sec-fetch-mode assert_equals: expected "" but got "navigate" > +FAIL Non-secure same-origin iframe => No headers: sec-fetch-site assert_equals: expected "" but got "same-origin" What’s the explanation for these new failures? Can we fix them? > LayoutTests/imported/w3c/web-platform-tests/fetch/metadata/redirect/multiple-redirect-https-downgrade-upgrade.sub-expected.txt:10 > -PASS Https downgrade-upgrade embed > -PASS Https downgrade-upgrade fetch() api > -PASS Https downgrade-upgrade object > +TIMEOUT Https downgrade-upgrade embed Test timed out > +NOTRUN Https downgrade-upgrade fetch() api > +NOTRUN Https downgrade-upgrade object What’s the explanation for this new failure? Can we fix it? > LayoutTests/imported/w3c/web-platform-tests/fetch/metadata/redirect/multiple-redirect-https-downgrade-upgrade.sub-expected.txt:13 > -PASS Https downgrade-upgrade stylesheet > -TIMEOUT Https downgrade-upgrade track Test timed out > +NOTRUN Https downgrade-upgrade stylesheet > +NOTRUN Https downgrade-upgrade track What’s the explanation for this new failure? Can we fix it? > LayoutTests/imported/w3c/web-platform-tests/fetch/metadata/redirect/redirect-http-upgrade.sub-expected.txt:10 > -PASS Http upgrade embed > -PASS Http upgrade fetch() api > -PASS Http upgrade object > +TIMEOUT Http upgrade embed Test timed out > +NOTRUN Http upgrade fetch() api > +NOTRUN Http upgrade object What’s the explanation for this new failure? Can we fix it? > LayoutTests/imported/w3c/web-platform-tests/fetch/metadata/redirect/redirect-http-upgrade.sub-expected.txt:13 > -PASS Http upgrade stylesheet > -TIMEOUT Http upgrade track Test timed out > +NOTRUN Http upgrade stylesheet > +NOTRUN Http upgrade track What’s the explanation for this new failure? Can we fix it? > LayoutTests/imported/w3c/web-platform-tests/fetch/metadata/redirect/redirect-https-downgrade.sub-expected.txt:10 > -PASS Https downgrade embed > -PASS Https downgrade fetch() api > -PASS Https downgrade object > +TIMEOUT Https downgrade embed Test timed out > +NOTRUN Https downgrade fetch() api > +NOTRUN Https downgrade object What’s the explanation for this new failure? Can we fix it? > LayoutTests/imported/w3c/web-platform-tests/fetch/metadata/redirect/redirect-https-downgrade.sub-expected.txt:14 > -PASS Https downgrade stylesheet > -TIMEOUT Https downgrade track Test timed out > +NOTRUN Https downgrade stylesheet > +NOTRUN Https downgrade track > NOTRUN Https downgrade image => No headers What’s the explanation for this new failure? Can we fix it? > LayoutTests/imported/w3c/web-platform-tests/fetch/metadata/redirect/redirect-https-downgrade.sub-expected.txt:17 > -PASS Https downgrade script => No headers: sec-fetch-mode > -PASS Https downgrade script => No headers: sec-fetch-site > +FAIL Https downgrade script => No headers: sec-fetch-mode assert_equals: expected "" but got "no-cors" > +FAIL Https downgrade script => No headers: sec-fetch-site assert_equals: expected "" but got "cross-site" What’s the explanation for these new failures? Can we fix them? > LayoutTests/imported/w3c/web-platform-tests/fetch/metadata/redirect/redirect-https-downgrade.sub-expected.txt:27 > -PASS Https downgrade script => No headers: sec-fetch-dest > -PASS Https downgrade iframe: sec-fetch-dest > -PASS Https downgrade iframe: sec-fetch-mode > -PASS Https downgrade iframe: sec-fetch-site > -PASS Https downgrade iframe: sec-fetch-user > -PASS Https downgrade top level navigation: sec-fetch-dest > -PASS Https downgrade top level navigation: sec-fetch-mode > -PASS Https downgrade top level navigation: sec-fetch-site > +FAIL Https downgrade script => No headers: sec-fetch-dest assert_equals: expected "" but got "script" > +FAIL Https downgrade top level navigation: sec-fetch-dest assert_equals: expected "" but got "document" > +FAIL Https downgrade top level navigation: sec-fetch-mode assert_equals: expected "" but got "navigate" > +FAIL Https downgrade top level navigation: sec-fetch-site assert_equals: expected "" but got "cross-site" > PASS Https downgrade top level navigation: sec-fetch-user > +FAIL Https downgrade iframe: sec-fetch-dest assert_equals: expected "" but got "iframe" > +FAIL Https downgrade iframe: sec-fetch-mode assert_equals: expected "" but got "navigate" > +FAIL Https downgrade iframe: sec-fetch-site assert_equals: expected "" but got "cross-site" > +PASS Https downgrade iframe: sec-fetch-user What’s the explanation for these new failures? Can we fix them?

(In reply to Darin Adler from comment #25) > Comment on attachment 460620 [details] > Implement FetchMetadataSite > > View in context: > https://bugs.webkit.org/attachment.cgi?id=460620&action=review > > I see a number of new failures in the test results. Can we fix more of them > as-part-of/before adding this feature? > > > LayoutTests/imported/w3c/web-platform-tests/fetch/metadata/fetch.sub-expected.txt:10 > > -PASS http->http fetch (non-trustworthy destination => no metadata): sec-fetch-site > > +FAIL http->http fetch (non-trustworthy destination => no metadata): sec-fetch-site assert_equals: expected "" but got "same-origin" > > What’s the explanation for this new failure? Can we fix it? All of our tests run on trustworthy destinations (localhost). So many of these tests cannot work unless we set up a fake domain like WPT does. This requires making system wide changes as root. I don't know that this is appropriate. > > LayoutTests/imported/w3c/web-platform-tests/fetch/metadata/iframe.sub-expected.txt:12 > > -PASS Non-secure same-origin iframe => No headers: sec-fetch-dest > > -PASS Non-secure same-origin iframe => No headers: sec-fetch-mode > > -PASS Non-secure same-origin iframe => No headers: sec-fetch-site > > +FAIL Non-secure same-origin iframe => No headers: sec-fetch-dest assert_equals: expected "" but got "iframe" > > +FAIL Non-secure same-origin iframe => No headers: sec-fetch-mode assert_equals: expected "" but got "navigate" > > +FAIL Non-secure same-origin iframe => No headers: sec-fetch-site assert_equals: expected "" but got "same-origin" > > What’s the explanation for these new failures? Can we fix them? Same. http://localhost is trustworthy. > > LayoutTests/imported/w3c/web-platform-tests/fetch/metadata/redirect/redirect-https-downgrade.sub-expected.txt:17 > > -PASS Https downgrade script => No headers: sec-fetch-mode > > -PASS Https downgrade script => No headers: sec-fetch-site > > +FAIL Https downgrade script => No headers: sec-fetch-mode assert_equals: expected "" but got "no-cors" > > +FAIL Https downgrade script => No headers: sec-fetch-site assert_equals: expected "" but got "cross-site" > > What’s the explanation for these new failures? Can we fix them? Trustworthy destination. http://localhost. > > LayoutTests/imported/w3c/web-platform-tests/fetch/metadata/redirect/redirect-https-downgrade.sub-expected.txt:27 > > -PASS Https downgrade script => No headers: sec-fetch-dest > > -PASS Https downgrade iframe: sec-fetch-dest > > -PASS Https downgrade iframe: sec-fetch-mode > > -PASS Https downgrade iframe: sec-fetch-site > > -PASS Https downgrade iframe: sec-fetch-user > > -PASS Https downgrade top level navigation: sec-fetch-dest > > -PASS Https downgrade top level navigation: sec-fetch-mode > > -PASS Https downgrade top level navigation: sec-fetch-site > > +FAIL Https downgrade script => No headers: sec-fetch-dest assert_equals: expected "" but got "script" > > +FAIL Https downgrade top level navigation: sec-fetch-dest assert_equals: expected "" but got "document" > > +FAIL Https downgrade top level navigation: sec-fetch-mode assert_equals: expected "" but got "navigate" > > +FAIL Https downgrade top level navigation: sec-fetch-site assert_equals: expected "" but got "cross-site" > > PASS Https downgrade top level navigation: sec-fetch-user > > +FAIL Https downgrade iframe: sec-fetch-dest assert_equals: expected "" but got "iframe" > > +FAIL Https downgrade iframe: sec-fetch-mode assert_equals: expected "" but got "navigate" > > +FAIL Https downgrade iframe: sec-fetch-site assert_equals: expected "" but got "cross-site" > > +PASS Https downgrade iframe: sec-fetch-user > > What’s the explanation for these new failures? Can we fix them? All trustworthy. > > LayoutTests/imported/w3c/web-platform-tests/fetch/metadata/redirect/multiple-redirect-https-downgrade-upgrade.sub-expected.txt:10 > > -PASS Https downgrade-upgrade embed > > -PASS Https downgrade-upgrade fetch() api > > -PASS Https downgrade-upgrade object > > +TIMEOUT Https downgrade-upgrade embed Test timed out > > +NOTRUN Https downgrade-upgrade fetch() api > > +NOTRUN Https downgrade-upgrade object > > What’s the explanation for this new failure? Can we fix it? > > > LayoutTests/imported/w3c/web-platform-tests/fetch/metadata/redirect/multiple-redirect-https-downgrade-upgrade.sub-expected.txt:13 > > -PASS Https downgrade-upgrade stylesheet > > -TIMEOUT Https downgrade-upgrade track Test timed out > > +NOTRUN Https downgrade-upgrade stylesheet > > +NOTRUN Https downgrade-upgrade track > > What’s the explanation for this new failure? Can we fix it? > > > LayoutTests/imported/w3c/web-platform-tests/fetch/metadata/redirect/redirect-http-upgrade.sub-expected.txt:10 > > -PASS Http upgrade embed > > -PASS Http upgrade fetch() api > > -PASS Http upgrade object > > +TIMEOUT Http upgrade embed Test timed out > > +NOTRUN Http upgrade fetch() api > > +NOTRUN Http upgrade object > > What’s the explanation for this new failure? Can we fix it? > > > LayoutTests/imported/w3c/web-platform-tests/fetch/metadata/redirect/redirect-http-upgrade.sub-expected.txt:13 > > -PASS Http upgrade stylesheet > > -TIMEOUT Http upgrade track Test timed out > > +NOTRUN Http upgrade stylesheet > > +NOTRUN Http upgrade track > > What’s the explanation for this new failure? Can we fix it? > > > LayoutTests/imported/w3c/web-platform-tests/fetch/metadata/redirect/redirect-https-downgrade.sub-expected.txt:10 > > -PASS Https downgrade embed > > -PASS Https downgrade fetch() api > > -PASS Https downgrade object > > +TIMEOUT Https downgrade embed Test timed out > > +NOTRUN Https downgrade fetch() api > > +NOTRUN Https downgrade object > > What’s the explanation for this new failure? Can we fix it? > > > LayoutTests/imported/w3c/web-platform-tests/fetch/metadata/redirect/redirect-https-downgrade.sub-expected.txt:14 > > -PASS Https downgrade stylesheet > > -TIMEOUT Https downgrade track Test timed out > > +NOTRUN Https downgrade stylesheet > > +NOTRUN Https downgrade track > > NOTRUN Https downgrade image => No headers > > What’s the explanation for this new failure? Can we fix it? I will look into this in a bit more detail why these are NOTRUN rather than FAIL. Some failures are expected for the same domain issue.

(In reply to Patrick Griffis from comment #26) > I will look into this in a bit more detail why these are NOTRUN I can at least confirm these are genuinely NOTRUN and they were not ran before this patch. All of the test cases appendChild() an element (be it a link, or embed, etc) and WebKit never loads the elements. The only one that WebKit loads is the link element with `rel=preload` set.

(In reply to Patrick Griffis from comment #26) > All of our tests run on trustworthy destinations (localhost). So many of > these tests cannot work unless we set up a fake domain like WPT does. This > requires making system wide changes as root. I don't know that this is > appropriate. This doesn’t sound right to me. It seems to me that we *could* set up our tests so they see a fake domain without system-wide changes as root. These might require platform-specific techniques so the way we do this with CFNetwork on macOS and iOS might not be the same as on other platforms. I’m not saying we need to do this immediately, or that it is required for this patch, but I would like to know more about whether we plan to deal with this long term or not. Is there someone else watching this bug who knows about this?

(In reply to Darin Adler from comment #28) > (In reply to Patrick Griffis from comment #26) > > All of our tests run on trustworthy destinations (localhost). So many of > > these tests cannot work unless we set up a fake domain like WPT does. This > > requires making system wide changes as root. I don't know that this is > > appropriate. > > This doesn’t sound right to me. It seems to me that we *could* set up our > tests so they see a fake domain without system-wide changes as root. These > might require platform-specific techniques so the way we do this with > CFNetwork on macOS and iOS might not be the same as on other platforms. Sure other ways are possible. On libsoup we can make a custom GResolver to fake DNS responses and CURL does allow hardcoding an IP to a hostname. They are all very platform specific.

(In reply to Darin Adler from comment #28) > (In reply to Patrick Griffis from comment #26) > > All of our tests run on trustworthy destinations (localhost). So many of > > these tests cannot work unless we set up a fake domain like WPT does. This > > requires making system wide changes as root. I don't know that this is > > appropriate. > > This doesn’t sound right to me. It seems to me that we *could* set up our > tests so they see a fake domain without system-wide changes as root. These > might require platform-specific techniques so the way we do this with > CFNetwork on macOS and iOS might not be the same as on other platforms. > > I’m not saying we need to do this immediately, or that it is required for > this patch, but I would like to know more about whether we plan to deal with > this long term or not. > > Is there someone else watching this bug who knows about this? This is Bug 127676.

(In reply to Sam Sneddon [:gsnedders] from comment #30) > This is Bug 127676. I can work on this for the GLib/libsoup ports.

Initial work done here, input appreciated: https://github.com/WebKit/WebKit/pull/2489

Created attachment 461641 [details] Implement FetchMetadata Site The attached depends on bug 243428 however now every test is a PASS on GTK.

Pull request: https://github.com/WebKit/WebKit/pull/4186

Committed 255810@main (e5f68a529bf1): <https://commits.webkit.org/255810@main> Reviewed commits have been landed. Closing PR #4186 and removing active labels.

Comment on attachment 461641 [details] Implement FetchMetadata Site View in context: https://bugs.webkit.org/attachment.cgi?id=461641&action=review > Source/WebCore/loader/cache/CachedResourceLoader.cpp:705 > + if (m_documentLoader->frame()->settings().fetchMetadataEnabled()) { This needed a null check. Adding in https://github.com/WebKit/WebKit/pull/8461