firefox-ios icon indicating copy to clipboard operation
firefox-ios copied to clipboard

Published 20 hours ago • verified publisher icon mozilla-mobile

Refactor FXIOS-21470 - Add Multipath TCP (MPTCP) support

Open Aperence opened this issue 1 year ago • 4 comments

MPTCP is a TCP extension allowing to improve network reliabilty by using mutiple network interface for the same TCP connection. Check https://www.mptcp.dev and https://developer.apple.com/documentation/foundation/urlsessionconfiguration/improving_network_reliability_using_multipath_tcp for details. Changes to this repository includes introducing new configurations (defaultMPTCP/ephemeralMPTCP), replacing uses of default/ephemeral by these new configurations, introducing a sharedMPTCP property on URLSession and modifying the makeURL function to enable the handover mode by default

:scroll: Tickets

Github issue

:bulb: Description

:pencil: Checklist

You have to check all boxes before merging

  • [x] Filled in the above information (tickets numbers and description of your work)
  • [X] Updated the PR name to follow our PR naming guidelines
  • [X] Wrote unit tests and/or ensured the tests suite is passing
  • [X] When working on UI, I checked and implemented accessibility (minimum Dynamic Text and VoiceOver)
  • [X] If needed, I updated documentation / comments for complex code and public methods
  • [X] If needed, added a backport comment (example @Mergifyio backport release/v120)

Aperence avatar Aug 07 '24 09:08 Aperence

Thanks for this PR @Aperence. Might be good to get some extra eyes on this (cc @nbhasin2).

mattreaganmozilla avatar Aug 07 '24 18:08 mattreaganmozilla

image

@Aperence I see the following tests failing, would you mind looking into it and let us know if any support is required

nbhasin2 avatar Aug 27 '24 15:08 nbhasin2

Hello, I've fixed the issues with these tests normally

Aperence avatar Aug 27 '24 16:08 Aperence

Messages
:book: Project coverage: 31.56%
:book: Edited 18 files
:book: Created 2 files

Client.app: Coverage: 30.09

File Coverage
BrowserViewController.swift 5.07% ⚠️
TemporaryDocument.swift 0.0% ⚠️
URLSessionConfiguration+defaultMPTCP.swift 100.0% ✅
PocketProvider.swift 86.21% ✅
WallpaperNetworkModule.swift 100.0% ✅
DownloadQueue.swift 50.15% ✅
BreachAlertsClient.swift 0.0% ⚠️
ActionProviderBuilder.swift 0.0% ⚠️
SearchSuggestClient.swift 72.45% ✅
ContileProvider.swift 90.91% ✅
OpenPassBookHelper.swift 0.0% ⚠️

Generated by :no_entry_sign: Danger Swift against ea7dc4825dff457cd780d8d8da93ce8e2d7e21fd

mobiletest-ci-bot avatar Aug 27 '24 16:08 mobiletest-ci-bot

This pull request has conflicts when rebasing. Could you fix it @Aperence? 🙏

mergify[bot] avatar Sep 06 '24 14:09 mergify[bot]

Rebased it normally !

Aperence avatar Sep 06 '24 16:09 Aperence

This PR has been automatically marked as stale. Please leave any comment to keep this PR opened. It will be closed automatically if no further update occurs in the next 7 days. Thank you for your contributions!

github-actions[bot] avatar Sep 21 '24 00:09 github-actions[bot]

Any news about this ?

Aperence avatar Sep 21 '24 07:09 Aperence

Just applied the requested change. It probably isn't enabled by default because some time ago, some other options weren't (well) supported with MPTCP (from what I know), and they probably never reconsidered up to now to enable it by default.

Aperence avatar Sep 25 '24 14:09 Aperence

Running BR and if all looks well I can merge it in 🤞🏼

nbhasin2 avatar Sep 27 '24 01:09 nbhasin2

@Aperence might wanna rebase this one last time as you have approvals but there are certain number of warnings that are more on this PR compared to main.

Thanks and once green I can hit merge

nbhasin2 avatar Sep 27 '24 18:09 nbhasin2

Just rebased it now, hope it will fix the warnings

Aperence avatar Sep 30 '24 06:09 Aperence