Bug 247527 - [GTK] Creates a lot of MPRIS notifications with no reason
Summary: [GTK] Creates a lot of MPRIS notifications with no reason
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: Other
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-11-05 09:24 PDT by Yosef Or Boczko
Modified: 2024-03-23 10:00 PDT (History)
9 users (show)

See Also:


Attachments
Screencast for this bug (9.73 MB, video/webm)
2022-11-05 09:24 PDT, Yosef Or Boczko
no flags Details
Another screenshot, these notifications persist after the browser is closed (85.54 KB, image/png)
2022-11-29 16:27 PST, Michael Catanzaro
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Yosef Or Boczko 2022-11-05 09:24:59 PDT
Created attachment 463428 [details]
Screencast for this bug

- System Information
* ArchLinux stable repo
* Epiphany 43.0, WebKit 2.38.1
* GNOME Shell 43.0

I reported this issue in epiphany, and I was told it is a webkit's issue.

- Bug summary
When I browse the web in epiphany, some sites cause a lot of media notifications in gnome-shell (more than one per site). Some sites have no any media player at all.

- Steps to reproduce
1. Open Epiphany
2. Browse in site with media and listen to music (in youtube for example)
3. Open sites with no media, like https://rotter.net/, and then you see a lot of media notifications in gnome-shell


- What happened
A lot of media notifications were created, for no reason.

- What did you expect to happen
Only sites with media need media notifications, and only once per site.

See the attached screencast.
Comment 1 Michael Catanzaro 2022-11-29 16:27:23 PST
On top of this, the notifications are also somehow leaked. GNOME continues to display the notifications even long after Epiphany is closed. I'm attaching a screenshot as proof, showing three "Kellyanne Conway" MPRIS notifications left alive after I had previously visited https://www.cnn.com/2022/11/28/politics/kellyanne-conway-meeting-january-6-committee/index.html.

