1543990 - Regression in Firefox 66: <link rel="preconnect"> no longer working

Status: RESOLVED FIXED

(Core :: Networking, defect, P1)

Description

[jakub.g]

Attachment: fx65-fx66-preconnect.png

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:66.0) Gecko/20100101 Firefox/66.0

Steps to reproduce:

Open https://jg-testpage.github.io/wpt/preconnect.html in Firefox 66

It contains <link rel="preconnect"> + setTimeout(1000) HTTP request

Actual results:

<link rel="preconnect"> was ignored, and DNS+TCP+TLS happened at the HTTP request time

From my tests with devtools it seems it was working until Firefox 65 (see attachment)

Expected results:

DNS+TCP+TLS should have happened earlier

Firefox 66 KO https://www.webpagetest.org/result/190411_12_ed5cfb727a48dcea11b1a4f54999922c/1/details/

Firefox 68 Nightly KO https://www.webpagetest.org/result/190411_T3_3cb066d3ceb1fbd2be1bf33cfb491bf5/1/details/

Chrome OK https://www.webpagetest.org/result/190412_AQ_147b2535813e4324497d471e22d67445/1/details/

Comment 1

[jakub.g]

Hmm it seems to be KO also on Firefox 60 ESR on WebPageTest which is quite surprising to me:

https://www.webpagetest.org/result/190412_VW_914427d9be5121b7bab6fac7c054ea64/1/details/#waterfall_view_step1

I was thinking that since Firefox ESR always reports "60.0" in useragent string, it could be that WPT has ESR v60.6 which may contain the faulty code from Firefox 66, but I tested 60.6 ESR locally and it is not affected.

Comment 2

[jakub.g]

If someone takes this bug, note that test methodology is important to repro correctly: if you repeat the test (refresh the page), you won't repro the issue anymore, because the browser maintains the connection open for some number of seconds.

The ideal test methodology is:

