Closed Bug 1606782 Opened 3 months ago Closed 2 months ago

Keep media controller's life cycle as the same as its corresponding top-level browsing context

Categories

(Core :: Audio/Video: Playback, task, P2)

task

Tracking

()

RESOLVED FIXED
mozilla74
Tracking Status
firefox74 --- fixed

People

(Reporter: alwu, Assigned: alwu)

References

(Blocks 1 open bug)

Details

Attachments

(5 files, 1 obsolete file)

As each media controller corresponds to a tab (a browsing context tree), so the controller's life cycle should be equal to the tab. Currently we use the top-level browsing context to represent a tab, so when that browsing context is being destroyed, we should also destroy the corresponding media controller.

That can fix some problems,

(1) to make sure all controllers in MediaControlSerivce are valid
We add media controller to the service when it becomes active, and remove it from the service when it becomes inactive.

However, in order to inactivate a controller, we need to receive the notification from the content process, which is used to update the controlled media number. But this notification might fail to sent in some cases [1]. If the corresponding browsing context has been discarded, then we couldn't receive the correct update to remove the media controller from the service.

Therefore, if the life of media controller is controlled by its corresponding context, then we can ensure to remove the controller from the service when the browsing context is going to be destroyed.

[1] https://searchfox.org/mozilla-central/rev/053826b10f838f77c27507e5efecc96e34718541/dom/media/mediacontrol/MediaControlUtils.cpp#56-58

(2) to avoid unneccesary creation and destruction of the media controller
Currently the controller would be destroyed when it become inactive (no controlled media existing), and it might be recreated again later if we have other new created controlled media.

We should reuse same media controller all the time until the tab is going to be destroyed.

(3) to support the implementation of bug1592037

In order to support adding metadata on the media controller, we have to keep media controller alive even if we don't start any controlled media. Current implementation can only keep media controller alive when it has controlled media, but the metadata can be set before having any controlled media.

Therefore, we have to change the controller's life cycle in order to store metadata before any controlled media starts.

Attachment #9118466 - Attachment description: Bug 1606782 - part3 : create media controller via Browsing Context. → Bug 1606782 - part3 : create media controller via canonical browsing context.
Attachment #9118466 - Attachment description: Bug 1606782 - part3 : create media controller via canonical browsing context. → Bug 1606782 - part2 : create media controller via canonical browsing context.
Attachment #9118467 - Attachment is obsolete: true

We should ensure that controllers has been removed from the service when it destroys. Therefore, we remove the assertion in MediaControlService::RegisterActiveMediaController() and MediaControlService::UnregisterActiveMediaController(), and return the result to allow caller to know if it has been registered or unregistered or not in order to do error handling or add assertions.

Attachment #9118740 - Attachment description: Bug 1606782 - part4 : decouple canonical browsing context and audio focus manager. → Bug 1606782 - part4 : make audio focus manager independent from other components.
Pushed by alwu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1b9a35a629b3
part1 : rename controller related functions and member to emphasize that only active controller would be added into the controller list in the service. r=chunmin,bryce
https://hg.mozilla.org/integration/autoland/rev/f41cb58b7afc
part2 : create media controller via canonical browsing context. r=chunmin,nika
https://hg.mozilla.org/integration/autoland/rev/45684b8d59e1
part3 : replace assertion with returning result of register/unregister controller r=chunmin,bryce
https://hg.mozilla.org/integration/autoland/rev/20e34fd5fa51
part4 : make audio focus manager independent from other components. r=chunmin,bryce
https://hg.mozilla.org/integration/autoland/rev/4e4e597e9281
part5 : handle audio competing first r=chunmin,bryce
Regressions: 1610158
You need to log in before you can comment on or make changes to this bug.