We should probably turn off MPRIS entirely until we have time to investigate this further, because the user experience is worse than no MPRIS. It's taking up too much space in the notification area.
Comment 2 Michael Catanzaro 2022-11-29 16:27:43 PST
Created attachment 463794 [details]
Another screenshot, these notifications persist after the browser is closed
Comment 3 Patrick Griffis 2022-11-29 20:58:32 PST
(In reply to Michael Catanzaro from comment #2)
> Created attachment 463794 [details]
> Another screenshot, these notifications persist after the browser is closed

So you have webkit processes still alive?

These aren't notifications, they are dbus services, they should disappear when the process dies.
Comment 4 Michael Catanzaro 2022-11-30 05:47:59 PST
(In reply to Patrick Griffis from comment #3)
> So you have webkit processes still alive?

Maybe, but that would imply that we have a web process leak. Well, users *have* been complaining about this happening, so I wouldn't be surprised....

I guess let's focus this bug on limiting MPRIS to (a) only appear when the page actually has real media that is actually playing, and (b) only once per page. Currently I have one Element tab open that is playing zero videos, but it has created five MPRIS notifications. All five notifications are broken: pressing the play button doesn't do anything. Who knows what media elements they supposedly correspond to.
Comment 5 Patrick Griffis 2022-11-30 09:23:24 PST
I think its reasonable to disable by default for now. I've also hit some of these issues.
Comment 6 Michael Catanzaro 2022-12-05 13:21:58 PST
Looks like MediaSessionManagerGLib is designed to own multiple MediaSessionGLibs at once, and each MediaSessionGLib creates a MPRIS notification for as long as it exists. This was redesigned in bug #230250 and has probably been broken since then, but obscured by the D-Bus name issues.
Comment 7 Michael Catanzaro 2022-12-05 13:31:49 PST
So I think MediaSessionManagerGLib needs to track which media session is the best, and tell each MediaSessionGLib that it owns whether to register or unregister its MPRIS interface accordingly. That should get us down to only one broken notification per tab. (Currently I have 10 broken MPRIS notifications, five for each of two Element tabs, and seeing 2 broken notifications is a lot better than 10.)

That won't fully solve the bug, of course, because we should not show any broken MPRIS notifications. But one thing at a time.
Comment 8 Michael Catanzaro 2022-12-13 15:06:51 PST
(In reply to Michael Catanzaro from comment #4)
> Maybe, but that would imply that we have a web process leak. Well, users
> *have* been complaining about this happening, so I wouldn't be surprised....

OK, I noticed today that I had a bunch of MPRIS notifications hanging around even though no applications using WebKit were running, and finally checked this out. I noticed three different hung web processes, alive after the UI process exited. I killed them using core signals and examined the backtrace using flatpak-coredumpctl. Two of them were the same bug, which I'll report in bug #247057. The other one was a different bug; I didn't properly save that one, and will have to report it separately next time I see it.

So let's handle the web process hangs elsewhere, and save the rest of this bug report for dealing with MPRIS. I don't have time to investigate and fix things properly right now, but I could post a hack patch to just sabotage MediaSessionGLib and avoid the immediate problem. Carlos Garcia doesn't generally like it when I do that, though. :)
Comment 9 Philippe Normand 2022-12-14 02:00:03 PST
I don't see how the ImageDecoder would trigger MPRIS notifications TBH.
Comment 10 Michael Catanzaro 2022-12-14 06:43:16 PST
(In reply to Philippe Normand from comment #9)
> I don't see how the ImageDecoder would trigger MPRIS notifications TBH.

It doesn't.

There are three problems discussed in this bug report:

1. Bug #247057 is causing the MPRIS notifications to survive after the UI process quits. We can worry about that separately.

2. From comment #7: "I think MediaSessionManagerGLib needs to track which media session is the best, and tell each MediaSessionGLib that it owns whether to register or unregister its MPRIS interface accordingly. That should get us down to only one broken notification per tab."

3. Don't show any broken MPRIS notifications in the first place. If the rewind, play, fast forward buttons do not work, just don't show it. I have 9 MPRIS notifications currently, presumably corresponding to unknown media elements, and none of the buttons do anything.

Might need to take heuristics from other browsers, like Firefox.
Comment 11 Philippe Normand 2023-01-07 06:52:57 PST
I think the best short-term thing to do, so close to the release, is to add a websetting for this feature and disable it by default...
Comment 12 Michael Catanzaro 2023-01-07 08:07:27 PST
Makes sense, but note this feature has been enabled by default since 2.36.4 (241579@main) so I suppose we should disable it on the stable branch too.
Comment 13 Philippe Normand 2023-01-07 08:10:15 PST
(In reply to Michael Catanzaro from comment #12)
> Makes sense, but note this feature has been enabled by default since 2.36.4
> (241579@main)

As experimental feature.

It was truly enabled by default only a couple months ago afaik.
Comment 14 Philippe Normand 2023-01-07 08:11:48 PST
(In reply to Philippe Normand from comment #13)
> (In reply to Michael Catanzaro from comment #12)
> > Makes sense, but note this feature has been enabled by default since 2.36.4
> > (241579@main)
> 
> As experimental feature.
> 
> It was truly enabled by default only a couple months ago afaik.

See 250153@main
Comment 15 Michael Catanzaro 2023-01-07 11:58:46 PST
Oh you're right, but that's still in 2.38. I will push a commit there to disable it.
Comment 16 Michael Catanzaro 2023-01-07 12:03:16 PST
So Phil created https://github.com/WebKit/WebKit/pull/8358 for main. And I landed https://github.com/WebKit/WebKit/commit/9a224f3ed860757adc2f8ee13423b4dd45c78737 for 2.38.
Comment 17 Alex 2024-03-23 00:30:43 PDT
Pull request: https://github.com/WebKit/WebKit/pull/26367
Comment 18 EWS 2024-03-23 09:59:30 PDT
Committed 276591@main (5e2b534ceb82): <https://commits.webkit.org/276591@main>

Reviewed commits have been landed. Closing PR #26367 and removing active labels.
Comment 19 Radar WebKit Bug Importer 2024-03-23 10:00:16 PDT
<rdar://problem/125293330>