1 Close the test URL tab (to make sure it doesn't reload automatically on browser restart)
2 Close the browser (to clean the connection pool)
3 Optionally, clear DNS cache of the operating system
4 Reopen the browser
5 Open a new tab
6 Open devtools (to make sure you capture all the network requests)
7 Load test URL

Comment 3

[bulk88]

Attachment: ff87-no-preconnect.png

Comment 4

[bulk88]

This is still a bug in Firefox 87. <link> Preconnect is non-functional in FF 87.

Comment 5

[Nicolas Hoizey]

When well used, preconnect can help enhance the user experience, so fixing this would be really great. 🙏

Comment 6

[saurabhchardereal]

As of FF version 105, it is still not working.

Comment 7

[Andrew Creskey [:acreskey]]

Thanks for reporting this one and to the folks following up.
This should be fixed with Bug 1813618, I'll verify once it has landed.

Comment 8

[Andrew Creskey [:acreskey]]

Performance impact set as high due to widespread use on the web.

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is set to S3. However, the Performance Impact field flags this bug as having a high impact on the performance.
:valentin, could you consider increasing the severity of this performance-impacting bug? Alternatively, if you think the performance impact is lower than previously assessed, could you request a re-triage from the performance team by setting the Performance Impact flag to ??

For more information, please visit auto_nag documentation.

Comment 10

[Andrew Creskey [:acreskey]]

I propose that this is Priority 1 (definitely want), and S2 (high impact, no workaround).

Comment 11

[Valentin Gosu [:valentin]]

This still depends on bug 1813618 being fixed.
Assigning the bug to myself to silence the autonag bot.

Comment 12

[Andrew Creskey [:acreskey]]

With Bug 1813618 fixed, I would expect to see rel=preconnect functioning.

I'm using a test based on jakub.g's:
https://acreskeymoz.github.io/preconnect.html
and a variation without the preconnect: https://acreskeymoz.github.io/no_preconnect.html

However while I can see the speculative connect (and resulting DNS, TCP, TLS) begin upon reading the preconnect, as far as I can tell, we do this all over again when the actual resource load is requested.
And so no improvement is visible.

Here's a profile, preconnect to https://www.wikipedia.org, and later on load an image from that host.
https://share.firefox.dev/44ghCNz

And one where I load the site without the rel=preconnect
https://share.firefox.dev/44k7O55
I can see that there is no initial speculative connect, but the resource load time and time to fully loaded are not improved.

I'm using traffic shaping to increase my network latency so that any differences are more visible.

I do see this log during the resource load in the preconnect scenario,

GetH2orH3ActiveConn() request for ent 12ceed910 .S........[tlsflags0x00000000]www.wikipedia.org:443^partitionKey=%28https%2Cacreskeymoz.github.io%29 did not find an active connection 

So maybe we are making a new connection for some reason.

Comment 13

[Valentin Gosu [:valentin]]

nsHttpConnectionMgr::OnMsgSpeculativeConnect [ci=.S.......[tlsflags0x00000000]www.wikipedia.org:443^partitionKey=%28https%2Cwikipedia.org%29, mFetchHTTPSRR=1]

Indeed, it seems that the preconnect is using the origin of the link, instead of the top level domain (acreskeymoz.github.io) when loaded via rel=preconnect so it ends up not being used.

The code this is coming from doesn't seem immediately wrong, so probably the issue is somewhere deeper in Necko.

Comment 14

[Kershaw Chang [:kershaw]]

I think the problem is at this line.
Since we passed the URI of link header (www.wikipedia.org) to the function UpdateOriginAttributesForNetworkState, we get the the partition key partitionKey=%28https%2Cwikipedia.org%29 and we create a speculative connection with the key .S........[tlsflags0x00000000]www.wikipedia.org:443^partitionKey=%28https%2Cacreskeymoz.github.io%29. However, what we should pass to UpdateOriginAttributesForNetworkState is the first party URI, not the one in link header. I think the correct first party URI here is acreskeymoz.github.io.

Comment 15

[Kershaw Chang [:kershaw]]

Attachment: Bug 1543990 - Use correct first party URI to update OriginAttributes, r=#necko

Comment 16

[Andrew Creskey [:acreskey]]

Thank you Valentin and nicely done, Kershaw.
With your patch I can confirm that rel=preconnect is now working as expected with this profile: https://share.firefox.dev/3HAWgkm
looking at https://www.wikipedia.org/portal/wikipedia.org/assets/img/[e-mail]

Comment 17

[Andrew Creskey [:acreskey]]

Attachment: preconnect.png

The dev tools highlight the improvement, here with preconnect.

Comment 18

[Andrew Creskey [:acreskey]]

Attachment: no_preconnect.png

And without preconnect.

Comment 19

[Andrew Creskey [:acreskey]]

In code review we discovered that the solution is a little more complicated but we expect to fix this shortly.

Comment 21

[Kershaw Chang [:kershaw]]

Take this from Valentin, since I am working on this.

Comment 22

[Kershaw Chang [:kershaw]]

Attachment: Bug 1543990 - Simplify nsISpeculativeConnect API, r=#necko

Comment 23

[Kershaw Chang [:kershaw]]

Attachment: Bug 1543990 - Introduce a new API to create speculative connection with the partition key in originAttributes, r=#necko

Depends on D179066

Comment 24

[Kershaw Chang [:kershaw]]

Attachment: Bug 1543990 - Use origin attributes to start speculative connections for link header, r=#necko

Depends on D179067

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/89a8aa4b8d28
Simplify nsISpeculativeConnect API, r=necko-reviewers,geckoview-reviewers,search-reviewers,valentin,m_kato
https://hg.mozilla.org/integration/autoland/rev/f1280b2e8a0a
Introduce a new API to create speculative connection with the partition key in originAttributes, r=necko-reviewers,valentin
https://hg.mozilla.org/integration/autoland/rev/3ee1184f75ef
Use origin attributes to start speculative connections for link header, r=necko-reviewers,timhuang,valentin

Comment 26

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/89a8aa4b8d28
https://hg.mozilla.org/mozilla-central/rev/f1280b2e8a0a
https://hg.mozilla.org/mozilla-central/rev/3ee1184f75ef