Closed Bug 1426474 Opened 4 years ago Closed 4 years ago

Turn on TFO for MacOS but only for Darwin 17.3 and later

Categories

(Core :: Networking: HTTP, enhancement, P1)

59 Branch
enhancement

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: dragana, Assigned: dragana)

Details

(Whiteboard: [necko-triaged])

Attachments

(1 file, 1 obsolete file)

The latest version of macOS has beter detection of the misbehavior of the network with TFO. So we will enable TFO only for Darwin 17.3.
Attached patch bug_turn_on_tfo_on_osx.patch (obsolete) — Splinter Review
Attachment #8938116 - Flags: review?(hurley)
Comment on attachment 8938116 [details] [diff] [review]
bug_turn_on_tfo_on_osx.patch

Review of attachment 8938116 [details] [diff] [review]:
-----------------------------------------------------------------

The condition needs updating, but otherwise this looks fine. Also, one nit: should the commit message say "17.3 and later"? (Not that there is a "later" right now, but there will be at some point, and I presume it will stay on when there is.)

::: modules/libpref/init/all.js
@@ +4781,5 @@
>  #if defined(XP_UNIX) && !defined(XP_MACOSX)
>  pref("network.tcp.keepalive.probe_count", 4);
>  #endif
>  
> +#if defined(XP_WIN) && defined(XP_MACOSX)

This should be || not &&. Unless something strange has happened that I missed the news about, and windows has merged with os x :-D
Attachment #8938116 - Flags: review?(hurley) → review+
(In reply to Nicholas Hurley [:nwgh][:hurley] (also hurley@todesschaf.org) from comment #2)
> Comment on attachment 8938116 [details] [diff] [review]
> bug_turn_on_tfo_on_osx.patch
> 
> Review of attachment 8938116 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> The condition needs updating, but otherwise this looks fine. Also, one nit:
> should the commit message say "17.3 and later"? (Not that there is a "later"
> right now, but there will be at some point, and I presume it will stay on
> when there is.)
> 

Yes.

> ::: modules/libpref/init/all.js
> @@ +4781,5 @@
> >  #if defined(XP_UNIX) && !defined(XP_MACOSX)
> >  pref("network.tcp.keepalive.probe_count", 4);
> >  #endif
> >  
> > +#if defined(XP_WIN) && defined(XP_MACOSX)
> 
> This should be || not &&. Unless something strange has happened that I
> missed the news about, and windows has merged with os x :-D

It is time for holidays... :)
Summary: Turn on TFO for MacOS but only for Darwin 17.3 → Turn on TFO for MacOS but only for Darwin 17.3 and later
Attachment #8938116 - Attachment is obsolete: true
Attachment #8938439 - Flags: review+
Pushed by dd.mozilla@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3aa13bbb2a99
Turn on TFO only for Darwin 17.3 and later. r=hurley
https://hg.mozilla.org/mozilla-central/rev/3aa13bbb2a99
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.