[GTK] Creates a lot of MPRIS notifications with no reason

https://bugs.webkit.org/show_bug.cgi?id=247527 Reviewed by Philippe Normand. At any given time, there should be only one active MPRIS session. MediaSessionManagerGLib would not unregister inactive MPRIS sessions once a new one became the current session. This patch ensures that once a new current session is set, all others both unregistered and marked as ineligible for future registration on status changes. * Source/WebCore/platform/audio/glib/MediaSessionGLib.cpp: (WebCore::MediaSessionGLib::~MediaSessionGLib): (WebCore::MediaSessionGLib::ensureMprisSessionRegistered): (WebCore::MediaSessionGLib::unregisterMprisSession): (WebCore::MediaSessionGLib::emitPositionChanged): (WebCore::MediaSessionGLib::getPlaybackStatusAsGVariant): (WebCore::MediaSessionGLib::emitPropertiesChanged): * Source/WebCore/platform/audio/glib/MediaSessionGLib.h: (WebCore::MediaSessionGLib::setMprisRegistrationEligibility): (WebCore::MediaSessionGLib::mprisRegistrationEligibility const): (WTF::LogArgument<WebCore::MediaSessionGLib::MprisRegistrationEligiblilty>::toString): * Source/WebCore/platform/audio/glib/MediaSessionManagerGLib.cpp: (WebCore::MediaSessionManagerGLib::setPrimarySessionIfNeeded): (WebCore::MediaSessionManagerGLib::unregisterAllOtherSessions): (WebCore::MediaSessionManagerGLib::nowPlayingEligibleSession): Modified to reflect the Cocoa implementation (WebCore::MediaSessionManagerGLib::updateNowPlayingInfo): (WebCore::MediaSessionManagerGLib::setCurrentSession): * Source/WebCore/platform/audio/glib/MediaSessionManagerGLib.h: Canonical link: https://commits.webkit.org/276591@main
WebKit · Mar 23, 2024 · 5e2b534 · 5e2b534
1 parent 28e1e82
commit 5e2b534
Show file tree

Hide file tree

