Bug 230250 – [GLib] Media session manager unable to handle more than one session

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Philippe Normand]

The dbus name it tries to acquire is unique to the app PID. We need some session id there, and the manager should handle more than one dbus object then, I think.

[Philippe Normand]

Created attachment 438586 [details]
Patch

[Peng Liu]

Comment on attachment 438586 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=438586&action=review

> Source/WebCore/platform/audio/glib/MediaSessionGLib.cpp:50
> +        return { };

Do you mean `return std::nullopt;` here?

> Source/WebCore/platform/audio/glib/MediaSessionGLib.cpp:117
> +        return { };

Ditto.

> Source/WebCore/platform/audio/glib/MediaSessionGLib.h:37
> +    explicit MediaSessionGLib(MediaSessionManagerGLib&, GRefPtr<GDBusConnection>&&, MediaSessionIdentifier);

Do we need explicit here?

[Chris Dumez]

Comment on attachment 438586 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=438586&action=review

>> Source/WebCore/platform/audio/glib/MediaSessionGLib.cpp:50
>> +        return { };
> 
> Do you mean `return std::nullopt;` here?

Why? What would be the difference?

[Peng Liu]

Comment on attachment 438586 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=438586&action=review

>>> Source/WebCore/platform/audio/glib/MediaSessionGLib.cpp:50
>>> +        return { };
>> 
>> Do you mean `return std::nullopt;` here?
> 
> Why? What would be the difference?

Just a style thing. I have seen a lot of usages of `std::nullopt`.

[Adrian Perez]

Comment on attachment 438586 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=438586&action=review

>> Source/WebCore/platform/audio/glib/MediaSessionGLib.h:37
>> +    explicit MediaSessionGLib(MediaSessionManagerGLib&, GRefPtr<GDBusConnection>&&, MediaSessionIdentifier);
> 
> Do we need explicit here?

Never hurts :)

[EWS]

Committed r283437 (242424@main): <https://commits.webkit.org/242424@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 438586 [details].

[Radar WebKit Bug Importer]

<rdar://problem/83795322>

[Arcady Goldmints-Orlov]

This appears to have caused a number of regressions, (list filtered to what I think could be related to the change):

Regressions: Unexpected text-only failures

	media/audio-background-playback-playlist.html [ Failure ]
	media/media-session/actionHandlerInternalMappings.html [ Failure ]
	media/media-session/default-actionHandlers.html [ Failure ]
	media/video-background-playback.html [ Failure ]
	media/video-concurrent-playback.html [ Failure ]
	media/video-interruption-with-resume-not-allowing-play.html [ Failure ]
	media/video-multiple-concurrent-playback.html [ Failure ]
	media/webaudio-background-playback.html [ Failure ]
	webrtc/concurrentVideoPlayback2.html [ Failure ]

Regressions: Unexpected timeouts
	media/media-session/callActionHandler.html [ Timeout ]
	media/media-session/play-after-seek.html [ Timeout ]
	media/remote-control-command-is-user-gesture.html [ Timeout ]
	media/remote-control-command-scrubbing.html [ Timeout ]
	media/remote-control-command-seek.html [ Timeout ]
	media/video-inactive-playback.html [ Timeout ]
	media/video-interruption-with-resume-allowing-play.html [ Timeout ]
	media/video-isplayingtoautomotiveheadunit.html [ Timeout ]
	media/video-system-sleep.html [ Timeout ]
	webaudio/audiocontext-state-interrupted.html [ Timeout ]
	webaudio/suspend-context-while-interrupted.html [ Timeout ]

[Philippe Normand]

I can't reproduce this on my desktop...

[Arcady Goldmints-Orlov]

I can't reproduce it locally either but it seems to happen consistently on the bots, e.g. https://build.webkit.org/results/GTK-Linux-64-bit-Release-Skip-Failing-Tests/r284248%20(2041)/results.html