Don't pause when the screen locks if playing to AirPlay.
<rdar://problem/22300597>
Created attachment 259637 [details] Proposed patch
Attachment 259637 [details] did not pass style-queue: ERROR: Source/WebCore/platform/audio/PlatformMediaSession.cpp:44: Non-label code inside switch statements should be indented. [whitespace/indent] [4] ERROR: Source/WebCore/platform/audio/PlatformMediaSession.cpp:58: Non-label code inside switch statements should be indented. [whitespace/indent] [4] Total errors found: 2 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 259637 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=259637&action=review r=me, with nit. > Source/WebCore/platform/audio/ios/MediaSessionManagerIOS.mm:267 > + Vector<PlatformMediaSession*> sessions = this->sessions(); > + for (auto* session : sessions) { nit: Why the local variable? Seems like this could just be: for (auto* session : sessions()) { ... } Looking into PlatformMediaSessionManager, I don't understand why sessions() doesn't return a Vector<>&. But it also seems a waste to have to create a new Vector every time this method is called.
(In reply to comment #4) > Comment on attachment 259637 [details] > Proposed patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=259637&action=review > > r=me, with nit. > > > Source/WebCore/platform/audio/ios/MediaSessionManagerIOS.mm:267 > > + Vector<PlatformMediaSession*> sessions = this->sessions(); > > + for (auto* session : sessions) { > > nit: Why the local variable? Seems like this could just be: > > for (auto* session : sessions()) { ... } > > Looking into PlatformMediaSessionManager, I don't understand why sessions() > doesn't return a Vector<>&. But it also seems a waste to have to create a > new Vector every time this method is called. I think because calling session.whatever() can cause m_sessions to be modified.
(In reply to comment #5) > (In reply to comment #4) > > Comment on attachment 259637 [details] > > Proposed patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=259637&action=review > > > > r=me, with nit. > > > > > Source/WebCore/platform/audio/ios/MediaSessionManagerIOS.mm:267 > > > + Vector<PlatformMediaSession*> sessions = this->sessions(); > > > + for (auto* session : sessions) { > > > > nit: Why the local variable? Seems like this could just be: > > > > for (auto* session : sessions()) { ... } > > > > Looking into PlatformMediaSessionManager, I don't understand why sessions() > > doesn't return a Vector<>&. But it also seems a waste to have to create a > > new Vector every time this method is called. > > I think because calling session.whatever() can cause m_sessions to be > modified. Ah yes, I see that pattern elsewhere in PlatformMediaSessionManager.cpp. Carry on!
Committed r190434: <http://trac.webkit.org/changeset/190434>
Note: This fix broke the test 'webaudio/audiocontext-state-interrupted.html', because it was not updated to pass an argument to internals.beginMediaSessionInterruption.
Test fix committed in: Committed r190434: <http://trac.webkit.org/changeset/190438>
(In reply to comment #7) > Committed r190434: <http://trac.webkit.org/changeset/190434> Is there any reason we don't use *interruption* local variable in beginInterruption() ? void Internals::beginMediaSessionInterruption(const String& interruptionString, ExceptionCode& ec) { PlatformMediaSession::InterruptionType interruption = PlatformMediaSession::SystemInterruption; if (equalIgnoringCase(interruptionString, "System")) interruption = PlatformMediaSession::SystemInterruption; else if (equalIgnoringCase(interruptionString, "SystemSleep")) interruption = PlatformMediaSession::SystemSleep; else if (equalIgnoringCase(interruptionString, "EnteringBackground")) interruption = PlatformMediaSession::EnteringBackground; else { ec = INVALID_ACCESS_ERR; return; } PlatformMediaSessionManager::sharedManager().beginInterruption(PlatformMediaSession::SystemInterruption); }
(In reply to comment #10) > (In reply to comment #7) > > Committed r190434: <http://trac.webkit.org/changeset/190434> > > Is there any reason we don't use *interruption* local variable in > beginInterruption() ? > > > void Internals::beginMediaSessionInterruption(const String& > interruptionString, ExceptionCode& ec) > { > PlatformMediaSession::InterruptionType interruption = > PlatformMediaSession::SystemInterruption; > > if (equalIgnoringCase(interruptionString, "System")) > interruption = PlatformMediaSession::SystemInterruption; > else if (equalIgnoringCase(interruptionString, "SystemSleep")) > interruption = PlatformMediaSession::SystemSleep; > else if (equalIgnoringCase(interruptionString, "EnteringBackground")) > interruption = PlatformMediaSession::EnteringBackground; > else { > ec = INVALID_ACCESS_ERR; > return; > } > > > PlatformMediaSessionManager::sharedManager(). > beginInterruption(PlatformMediaSession::SystemInterruption); > } I upload a fix to Bug 148315. If there is any reason we don't use *interruption* variable, please let me know. https://bugs.webkit.org/show_bug.cgi?id=148315
(In reply to comment #11) > (In reply to comment #10) > > (In reply to comment #7) > > > Committed r190434: <http://trac.webkit.org/changeset/190434> > > > > Is there any reason we don't use *interruption* local variable in > > beginInterruption() ? > > > > > > void Internals::beginMediaSessionInterruption(const String& > > interruptionString, ExceptionCode& ec) > > { > > PlatformMediaSession::InterruptionType interruption = > > PlatformMediaSession::SystemInterruption; > > > > if (equalIgnoringCase(interruptionString, "System")) > > interruption = PlatformMediaSession::SystemInterruption; > > else if (equalIgnoringCase(interruptionString, "SystemSleep")) > > interruption = PlatformMediaSession::SystemSleep; > > else if (equalIgnoringCase(interruptionString, "EnteringBackground")) > > interruption = PlatformMediaSession::EnteringBackground; > > else { > > ec = INVALID_ACCESS_ERR; > > return; > > } > > > > > > PlatformMediaSessionManager::sharedManager(). > > beginInterruption(PlatformMediaSession::SystemInterruption); > > } > > > I upload a fix to Bug 148315. If there is any reason we don't use > *interruption* variable, please let me know. > > https://bugs.webkit.org/show_bug.cgi?id=148315 Oops, r190447 already fixed it. - http://trac.webkit.org/changeset/190447
(In reply to comment #10) > (In reply to comment #7) > > Committed r190434: <http://trac.webkit.org/changeset/190434> > > Is there any reason we don't use *interruption* local variable in > beginInterruption() ? It's just an oversight. The initial test case always used System, so it was missed earlier today. I had a note to myself to fix it tomorrow, but you've beat me to it! :)