| Summary: | REGRESSION (iOS 15): HTMLAudioElement fails to load new audio when device is locked or safari is in background | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Rick Li <racke1983cn> | ||||||||||||||
| Component: | Web Audio | Assignee: | Eric Carlson <eric.carlson> | ||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||
| Severity: | Major | CC: | cdumez, darin, eric.carlson, ews-watchlist, glenn, hta, jer.noble, peng.liu6, philipj, sergio, tommyw, webkit-bug-importer | ||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||
| Version: | Safari Technology Preview | ||||||||||||||||
| Hardware: | iPhone / iPad | ||||||||||||||||
| OS: | Other | ||||||||||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=239812 | ||||||||||||||||
| Attachments: |
|
||||||||||||||||
|
Description
Rick Li
2021-08-30 15:11:01 PDT
Created attachment 449683 [details]
Patch
Created attachment 449698 [details]
Patch
Comment on attachment 449698 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=449698&action=review > Source/WebCore/platform/audio/cocoa/MediaSessionManagerCocoa.mm:146 > + if (!hasAudibleAudioOrVideoMediaType && m_previousHadAudibleAudioOrVideoMediaType) { > + if (!m_delayCategoryChangeTimer.isActive()) { > + m_delayCategoryChangeTimer.startOneShot(delayBeforeSettingCategoryNone); > + ALWAYS_LOG(LOGIDENTIFIER, "setting timer"); > + } > + hasAudibleAudioOrVideoMediaType = true; > + } > + m_previousHadAudibleAudioOrVideoMediaType = hasAudibleAudioOrVideoMediaType; > + I wonder what will happen when a page is using WebAudio; we may go from Playback -> Ambient without hitting this timer. > Source/WebKit/WebProcess/GPU/media/RemoteAudioSession.cpp:-148 > -#if PLATFORM(COCOA) > return m_category; > -#else > - return AudioSession::CategoryType::None; > -#endif Is this necessary? > LayoutTests/ChangeLog:12 > + * media/audio-session-category-expected.txt: Added. > + * media/audio-session-category.html: Added. Should these go into platform/ios/media or platform/mac/media? Comment on attachment 449698 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=449698&action=review > Source/WebCore/platform/audio/cocoa/MediaSessionManagerCocoa.mm:133 > + m_delayCategoryChangeTimer.stop(); I think this may be in the wrong place. It'll only get hit if there are no audible <video> or <audio> elements, but the session is "potentially audible". > Source/WebCore/platform/audio/cocoa/MediaSessionManagerCocoa.mm:145 > + if (!hasAudibleAudioOrVideoMediaType && m_previousHadAudibleAudioOrVideoMediaType) { > + if (!m_delayCategoryChangeTimer.isActive()) { > + m_delayCategoryChangeTimer.startOneShot(delayBeforeSettingCategoryNone); > + ALWAYS_LOG(LOGIDENTIFIER, "setting timer"); > + } > + hasAudibleAudioOrVideoMediaType = true; > + } > + m_previousHadAudibleAudioOrVideoMediaType = hasAudibleAudioOrVideoMediaType; I think it should move down here, in an "else" statement after the "if" (which may need to be restructured). Maybe something like: if (hasAudibleAudioOrVideoMediaType) m_delayCategoryChangeTimer.stop(); else if (m_previousHadAudibleAudioOrVideoMediaType) { .... } Created attachment 449735 [details]
Patch
Created attachment 449821 [details]
Patch for landing
Comment on attachment 449821 [details]
Patch for landing
Test failures are unrelated.
Committed r288466 (246352@main): <https://commits.webkit.org/246352@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 449821 [details]. Comment on attachment 449821 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=449821&action=review > Source/WebCore/platform/audio/cocoa/MediaSessionManagerCocoa.mm:97 > + static const Seconds delayBeforeSettingCategoryNone = { 2_s }; constexpr auto delayBeforeSettingCategoryNone = 2_s; Reopening to attach new patch. Created attachment 449867 [details]
Address post-review feedback
Committed r288493 (246365@main): <https://commits.webkit.org/246365@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 449867 [details]. Reopening to attach new patch. Created attachment 458443 [details]
Patch
Comment on attachment 458443 [details]
Patch
Nothing to see here, I attached the patch to the wrong bug.
|