Bug 236620

Summary: [Cocoa] Update audio session category before setting NowPlaying status
Product: WebKit Reporter: Eric Carlson <eric.carlson>
Component: MediaAssignee: Eric Carlson <eric.carlson>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, glenn, jer.noble, lmoura, philipj, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing
none
Patch for landing
none
patch for landing
none
patch for landing none

Description Eric Carlson 2022-02-14 16:42:32 PST
NowPlaying doesn't update correctly when the audio category changes from Ambient to Playback after becoming active, so always set category first.
Comment 1 Eric Carlson 2022-02-14 16:43:20 PST
rdar://88827167
Comment 2 Eric Carlson 2022-02-14 16:53:05 PST
Created attachment 451960 [details]
Patch
Comment 3 Eric Carlson 2022-02-14 17:05:01 PST
Created attachment 451963 [details]
Patch
Comment 4 EWS 2022-02-14 19:30:32 PST
Found 2 new test failures: media/picture-in-picture/picture-in-picture-interruption.html, webaudio/audiocontext-state-interrupted.html
Comment 5 Eric Carlson 2022-02-15 09:01:00 PST
Created attachment 452030 [details]
Patch for landing
Comment 6 EWS 2022-02-15 11:05:56 PST
Found 1 new test failure: webaudio/audiocontext-state-interrupted.html
Comment 7 Lauro Moura 2022-02-15 11:48:49 PST
This patch seems to have triggered lots of crashes in GTK:

https://ews-build.webkit.org/#/builders/35/builds/9078

Results page: https://ews-build.s3-us-west-2.amazonaws.com/GTK-WK2-Tests-EWS/452030-9078/results.html

Sample trace of the crash:

Thread 1 (Thread 0x7fcc17dc09c0 (LWP 10616)):
#0  g_logv (log_domain=0x7fcc1a72700e "GLib", log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>, args=<optimized out>) at ../glib/gmessages.c:1417
#1  0x00007fcc1a6d9073 in g_log (log_domain=log_domain@entry=0x7fcc1a72700e "GLib", log_level=log_level@entry=G_LOG_LEVEL_CRITICAL, format=format@entry=0x7fcc1a731b70 "%s: assertion '%s' failed") at ../glib/gmessages.c:1455
#2  0x00007fcc1a6d98ad in g_return_if_fail_warning (log_domain=log_domain@entry=0x7fcc1a72700e "GLib", pretty_function=pretty_function@entry=0x7fcc1a78c760 <__func__.76> "g_variant_new_variant", expression=expression@entry=0x7fcc1a78a119 "value != NULL") at ../glib/gmessages.c:2891
#3  0x00007fcc1a70fcd6 in g_variant_new_variant (value=<optimized out>) at ../glib/gvariant.c:723
#4  g_variant_new_variant (value=<optimized out>) at ../glib/gvariant.c:721
#5  0x00007fcc1a7151dd in g_variant_valist_new (str=0x7fff63d79cf8, app=0x7fff63d79d20) at ../glib/gvariant.c:5225
#6  0x00007fcc1a7157b2 in g_variant_new_va (format_string=<optimized out>, endptr=endptr@entry=0x0, app=app@entry=0x7fff63d79d20) at ../glib/gvariant.c:5401
#7  0x00007fcc1a715d11 in g_variant_builder_add (builder=0x7fff63d79e10, format_string=<optimized out>) at ../glib/gvariant.c:5558
#8  0x00007fcc2355cc26 in WebCore::MediaSessionManagerGLib::sessionStateChanged(WebCore::PlatformMediaSession&) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#9  0x00007fcc2354f4a8 in WebCore::PlatformMediaSession::setState(WebCore::PlatformMediaSession::State) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#10 0x00007fcc235512d0 in WebCore::PlatformMediaSession::clientWillBeginPlayback() () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#11 0x00007fcc22f76c6f in WebCore::MediaElementSession::clientWillBeginPlayback() () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#12 0x00007fcc22ed71d0 in WebCore::HTMLMediaElement::playInternal() () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#13 0x00007fcc21c9ab2b in WebCore::jsHTMLMediaElementPrototypeFunction_play(JSC::JSGlobalObject*, JSC::CallFrame*) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#14 0x00007fcbd6bff1d8 in  ()
#15 0x00007fff63d7a0a0 in  ()
#16 0x00007fcc1cf7c88a in op_call_slow_return_location () at /app/webkit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.1.so.0
#17 0x0000000000000000 in  ()
Comment 8 Lauro Moura 2022-02-15 11:53:33 PST
Looks like the fix would be adding the new enum value to the switch in MediaSessionGLib::getPlaybackStatusAsGVariant(...) in Source/WebCore/platform/audio/glib/MediaSessionGlib.cpp
Comment 9 Eric Carlson 2022-02-15 12:17:44 PST
Created attachment 452072 [details]
Patch for landing
Comment 10 EWS 2022-02-15 14:54:31 PST
Found 1 new test failure: webaudio/audiocontext-state-interrupted.html
Comment 11 Eric Carlson 2022-02-15 15:10:43 PST
Comment on attachment 452072 [details]
Patch for landing

I don't believe the gtk API test failure is related.
Comment 12 Eric Carlson 2022-02-15 17:53:56 PST
Created attachment 452119 [details]
patch for landing
Comment 13 Eric Carlson 2022-02-16 08:20:30 PST
Created attachment 452199 [details]
patch for landing
Comment 14 Eric Carlson 2022-02-16 09:55:17 PST
Comment on attachment 452199 [details]
patch for landing

The Windows failure is unrelated.
Comment 15 EWS 2022-02-16 12:23:54 PST
Committed r289942 (247346@main): <https://commits.webkit.org/247346@main>

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