Heads up! This PR modifies the following files:

• @KiChjang: components/net/file_loader.rs, components/script/dom/bindings/trace.rs, components/net/http_loader.rs, components/script/dom/xmlhttprequest.rs, components/net_traits/lib.rs, components/net_traits/lib.rs, components/script/document_loader.rs, components/script/script_thread.rs, components/script/dom/window.rs, components/script/dom/document.rs, components/net/image_cache_thread.rs

@jdm fyi on this PR. Also, merge conflicts, lame. I'll look into addressing.

r? @jdm

☔ The latest upstream changes (presumably #10695) made this pull request unmergeable. Please resolve the merge conflicts.

Want to point out - LoadData creation with referrer in components/msg/constellation_msg.rs is handled with a new_with_referrer() method. In components/net_traits/lib.rs, I edited the new() method to just add these 2 new fields in.

I assume I should make them consistent, but wanted your opinion on which is better 1st. new_with_referrer means less changes to track down throughout the code, and you don't just have "None, None" thrown on the end most of the time you are creating. Editing new is maybe cleaner though, considering there may be future adds.

Either way.

Yeah, I'd prefer to stay with editing the new method for now just to make sure we catch all the right places.

As for these changes, great work! I'm a bit leery of the way we pass around URLs and strings in the main referrer determination algorithm, so I'd like to see what that looks like if we write it more like the specification algorithm and only deal in URLs instead.

-S-awaiting-review +S-needs-code-changes

Reviewed 3 of 15 files at r1, 12 of 12 files at r2, 6 of 6 files at r4.
Review status: all files reviewed at latest revision, 21 unresolved discussions.

components/msg/constellation_msg.rs, line 420 [r4] (raw file):
Let's go with [Policies](https://w3c.github.io/webappsec-referrer-policy/#referrer-policy-states) for providing a referrer header for a request

components/net/http_loader.rs, line 369 [r2] (raw file):
This can accept &Url values. Let's just call serialize instead of accepting the serialized form as an argument as well.

components/net/http_loader.rs, line 370 [r2] (raw file):
Yes.

components/net/http_loader.rs, line 380 [r2] (raw file):
That is correct.

components/net/http_loader.rs, line 381 [r2] (raw file):
We should be relying on the Url::origin method here instead.

components/net/http_loader.rs, line 388 [r2] (raw file):
mut referrer_url instead of creating ref_mutable below.

components/net/http_loader.rs, line 391 [r2] (raw file):
relative.path.truncate()

components/net/http_loader.rs, line 398 [r2] (raw file):
mut referrer_url: Url
Also, nonorigin in the name is unclear. I assume this is referring to the origin-only flag specified in the original algorithm? I'd prefer to implement the algorithm from the specification as written (ie. as single method that accepts a boolean argument), since it's much easier to reason about the results in the caller than comparing the implementation of generate_nonorigin_referer_url to generate_origin_referer_url and the specification.

components/net/http_loader.rs, line 401 [r2] (raw file):
No need for this.

components/net/http_loader.rs, line 413 [r2] (raw file):
Let's have this return an Option<Url> instead of modifying the header directly. Then we could call this determine_request_referrer and have it more clearly match the specification algorithm.

components/net/http_loader.rs, line 416 [r2] (raw file):
We should be able to assert that this is false instead.

components/net/http_loader.rs, line 427 [r2] (raw file):
Let's use explicit patterns here instead of the catch-all _.

components/net/http_loader.rs, line 412 [r4] (raw file):
The proper way to support step 2 and related bits about redirections isn't totally clear to me right now. Let's add some TODO comments to call that out and deal with it later.

components/net_traits/lib.rs, line 90 [r2] (raw file):
I'd go with /// The policy and referring URL for the originator of this request

components/script/script_thread.rs, line 1886 [r2] (raw file):
Is this still relevant?

components/script/dom/document.rs, line 229 [r2] (raw file):
Let's make this link https://w3c.github.io/webappsec-referrer-policy/#referrer-policy-states

components/script/dom/document.rs, line 1689 [r2] (raw file):
Let's initialize this to NoReferrer.

components/script/dom/document.rs, line 1821 [r2] (raw file):
Let's make this return self.referrer_policy

components/script/dom/xmlhttprequest.rs, line 585 [r2] (raw file):
For future reference, we'll want to add an API to GlobalRef that returns a ReferrerPolicy value; we can use global.get_url() for the other I believe.

tests/unit/net/http_loader.rs, line 1461 [r2] (raw file):
The explicit type after load isn't necessary any more.

tests/unit/net/http_loader.rs, line 1470 [r2] (raw file):
Since these tests have so much code in common, let's merge them like so:

fn assert_referer_header_matches(request_url: &str, referrer_url: &str, referrer_policy: Option<ReferrerPolicy>, expected_referrer: &str) {
    // set up the URLs, create the LoadData, set the header value, call load
}

Then we can either have a bunch of different #[test] functions that all delegate to that one, or just one that contains a bunch of tests.

Comments from Reviewable

☔ The latest upstream changes (presumably #9942) made this pull request unmergeable. Please resolve the merge conflicts.

Review status: all files reviewed at latest revision, 21 unresolved discussions, some commit checks failed.

components/msg/constellation_msg.rs, line 420 [r4] (raw file):
Done.

components/net/http_loader.rs, line 369 [r2] (raw file):
Done.

components/net/http_loader.rs, line 381 [r2] (raw file):
Done.

components/net/http_loader.rs, line 391 [r2] (raw file):
Done.

components/net/http_loader.rs, line 398 [r2] (raw file):
Done.

components/net/http_loader.rs, line 416 [r2] (raw file):
No explicit method for it - this is how the other headers are handled fyi.

components/net/http_loader.rs, line 427 [r2] (raw file):
Done.

components/net/http_loader.rs, line 412 [r4] (raw file):
Done.

components/script/script_thread.rs, line 1886 [r2] (raw file):
nope.

components/script/dom/document.rs, line 229 [r2] (raw file):
Done.

components/script/dom/document.rs, line 1689 [r2] (raw file):
Done.

Comments from Reviewable

FYI - code comments addressed. Test changes are still a todo.

Please re-review the strip_url method after the latest commit - looks like rust-url was updated, so that method is changed.

Latest commit has that change and all comments to this point addressed.

Looking great!
-S-awaiting-review +S-needs-code-changes

Reviewed 11 of 15 files at r9, 4 of 4 files at r10.
Review status: all files reviewed at latest revision, 4 unresolved discussions.

components/net/http_loader.rs, line 416 [r2] (raw file):
Any headers object we pass in here should not already have a Referer header; APIs like XMLHttpRequest/Fetch forbid setting it, and other browser code shouldn't be setting it either. We don't carry over headers that were set by the browser prior to a redirect, either.

components/net/http_loader.rs, line 369 [r10] (raw file):
Add .unwrap() to each of the lines that complains about unused results. These operations shouldn't fail, and we'll want to know if they do, so unwrap is the right thing here..

tests/unit/net/http_loader.rs, line 1565 [r10] (raw file):
Shouldn't this be https?

tests/unit/net/http_loader.rs, line 1575 [r10] (raw file):
The test is called http_to_http...

Comments from Reviewable

Review status: all files reviewed at latest revision, 4 unresolved discussions.

components/net/http_loader.rs, line 416 [r2] (raw file):
Done.

components/net/http_loader.rs, line 369 [r10] (raw file):
Done.

tests/unit/net/http_loader.rs, line 1565 [r10] (raw file):
Oops, flipped these two.

tests/unit/net/http_loader.rs, line 1575 [r10] (raw file):
Done.

Comments from Reviewable

A+ would review again. Go ahead and squash the commits into one!
-S-awaiting-review +S-needs-squash

Reviewed 2 of 2 files at r11.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

@bors-servo: r+

📌 Commit 526525b has been approved by jdm

⌛ Testing commit 526525b with merge 3490081...

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt