Closed Bug 1632301 Opened 3 months ago Closed 3 months ago

Use current browsing context to propagate the controlled media state

Categories

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

task

Tracking

()

RESOLVED FIXED
mozilla77
Tracking Status
firefox77 --- fixed

People

(Reporter: alwu, Assigned: alwu)

References

(Blocks 1 open bug)

Details

Attachments

(4 files)

Currently, when we propagate any controlled media related states, we would find the top level browsing context first, then pass it through IPC in order to get the correct media controller in the chrome process.

However, we have implemented [1] which can find the correct media controller even if we are not passing the top level browsing context.

In addition, in bug1627999, we would like to know which browsing context those states come from. Therefore, we should replace the top browsing context with the current browsing context where controlled media exists.

[1] https://searchfox.org/mozilla-central/rev/41c3ea3ee8eab9ce7b82932257cb80b703cbba67/docshell/base/CanonicalBrowsingContext.cpp#511-515

This patch will do :

  • no longer inherit from MediaControlKeysEventListener/MediaControlKeysEventSource and manually implement the method we needs

The avantage of doing so :

  • increase the flexibilty of modification of ContentMediaAgent and those modification won't be used for other MediaControlKeysEventListener

More details about this change :

As we would like to extend the class ContentMediaAgent and allow the ContentMediaController to call its extended method, but if we want to do so in current implementation, the extended method would also affect other classes inherited from MediaControlKeysEventListener and that is something we don't want to see.

Considering that, I decided to decouple the inheritance relationship and manually create the function I need (which will be implemented in the next patch)

This patch will do :

  • use current broswing context as a parameter when propagate state change to the chrome process.

The avantage of doing so :

  • the chrome process can know which browsing context the state change actually comes from.

More details about this change :

Currently, when we propagate any controlled media related states, we would find the top level browsing context first, then pass it through IPC in order to get the correct media controller in the chrome process. However, we have implemented [1] which can find the correct media controller even if we are not passing the top level browsing context.

In addition, in bug1627999, we would like to know which browsing context those states come from. Therefore, we should replace the top browsing context with the current browsing context where controlled media exists.

[1] https://searchfox.org/mozilla-central/rev/41c3ea3ee8eab9ce7b82932257cb80b703cbba67/docshell/base/CanonicalBrowsingContext.cpp#511-515

This patch will do :

  • replace boolean with enum class MediaAudibleState

The avantage of doing so :

  • It's easier to understand what actually meaning of the parameter we set

This patch will do :

  • rename ControlledMediaState to MediaPlaybackState

The avantage of doing so :

  • more consistent with MediaAudibleState
Attachment #9142559 - Attachment description: Bug 1632301 - rename 'ControlledMediaState' to 'MediaPlaybackState'. → Bug 1632301 - part4 : rename 'ControlledMediaState' to 'MediaPlaybackState'.
Blocks: 1633010
Pushed by alwu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/48ef712d8017
part1 : decouple ContentMediaController from MediaControlKeysEventListener/MediaControlKeysEventSource. r=bryce
https://hg.mozilla.org/integration/autoland/rev/636b863fce4b
part2 : use `ContentControlKeyEventReceiver`'s browsing context to notify any changes. r=bryce
https://hg.mozilla.org/integration/autoland/rev/ddf995c1bb1d
part3 : use MediaAudibleState to replace boolean value. r=bryce
https://hg.mozilla.org/integration/autoland/rev/55ac8f5b9472
part4 : rename 'ControlledMediaState' to 'MediaPlaybackState'. r=bryce
You need to log in before you can comment on or make changes to this bug.