Bug 21116 – Audio from <audio> or <video> element continues after tab is closed

• Component: New Bugs
• Status: RESOLVED FIXED

[Eric Carlson]

If a page with an <audio> or <video> element has been in the page cache, audio continues to play for several seconds after the tab with the element is closed.

To reproduce:
1. Open Safari
2. Open two blank tabs
3. In one tab, open a page with a media element that has audio.
4. Go to http://webkit.org/ in the tab with the media element
5. After the page loads, click the Back button
6. Start the media element playing and close the tab

Audio continues to play for a few seconds.

[Eric Carlson]

Created attachment 23816 [details]
proposed patch

[Adele Peterson]

the basic concept seems ok to me.  I'd like to get comments from Hyatt or Brady.  CC'ing them.

[Adele Peterson]

Another option is to add the notification to Frame::pageDestroyed-- if we didn't want to change the willSaveToCache/didRestoreFromCache concept.

[Eric Carlson]

FWIW, Antti suggested this approach. CC'ing him as well.

[Antti Koivisto]

I noticed that there are still some references to page cache left in connection with these callbacks. I see these in the patch:

Document::m_pageCacheCallbackElements
HTMLInputElement ::needsCacheCallback()
HTMLMediaElement::m_inPageCache

For consistency it would be good to rename them as well and check that semantics still make sense (that is they really care about whether document is active or not rather than specifically about being in page cache or not). At least something like HTMLMediaElement::m_inActiveDocument makes total sense. 

[Eric Carlson]

Created attachment 23851 [details]
revised patch

Revised patch per Antti's comments.

[Antti Koivisto]

Comment on attachment 23851 [details]
revised patch

This should still be renamed to match variable name for consistency:

bool inPageCache() const { return !m_inActiveDocument; }

It should return the value without negation and call sites and should be reversed instead. (or alternatively find a good variable name that is true when inactive, m_inInactiveDocument sounds bad).

This is minor, r=me with or without the change.

[Eric Carlson]

Revised per last comment and committed as r36957.

[Simon Fraser (smfr)]

See also bug 24031.