2005006 - Store parsed URIs in InternalRequest/InternalResponse

Status: RESOLVED FIXED

(Core :: DOM: Networking, enhancement, P2)

Description

[Nika Layzell [:nika] (ni? for response)]

Currently we have some test failures around Blob URLs due to the URL not being parsed when a Request object is created. Not parsing the Blob URL quickly enough can lead to the request failing, as the revoked flag is set on the Blob URL as the URL is parsed (which is our implementation of recording the specific entry).

We should store parsed nsIURI instances within InternalRequest and InternalResponse so that extra state, such as the revoked flag, is maintained.

Comment 1

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 2005006 - Store parsed URIs in InternalRequest/InternalResponse, r=#dom-core!

This patch updates InternalRequest and InternalResponse such that it parses the
URL passed to it at creation time, rather than when the channel is created.
This should improve the behaviour of these types around Blob URLs, so that
extra state, such as the "revoked" flag, is calculated at the correct time
as-per spec.

While making this change, I noticed that a previous Cache DB migration could
insert an "" URL in the DB when migrating from a single response URL to a URL
list, rather than storing an empty URL list. The choice was made to do
filtering when reading, rather than performing a migration, to avoid migration
first-time start-up overhead.

Comment 2

[BugBot [:suhaib / :marco]]

There is an r+ patch which didn't land and no activity in this bug for 2 weeks.
:nika, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit BugBot documentation.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/e7bf4a196fb7
https://hg.mozilla.org/integration/autoland/rev/8f9dcf127f8f
Store parsed URIs in InternalRequest/InternalResponse, r=dom-core,dom-worker-reviewers,necko-reviewers,smaug,kershaw,edenchuang

Comment 4

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/8f9dcf127f8f

Comment 5

[Alex Finder]

(In reply to Pulsebot from comment #3)

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/e7bf4a196fb7
https://hg.mozilla.org/integration/autoland/rev/8f9dcf127f8f
Store parsed URIs in InternalRequest/InternalResponse,
r=dom-core,dom-worker-reviewers,necko-reviewers,smaug,kershaw,edenchuang

Perfherder has detected a devtools performance change from push 8f9dcf127f8f64e0ebe79f6ac3af02321a4eded0.

No action is required from the author; this comment is provided for informational purposes only.

Improvement	Test	Platform	Options	Absolute values [old vs new]
12%	damp custom.netmonitor.bigdatarequests.navigate.DAMP (doc)	linux2404-64-shippable	e10s fission stylo webrender	1,075.81 ms -> 945.77 ms
8%	damp custom.netmonitor.bigdatarequests.requestsFinished.DAMP (doc)	linux2404-64-shippable	e10s fission stylo webrender	1,300.60 ms -> 1,197.84 ms

Need Help or Information?

If you have any questions, please reach out to [e-mail]. Alternatively, you can find help on Slack by joining #perf-help, and on Matrix you can find help by joining #perftest.

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.