Showing 4 changed files with 98 additions and 24 deletions.
diff --git a/Source/WebCore/platform/audio/glib/MediaSessionGLib.cpp b/Source/WebCore/platform/audio/glib/MediaSessionGLib.cpp
@@ -205,20 +205,16 @@ MediaSessionGLib::MediaSessionGLib(MediaSessionManagerGLib& manager, GRefPtr<GDB
 
 MediaSessionGLib::~MediaSessionGLib()
 {
-    if (m_connection) {
-        if (m_rootRegistrationId && !g_dbus_connection_unregister_object(m_connection.get(), m_rootRegistrationId))
-            g_warning("Unable to unregister MPRIS D-Bus object.");
-        if (m_playerRegistrationId && !g_dbus_connection_unregister_object(m_connection.get(), m_playerRegistrationId))
-            g_warning("Unable to unregister MPRIS D-Bus player object.");
-    }
-    if (m_ownerId)
-        g_bus_unown_name(m_ownerId);
+    unregisterMprisSession();
 }
 
-void MediaSessionGLib::ensureMprisSessionRegistered()
+bool MediaSessionGLib::ensureMprisSessionRegistered()
 {
-    if (m_ownerId || !m_connection)
-        return;
+    if (!m_connection || mprisRegistrationEligibility() == MediaSessionGLibMprisRegistrationEligiblilty::NotEligible)
+        return false;
+
+    if (m_ownerId)
+        return true;
 
     const auto& mprisInterface = m_manager.mprisInterface();
     GUniqueOutPtr<GError> error;
@@ -227,29 +223,52 @@ void MediaSessionGLib::ensureMprisSessionRegistered()
 
     if (!m_rootRegistrationId) {
         g_warning("Failed to register MPRIS D-Bus object: %s", error->message);
-        return;
+        return false;
     }
 
     m_playerRegistrationId = g_dbus_connection_register_object(m_connection.get(), DBUS_MPRIS_OBJECT_PATH, mprisInterface->interfaces[1],
         &gInterfaceVTable, this, nullptr, &error.outPtr());
 
     if (!m_playerRegistrationId) {
         g_warning("Failed at MPRIS object registration: %s", error->message);
-        return;
+        return false;
     }
 
     const auto& applicationID = getApplicationID();
     m_instanceId = applicationID.isEmpty() ? makeString("org.mpris.MediaPlayer2.webkit.instance", getpid(), "-", m_identifier.toUInt64()) : makeString("org.mpris.MediaPlayer2.", applicationID.ascii().data(), ".Sandboxed.instance-", m_identifier.toUInt64());
 
     m_ownerId = g_bus_own_name_on_connection(m_connection.get(), m_instanceId.ascii().data(), G_BUS_NAME_OWNER_FLAGS_NONE, nullptr, nullptr, this, nullptr);
+
+    return true;
+}
+
+void MediaSessionGLib::unregisterMprisSession()
+{
+    if (m_connection) {
+        if (m_rootRegistrationId && !g_dbus_connection_unregister_object(m_connection.get(), m_rootRegistrationId))
+            g_warning("Unable to unregister MPRIS D-Bus object.");
+        m_rootRegistrationId = 0;
+
+        if (m_playerRegistrationId && !g_dbus_connection_unregister_object(m_connection.get(), m_playerRegistrationId))
+            g_warning("Unable to unregister MPRIS D-Bus player object.");
+        m_playerRegistrationId = 0;
+    }
+    if (m_ownerId) {
+        g_bus_unown_name(m_ownerId);
+        m_ownerId = 0;
+    }
+
+    // This session will only be eligible again once it is set to the primary session.
+    setMprisRegistrationEligibility(MprisRegistrationEligiblilty::NotEligible);
 }
 
 void MediaSessionGLib::emitPositionChanged(double time)
 {
     if (!m_connection)
         return;
 
-    ensureMprisSessionRegistered();
+    if (!ensureMprisSessionRegistered())
+        return;
 
     GUniqueOutPtr<GError> error;
     int64_t position = time * 1000000;
@@ -301,7 +320,7 @@ GVariant* MediaSessionGLib::getPlaybackStatusAsGVariant(std::optional<const Plat
         if (session)
             return session.value()->state();
 
-        auto* nowPlayingSession = m_manager.nowPlayingEligibleSession();
+        auto nowPlayingSession = m_manager.nowPlayingEligibleSession();
         if (nowPlayingSession)
             return nowPlayingSession->state();
 
@@ -327,7 +346,8 @@ void MediaSessionGLib::emitPropertiesChanged(GVariant* parameters)
     if (!m_connection)
         return;
 
-    ensureMprisSessionRegistered();
+    if (!ensureMprisSessionRegistered())
+        return;
 
     GUniqueOutPtr<GError> error;
     if (!g_dbus_connection_emit_signal(m_connection.get(), nullptr, DBUS_MPRIS_OBJECT_PATH, "org.freedesktop.DBus.Properties", "PropertiesChanged", parameters, &error.outPtr()))

diff --git a/Source/WebCore/platform/audio/glib/MediaSessionGLib.h b/Source/WebCore/platform/audio/glib/MediaSessionGLib.h
@@ -1,5 +1,6 @@
 /*
  *  Copyright (C) 2021 Igalia S.L
+ *  Copyright (C) 2024 Alexander M (webkit@sata.lol)
  *
  *  This library is free software; you can redistribute it and/or
  *  modify it under the terms of the GNU Lesser General Public
@@ -26,6 +27,11 @@
 
 namespace WebCore {
 
+enum class MediaSessionGLibMprisRegistrationEligiblilty : uint8_t {
+    Eligible,
+    NotEligible,
+};
+
 class MediaSessionManagerGLib;
 
 class MediaSessionGLib {
@@ -48,20 +54,43 @@ class MediaSessionGLib {
     void updateNowPlaying(NowPlayingInfo&);
     void playbackStatusChanged(PlatformMediaSession&);
 
+    void unregisterMprisSession();
+
+    using MprisRegistrationEligiblilty = MediaSessionGLibMprisRegistrationEligiblilty;
+    void setMprisRegistrationEligibility(MprisRegistrationEligiblilty eligibility) { m_registrationEligibility = eligibility; }
+    MprisRegistrationEligiblilty mprisRegistrationEligibility() const { return m_registrationEligibility; }
 private:
     void emitPropertiesChanged(GVariant*);
     std::optional<NowPlayingInfo> nowPlayingInfo();
-    void ensureMprisSessionRegistered();
+    bool ensureMprisSessionRegistered();
 
     MediaSessionIdentifier m_identifier;
     MediaSessionManagerGLib& m_manager;
     GRefPtr<GDBusConnection> m_connection;
+    MprisRegistrationEligiblilty m_registrationEligibility { MprisRegistrationEligiblilty::NotEligible };
     String m_instanceId;
     unsigned m_ownerId { 0 };
     unsigned m_rootRegistrationId { 0 };
     unsigned m_playerRegistrationId { 0 };
 };
 
+String convertEnumerationToString(MediaSessionGLib::MprisRegistrationEligiblilty);
+
 } // namespace WebCore
 
+namespace WTF {
+
+template<typename Type>
+struct LogArgument;
+
+template <>
+struct LogArgument<WebCore::MediaSessionGLib::MprisRegistrationEligiblilty> {
+    static String toString(const WebCore::MediaSessionGLib::MprisRegistrationEligiblilty state)
+    {
+        return convertEnumerationToString(state);
+    }
+};
+
+} // namespace WTF
+
 #endif // USE(GLIB) && ENABLE(MEDIA_SESSION)
diff --git a/Source/WebCore/platform/audio/glib/MediaSessionManagerGLib.cpp b/Source/WebCore/platform/audio/glib/MediaSessionManagerGLib.cpp
@@ -1,5 +1,6 @@
 /*
  *  Copyright (C) 2021 Igalia S.L
+ *  Copyright (C) 2024 Alexander M (webkit@sata.lol)
  *
  *  This library is free software; you can redistribute it and/or
  *  modify it under the terms of the GNU Lesser General Public
@@ -195,6 +196,7 @@ void MediaSessionManagerGLib::setCurrentSession(PlatformMediaSession& session)
 {
     PlatformMediaSessionManager::setCurrentSession(session);
 
+    setPrimarySessionIfNeeded(session);
     m_nowPlayingManager->updateSupportedCommands();
 }
 
@@ -260,18 +262,39 @@ RemoteCommandListener::RemoteCommandsSet MediaSessionManagerGLib::supportedComma
     return m_nowPlayingManager->supportedCommands();
 }
 
-PlatformMediaSession* MediaSessionManagerGLib::nowPlayingEligibleSession()
+void MediaSessionManagerGLib::setPrimarySessionIfNeeded(PlatformMediaSession& platformSession)
 {
-    // FIXME: Fix this layering violation.
-    if (auto element = HTMLMediaElement::bestMediaElementForRemoteControls(MediaElementSession::PlaybackControlsPurpose::NowPlaying))
-        return &element->mediaSession();
+    if (PlatformMediaSessionManager::currentSession() != &platformSession)
+        return;
+
+    auto session = m_sessions.get(platformSession.mediaSessionIdentifier());
+    ASSERT(session);
+    if (!session)
+        return;
+
+    session->setMprisRegistrationEligibility(MediaSessionGLib::MprisRegistrationEligiblilty::Eligible);
+    unregisterAllOtherSessions(platformSession);
+}
 
-    return nullptr;
+void MediaSessionManagerGLib::unregisterAllOtherSessions(PlatformMediaSession& platformSession)
+{
+    ALWAYS_LOG(LOGIDENTIFIER, platformSession.logIdentifier());
+    for (auto& [sessionId, session] : m_sessions) {
+        if (sessionId != platformSession.mediaSessionIdentifier())
+            session->unregisterMprisSession();
+    }
+}
+
+WeakPtr<PlatformMediaSession> MediaSessionManagerGLib::nowPlayingEligibleSession()
+{
+    return bestEligibleSessionForRemoteControls([](auto& session) {
+        return session.isNowPlayingEligible();
+    }, PlatformMediaSession::PlaybackControlsPurpose::NowPlaying);
 }
 
 void MediaSessionManagerGLib::updateNowPlayingInfo()
 {
-    auto* platformSession = nowPlayingEligibleSession();
+    auto platformSession = nowPlayingEligibleSession();
     if (!platformSession) {
         if (m_registeredAsNowPlayingApplication) {
             ALWAYS_LOG(LOGIDENTIFIER, "clearing now playing info");

diff --git a/Source/WebCore/platform/audio/glib/MediaSessionManagerGLib.h b/Source/WebCore/platform/audio/glib/MediaSessionManagerGLib.h
@@ -51,7 +51,9 @@ class MediaSessionManagerGLib
     void dispatch(PlatformMediaSession::RemoteControlCommandType, PlatformMediaSession::RemoteCommandArgument);
 
     const GRefPtr<GDBusNodeInfo>& mprisInterface() const { return m_mprisInterface; }
-    PlatformMediaSession* nowPlayingEligibleSession();
+    void setPrimarySessionIfNeeded(PlatformMediaSession&);
+    void unregisterAllOtherSessions(PlatformMediaSession&);
+    WeakPtr<PlatformMediaSession> nowPlayingEligibleSession();
 
     void setDBusNotificationsEnabled(bool dbusNotificationsEnabled) { m_dbusNotificationsEnabled = dbusNotificationsEnabled; }
     bool areDBusNotificationsEnabled() const { return m_dbusNotificationsEnabled; }