Closed Bug 1568691 Opened 5 years ago Closed 5 years ago

[wpt-sync] Sync PR 18061 - Use correct URLLoaderFactory in an unload handler.

Categories

(Core :: DOM: Networking, task, P5)

Product:
Core
Component:
DOM: Networking
Type:
task

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla70
Tracking Flags:
Tracking Status
firefox70 --- fixed

People

(Reporter: mozilla.org, Unassigned)

References

(
URL
)

Details

(Whiteboard: [necko-triaged][wptsync downstream])

Sync web-platform-tests PR 18061 into mozilla-central (this bug is closed when the sync is complete).

PR: https://github.com/web-platform-tests/wpt/pull/18061
Details from upstream follow.

Lukasz Anforowicz <lukasza@chromium.org> wrote:

Use correct URLLoaderFactory in an unload handler.

Context and behavior implemented + expected before and after the CL

A content::RenderFrameImpl stores |subresource_loader_factories| that
are used for all requests/XHRs/etc initiated by the frame. We don't
currently swap the RenderFrame when committing another same-process
document (this future work is tracked by https://crbug.com/936696).

The URLLoaderFactory for handling http/https requests contains a
|request_initiator_site_lock| in network::URLLoaderFactoryParams.
This lock limits what network::ResourceRequest::request_initiator may be
used. The lock is currently used in various, security-sensitive
features like CORB, CORP, Sec-Fetch-Site. In the future the lock may be
consulted for all requests handled by the NetworkService (see
https://crbug.com/920634 and also https://crbug.com/961614).

When a cross-origin, same-process navigation commits, then:

  • The commit IPC carries a bundle of |subresource_loader_factories|.
    The new |subresource_loader_factories| should be used by the newly
    committed document.

  • Before committing the new document, the old document's unload
    handler needs to run. The unload handler needs to use the old
    |subresource_loader_factories| (e.g. because otherwise
    |URLLoaderFactoryParams::request_initiator_site_lock| might
    not match the document origin).

The problematic behavior before this CL

Before the CL, content::RenderFrameImpl::CommitNavigationWithParams
would start using the new |subresource_loader_factories| before running
the unload handler of the old document (i.e. before calling
blink::WebNavigationControl::CommitNavigation on |frame_|).

The CL adds WPT tests that fail when the old, incorrect behavior
happens. The new test initiates a beacon request from a frame's unload
handler and verifies the Sec-Fetch-Site request header associated with
this request. The header depends on the correct
request_initiator_site_lock / on using the correct URLLoaderFactory.

The fixes in this CL

This CL introduces the |call_before_attaching_new_document| callback
that is taken by blink::WebNavigationControl::CommitNavigation and
called after finishing running the old document's unload handler
and before the new document can initiate any requests of its own.

This fix is a bit icky. In the long-term the callback can be avoided if
the |subresource_loader_factories| are stored in a per-document object
(i.e. if we swap RenderFrame on every document change, or if we replace
RenderFrame with a RenderDocument - see https://crbug.com/936696). The
CL adds a TODO to call this out.

The fix had to refactor BuildServiceWorkerNetworkProviderForNavigation
to make sure that it uses the |new_loader_factories| (even though they
have not yet been passes to SetLoaderFactoryBundle).

The fix also refactored GetLoaderFactoryBundleFromCreator to a separate
method, so that setting the creator-based factories can also be
postponed to |call_before_attaching_new_document|.

Bug: 986577
Change-Id: If0209df61b0305ec43b5179bfdc1b9f8668a88b5
Reviewed-on: https://chromium-review.googlesource.com/1716084
WPT-Export-Revision: 9248c2aa2fced8ffac6f4467bde195a81b181ca3

Component: web-platform-tests → DOM: Networking
Product: Testing → Core
Priority: P4 → P5
Whiteboard: [wptsync downstream] → [necko-triaged][wptsync downstream]
Ran 1 tests and 1 subtests
OK     : 1
FAIL   : 1

New tests that have failures or other problems:
/fetch/sec-metadata/unload.tentative.https.sub.html
    Fetch from an unload handler: FAIL
Pushed by wptsync@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c67426ef2222
[wpt PR 18061] - Use correct URLLoaderFactory in an unload handler., a=testonly
https://hg.mozilla.org/integration/mozilla-inbound/rev/2e654ff196f6
[wpt PR 18061] - Update wpt metadata, a=testonly

https://hg.mozilla.org/mozilla-central/rev/c67426ef2222
https://hg.mozilla.org/mozilla-central/rev/2e654ff196f6

Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox70: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla70
You need to log in before you can comment on or make changes to this bug.