Bugfix FXIOS-10335 Fix audio/video leak after closing tab

mozilla-mobile / firefox-ios

Firefox for iOS

Mozilla Public License 2.0

12.24k stars 2.94k forks source link

Bugfix FXIOS-10335 Fix audio/video leak after closing tab #23213

Closed mattreaganmozilla closed 2 days ago

mattreaganmozilla commented 3 days ago

:scroll: Tickets

Jira ticket Github issue

:bulb: Description

⚠️ For additional notes please see the linked Jira. This is a quick initial fix. I'm hoping to get extra eyes and regression testing on this. We are also still leaking a Tab object however I believe we can live with that if needed for v133, the critical leak is the WKWebView which causes audio/video for closed tabs to continue playing.

: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
[ ] Wrote unit tests and/or ensured the tests suite is passing
[ ] When working on UI, I checked and implemented accessibility (minimum Dynamic Text and VoiceOver)
[ ] If needed, I updated documentation / comments for complex code and public methods
[ ] If needed, added a backport comment (example @Mergifyio backport release/v120)

mattreaganmozilla commented 3 days ago

@PARAIPAN9 If you have time tomorrow (Tues) let's sync up and discuss further if possible.

PARAIPAN9 commented 3 days ago

@PARAIPAN9 If you have time tomorrow (Tues) let's sync up and discuss further if possible. @mattreaganmozilla, definitely!!

mattreaganmozilla commented 2 days ago

@PARAIPAN9 Thank you! I'll ping you shortly, if possible let's sync on Slack today. I haven't been able to find any issues with this change yet but I'm going to add a Do Not Merge until the team confirms how we want to tackle this for v133/v134.

mattreaganmozilla commented 2 days ago

Note: I've filed a separate Jira for the leaked Tab object mentioned in the PR description.

mattreaganmozilla commented 2 days ago

Going to kick off one additional Bitrise run as a sanity check.

mobiletest-ci-bot commented 2 days ago

	Messages
:book:	Project coverage: 33.04%
:book:	Edited 1 files
:book:	Created 0 files

Client.app: Coverage: 31.12

File	Coverage
BrowserViewController.swift	4.75%	⚠️

Generated by :no_entry_sign: Danger Swift against 719af83991038e03871c76f24249f84348a9c36f

mattreaganmozilla commented 2 days ago

@Mergifyio backport release/v133

mergify[bot] commented 2 days ago

backport release/v133

✅ Backports have been created

* [#23243 Bugfix FXIOS-10335 Fix audio/video leak after closing tab (backport #23213)](https://github.com/mozilla-mobile/firefox-ios/pull/23243) has been created for branch `release/v133`

mattreaganmozilla commented 2 days ago

Per team discussion, going to merge to main and continue testing. Backport PR for v133 will available if we decide to roll this into next RC. (cc @DonalMe)