1338466 - Get rid of AudioChannelService::IsServiceStarted()

Status: RESOLVED DUPLICATE of bug 1338137

(Core :: DOM: Core & HTML, defect)

Description

[Andrea Marchesini [:baku]]

It's possible that the AudioChannelService starts for many reasons. We should remove that static method and introduce a real check about having or not active AudioChannelAgents.

Comment 1

[Alastor Wu [:alwu]]

Sounds good!

Comment 2

[Andrea Marchesini [:baku]]

Attachment: acs.patch

Comment 3

[Andrea Marchesini [:baku]]

Attachment: acs.patch

Comment 4

[Andrea Marchesini [:baku]]

Comment on attachment 8836735 [details] [diff] [review]
acs.patch

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

::: dom/audiochannel/AudioChannelService.cpp
@@ +206,2 @@
>  {
> +  if (gAudioChannelService || gAudioChannelService->mWindows.IsEmpty()) {

here it is: if (!gAudioChannelService || gAudioChannelService->mWindows.IsEmpty()) {

Comment 5

[Alastor Wu [:alwu]]

After survey the whole flow, we can't just use mAudibleAgents as a condition for resuming the tab, because we might have non-audible agents. In addition, we have a little complex case in blocking autoplay media...

---

Let me explain, we need to start from bug1319771.

The blocking media mechanism is rely on the outer window's visibility state, and it assumes if the tab is not in the foreground, that the tab MUST be invisible. When the tab becomes visible first time, we resume the tab via [1].

However, sometime even the tab isn't in the foreground, it might still be visible in some situations [2]. It causes a serious problem for us, because the visibility state is no more robust, we need to use extra condition to check whether we can resume the tab or not.

So I introduced the check AudioChannelService::IsServiceStarted(). The idea is to filter out the early visible state changed in [2], because we won't create the AudioChannelService before loading the media components in that page.

But it causes another problem. If the tab doesn't have any media component, the tab won't be resumed even it has gone to foreground. Because of that, I introduced another method to resume the tab [3] if there is any new incoming media wanna to play but foreground tab doesn't be resumed yet.

---

This patch would cause the media can't playing successfully if the media started after the tab became to foreground.

When the tab goes to foreground, there isn't any audio channel agent, so we don't resume the tab. And then the media wanna to play, it create audio channel agent and then we notify window there is new agent created [3]. However, the tab still won't be resumed because the the agent doesn't start yet, the mAgents and mAudibleAgent are all empty.

---

Conclusion, the present code can fix the bug1319771, but it's not best solution. The best solution for me is we can rely on document's visibility, but it seems can't be done.

Because of that, we need AudioChannelService::IsServiceStarted() but it makes thing becomes more complex. We need to find out other way if want to remove this method. 

[1] https://goo.gl/avJj4k
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=1319771#c7
[3] https://goo.gl/nBXUZ2

Comment 6

[Alastor Wu [:alwu]]

According to the bug1338137 comment5, close this bug.