1554297 - GeckoSession.MediaDelegate.onMediaAdd() not getting called on soundcloud.com

Status: RESOLVED MOVED

(GeckoView :: Media, defect, P2)

Description

[Sebastian Kaspari (:sebastian; :pocmo)]

When playing audio on soundcloud.com then GeckoSession.MediaDelegate.onMediaAdd() does not get invoked. Without that we can't show a media notification and not control audio playback for that site.

GeckoView Nightly: 69.0.20190522093426

Comment 1

[Chris Peterson [:cpeterson]]

Emily said she would investigate. If this is a tricky bug, we can probably defer this bug until after Fenix MVP.

Comment 2

[Emily Toop (:fluffyemily)]

I think we are hitting this specialisation use case as on soundcloud.com BindToTree is never called. What seems to be going wrong is that the other UAWidgetSetupOrChange event is not sent. Investigating why.

https://searchfox.org/mozilla-central/source/toolkit/content/widgets/docs/ua_widget.rst#26

**Specialization**: for video controls, we do not want to do the work if the control is not needed (i.e. when the ``<video>`` or ``<audio>`` element has no "controls" attribute set), so we forgo dispatching the event from HTMLMediaElement in the BindToTree method. Instead, another ``UAWidgetSetupOrChange`` event will cause the sandbox and the widget instance to construct when the attribute is set to true. The same event is also responsible for triggering the ``onchange()`` method on UA Widgets if the widget is already initialized.

Comment 3

[Emily Toop (:fluffyemily)]

:imanol, :rbarker suggested that you might be able to help me on this one.

GeckoView::OnMediaAdd is fired after receiving the UAWidgetSetupOrChange event, which is sent from the Element. https://searchfox.org/mozilla-central/source/dom/base/Element.cpp#1268

For the HTMLMediaElement this is usually sent from a call to BindToTree (https://searchfox.org/mozilla-central/source/dom/html/HTMLMediaElement.cpp#4075), but I can also see that if we call AfterSetAttr with the name nsGkAtoms::controls it can also be called (https://searchfox.org/mozilla-central/source/dom/html/HTMLMediaElement.cpp#4037).

When playing media on bandcamp.com, BindToTree is called is and the UAWidgetSetupOrChange event is sent.

But on soundcloud.com neither BindToTree or AfterSetAttr with a name of nsGkAtoms::controls are called at any time (although AfterSetAttr is called several times with the names nsGkAtoms::preload, nsGkAtoms::msaudiocategory and nsGkAtoms::crossorigin, whereas these are not called on bandcamp.com).

I have found a comment in ua_widget.rst (https://searchfox.org/mozilla-central/source/toolkit/content/widgets/docs/ua_widget.rst#26) that suggests that there is a special case where UAWidgetSetupOrChange is sent via a different method, but the comment gives no clue as to how the event is sent in this case. I wonder if we are hitting this specialization and the alternative event send is failing.

Do you know anything about this? If you could give me any hints as to where I might want to look for the cause of this I would be really grateful.

Comment 4

[Emily Toop (:fluffyemily)]

After chatting with :imanol it seems as if the problem is that on soundcloud.com the HTMLMediaElement is being played from JS (possibly on a WebGL canvas) and is therefore not being added to DOM. As it is the adding of the element to DOM that will cause OnMediaAdd to be called it will not be called in this case.

If this is the case the solution would be to detect the element being added to the WebGL canvas and fire the event from there.

Comment 5

[Sebastian Kaspari (:sebastian; :pocmo)]

Not sure if helpful, but I noticed that Fennec is showing a media notification for soundcloud - is Fennec using a different mechanism?

Comment 6

[Chris Peterson [:cpeterson]]

Sebastian says Fenix doesn't use the media notifications yet, so this bug doesn't need to block Fenix MVP.

Comment 7

[Sebastian Kaspari (:sebastian; :pocmo)]

The media notification work is nearing completion in AC and we just enabled it in Fenix Nightly builds. This issue is probably going to become more important soon.

Comment 8

[Chris Peterson [:cpeterson]]

(In reply to Sebastian Kaspari (:sebastian; :pocmo) from comment #7)

The media notification work is nearing completion in AC and we just enabled it in Fenix Nightly builds. This issue is probably going to become more important soon.

In that case, adding [geckoview:fenix:m9] whiteboard tag so the GV team schedules this work soon.

Comment 9

[Chris Peterson [:cpeterson]]

Fenix issue: https://github.com/mozilla-mobile/fenix/issues/2551

Comment 11

[Sawyer Blatz [:sblatz]]

Do we have any update on the status of this? :) This is still something we're seeing reported semi-frequently on the Fenix side.

Comment 12

[Emily Toop (:fluffyemily)]

No update on this at this time. This issue has not been bumped up the priority list far enough to yet make it onto a release plan. If this is important, then we can up the priority so that it is looked at soon.

Comment 13

[[ex-Mozilla] Agi Sferro | :agi]

Attachment: testcase.html

Minimal testcase

Comment 14

[[ex-Mozilla] Agi Sferro | :agi]

Looks like this is caused by a HTMLMediaElement not attached to the document (as Emily and Imanol had implied up in the thread). I can see the call to ::Play in here having IsInComposedDoc = false . See also testcase.

jya: I see in https://phabricator.services.mozilla.com/D9026#270620 you recommended using UAWidgetBindToTree for this, but I don't think it can work for this testcase? do you have any recommendation on what we could use to detect media elements in this case?

Comment 15

[Jean-Yves Avenard [:jya]]

(In reply to :Agi | ⏰ PST | he/him from comment #14)

Looks like this is caused by a HTMLMediaElement not attached to the document (as Emily and Imanol had implied up in the thread). I can see the call to ::Play in here having IsInComposedDoc = false . See also testcase.

jya: I see in https://phabricator.services.mozilla.com/D9026#270620 you recommended using UAWidgetBindToTree for this, but I don't think it can work for this testcase? do you have any recommendation on what we could use to detect media elements in this case?

I wasn't the one making that suggestion :)

What I did say was that if a media element isn't attached to the dom, it won't be visible. Not sure where the geckosession has plugged itself to fire that call, but it seems that you're on the right track.

Comment 16

[[ex-Mozilla] Agi Sferro | :agi]

I'm starting to think we should just implement this using MediaControlKeysEventSource and using the metadata from the Media Session API. https://searchfox.org/mozilla-central/rev/4d2a9d5dc8f0e65807ee66e2b04c64596c643b7a/dom/media/mediacontrol/MediaControlKeysEvent.h#70

Comment 17

[Alastor Wu [:alwu]]

FYI, I've filed bug1623715 for supporting platform level MediaControl and MediaSession API implementation on GeckoView, and we've enabled that on MacOS, Windows and Linux on Nightly. We're glad to see if anyone from GeckoView team is willing to help :D

Comment 18

[Kevin Brosnan [Ex-Mozilla]]

bug 1623715 has not fixed this. Is there more on the Gecko(view) side or is there A-C/Fenix work now to plumb the new APIs?

Comment 19

[[ex-Mozilla] Agi Sferro | :agi]

:kbrosnan AC needs to use the new MediaSession API.

Comment 20

[[ex-Mozilla] Agi Sferro | :agi]

I see the AC change went in, looks like this works now!

Comment 21

[Sebastian Kaspari (:sebastian; :pocmo)]

👍

Comment 22

[Chris Peterson [:cpeterson]]

Moving some media bugs to the new GeckoView::Media component.