1421708 - Video or audio continue playing if tab is closed

Status: RESOLVED FIXED

(Firefox for iOS :: General, defect, P2)

Description

[Andreas Wagner [:TheOne] [use NI]]

STR:

1. Go to https://www.dailymotion.com/video/x60csed (other video websites may work as well, but youtube or vimeo don't)
2. Start playing the video.
3. Close the tab.

Expected result:

The audio stops playing.

Actual result:

The audio keeps playing.

--

The audio keeps playing even if you go back to the home-screen after step 3. It stops playing if you kill Firefox from the app-switcher.

Device: iPhone X
Firefox version: 10.3 (7866)

Comment 1

[Justin D'Arcangelo [:justindarc]]

Garvin, did we fix this?

Comment 2

[:garvan]

No ...and wow, the audio keeps playing regardless of how many tabs opened and closed.

Comment 3

[Justin D'Arcangelo [:justindarc]]

Can you investigate?

Comment 4

[Simon Basca [:SimonB]]

I manage to reproduce this issue on Firefox 10.3 (7866) on iPhone 8 (11.2) very easily. Also manage to reproduce this on beta 11.0 (8792) after following these steps:
1. Clean install beta 11.0 (8792)
2. Go to https://www.dailymotion.com/video/x60csed
3. Start playing the video.
4. Close the tab.

The audio keeps playing. After closing and restarting beta 11.0 (8792), repeating the STR will no longer reproduce the issue. After going to Settings, tapping "Clear Private Data", repeating the STR will reproduce again the issue, once.

I din't manage to reproduce this on 10.5 (8741) release.

Comment 5

[Justin D'Arcangelo [:justindarc]]

Check if patch for v10.x landed on master/v11.x

Comment 6

[:garvan]

Confirmed landed, the tab retain bug landed on 10.4 and master:
https://bugzilla.mozilla.org/show_bug.cgi?id=1419057

Comment 7

[Richard Newman [:rnewman]]

Still happening in current TestFlight (11?). 

I just had to force-quit the app to stop NPR from playing. Not a good user experience. 

This is tracking-11, so presumably more than a P3…!

Comment 8

[Justin D'Arcangelo [:justindarc]]

Re-noming per comment 7.

Comment 9

[:garvan]

We use AVAudioSession to handle background audio, but we have no way to message it that I can see to tell it to stop playing:
https://github.com/mozilla-mobile/firefox-ios/blob/master/Client/Application/AppDelegate.swift#L116

This is used in conjunction with the UIBackgroundMode `audio` in the plist.

At a minimum, we should remove AVAudioSession handoff until we have a full implementation of this.
Currently, YouTube will not play in background, so the UI feels inconsistent that some video sites play in background, but YouTube does not. YouTube did play in the background on various browsers (I recall 2 years ago some browsers supporting this), but all 3rd party browsers (Puffin, Dolphin, Maxthon, us) fail to play youtube in the background. Opera does however.

Comment 10

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

This is extremely simple to reproduce on npr.org. Just open a tab and play a news station in it. (audio only). When you close the tab, it keeps playing.

Comment 11

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

:garvan Are you sure we need to manage anything manually?

The WKWebView owns the media content. If the WKWebView is destroyed, it should take down the audio stream that it owns.

If audio keeps playing after we close a tab, it means something is still holding a refernece to the tab or we are looking at a WKWebView bug?

We can rule out UIBackgroundMode for now since I can reproduce this without ever backgrounding the app. See previous comment.

Comment 12

[:garvan]

Removing this AVAudioSession handoff line and I can't repro the bug https://github.com/mozilla-mobile/firefox-ios/blob/master/Client/Application/AppDelegate.swift#L116

I am not familiar with AVAudioSession, but how would it know to stop playing?

Comment 13

[:garvan]

> If audio keeps playing after we close a tab, it means something is still
> holding a refernece to the tab or we are looking at a WKWebView bug?

If don't have a test case for this, we should. We don't guard against strongly accessing the tab's webview, so a missing `weak` on a long-lived object could result in holding on to the webview. Same thing with the Tab object.

Comment 14

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Or we are looking at a change in behaviour in WKWebView around media playing.

This code you linked to has been in the product for a really long time. For the purpose of being able to mix accessbiity audio effects with other sound sources in the app.

We do not actually have those accessibility sound sources anymore, so we can probably remove this code, but it is suspicious that this was not always a problem.

Maybe ask QA to test this on 10.x and 11.x and see if WKWebView handles things different?

Comment 15

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

I have a 6+ with iOS 10.2.5 so that was easy to try: both iOS 10.2.5 and 10.3 have this same bug.


Removing the code at

  https://github.com/mozilla-mobile/firefox-ios/blob/master/Client/Application/AppDelegate.swift#L116 

makes the bug go away.

Comment 16

[:garvan]

> We do not actually have those accessibility sound sources anymore, so we can
> probably remove this code, but it is suspicious that this was not always a
> problem.

Unless we have a test case for this, I am not sure how we can assert that.

> Maybe ask QA to test this on 10.x and 11.x and see if WKWebView handles
> things different?

The bug was reported on 10.3 some months ago, and still happens on 11.x, what is your question for them?

Wdyt about these steps:
1) Remove AVAudioPlayer handoff code (confirming it has no detriment to do so)
2) Add TabManager test case to ensure object counts go to zero for Tab and WKWebView (we sort-of have this, but we are using high-level counts, we need to assert actual reference counts go to zero)

Comment 17

[:garvan]

Attachment: Pull request

Comment 18

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Mixed results here with the `setCategory(AVAudioSessionCategoryPlayback)` code removed:

1) Create a build that has the `setCategory(AVAudioSessionCategoryPlayback)` removed
2) Install a clean copy to device
3) Open Firefox, go to https://www.npr.org/stations/
4) Play the WXXI-AM News stream
5) Open the tab manager, you should have _one_ tab (NPR)
6) Close the NPR tab

Expected: Audio stops playing.

Actual: Audio is still playing

Comment 19

[:garvan]

Initially looking at this bug, I thought it was 100% reproducible, but now have found it is intermittent. For instance your latest STR: those don't repro for me. However, repeatedly testing the dailymotion case results in sometimes it continuing to play.

Following Stefan's suggestion, let's land this patch for 11.1 to remove our custom background audio behaviour, and keep investigating to see if reproducibility has changed.

I filed a follow up bug already for strange tab deletion behaviour in our tab manager. It could be a 2 factor bug, and that is contributing to the behaviour.

Comment 20

[:garvan]

If we can verify that removing `audio` UIBackgroundMode allows for AirPlay broadcast we should remove that flag also. 

(I don't have an AirPlay device to verify, but the UI in iOS for AirPlay it seems to show up ok for Firefox iOS as a sound source with that flag removed.)

Comment 21

[:garvan]

Landed PR to remove AVAudioSession on master, and on v11.x (0041531a0b96d82213a2a4feac2005ce4f5c6dd7).

Bug is not fixed. We are still investigating more causes of this.

Comment 22

[Farhan Patel [:farhan]]

Attachment: Pull Request

Comment 24

[Farhan Patel [:farhan]]

merged and uplifted

Comment 25

[Catalin Suciu [:csuciu]]

I'm no longer able to reproduce this exact bug using the latest beta - 11.1(10377) - but I see a slightly different manifestation of it when using multiple tabs - see bug #1457082