[iOS] <video> elements without audio tracks should not interrupt music
Created attachment 262724 [details] Patch
Comment on attachment 262724 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=262724&action=review Not sure exactly why the Mac build is failing. > Source/WebCore/html/HTMLMediaElement.cpp:6450 > +bool HTMLMediaElement::producesAudio() const > +{ > + if (m_player && m_readyState >= HAVE_METADATA) > + return hasAudio(); > + return hasTagName(HTMLNames::audioTag); > +} This function is hard to use correctly. It can return false for a video element and then change its mind and start returning true for the same element as the element loads content. I think it can even return true for an audio element and then start returning false for the same element as the element loads content in some strange cases. What notifies interested parties that this may have changed and it’s time to check it again? It also seems weak design for this to call hasTagName(HTMLNames::audioTag) when we already have separate classes for the audio and video elements. Maybe override this for video element and audio element instead of media element. Maybe a name like “producing audio” would make it a little more clear that this is a state that can change over time. > Tools/TestWebKitAPI/Tests/WebKit/ios/AudioSessionCategoryIOS.mm:34 > +#import <UIKit/SoftLinking.h> On the bot: TestWebKitAPI/Tests/WebKit/ios/AudioSessionCategoryIOS.mm:34:9: fatal error: 'UIKit/SoftLinking.h' file not found
(In reply to comment #2) > Comment on attachment 262724 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=262724&action=review > > Not sure exactly why the Mac build is failing. > > > Source/WebCore/html/HTMLMediaElement.cpp:6450 > > +bool HTMLMediaElement::producesAudio() const > > +{ > > + if (m_player && m_readyState >= HAVE_METADATA) > > + return hasAudio(); > > + return hasTagName(HTMLNames::audioTag); > > +} > > This function is hard to use correctly. It can return false for a video > element and then change its mind and start returning true for the same > element as the element loads content. I think it can even return true for an > audio element and then start returning false for the same element as the > element loads content in some strange cases. What notifies interested > parties that this may have changed and it’s time to check it again? Such is the nature of video on the internet; especially with HLS (and also with MSE) video can go from having an audio track to not having one without user intervention. However, we handle this case by re-running PlatformMediaSessionManager::updateSessionState() whenever the hasAudio() or hasVideo() change in HTMLMediaElement::mediaPlayerCharacteristicChanged(). > It also seems weak design for this to call hasTagName(HTMLNames::audioTag) > when we already have separate classes for the audio and video elements. True. > Maybe override this for video element and audio element instead of media > element. > > Maybe a name like “producing audio” would make it a little more clear that > this is a state that can change over time. Sure. > > Tools/TestWebKitAPI/Tests/WebKit/ios/AudioSessionCategoryIOS.mm:34 > > +#import <UIKit/SoftLinking.h> > > On the bot: TestWebKitAPI/Tests/WebKit/ios/AudioSessionCategoryIOS.mm:34:9: > fatal error: 'UIKit/SoftLinking.h' file not found I'll try to use one of the WebKit versions of that header.
Created attachment 262801 [details] Patch
Comment on attachment 262801 [details] Patch Patch does not build on Mac.
Looks like CanHandleRequest.cpp somehow got re-added to the TestWebKitAPI project (bad merge?). Will fix.
Created attachment 264438 [details] Patch
Comment on attachment 264438 [details] Patch r=me
Comment on attachment 264438 [details] Patch Whoops, forgot tests in this patch.
Created attachment 264593 [details] Patch for landing
Created attachment 264595 [details] Patch for landing
Comment on attachment 264595 [details] Patch for landing Rejecting attachment 264595 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 264595, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit ChangeLog entry in Source/WebCore/ChangeLog contains OOPS!. Full output: http://webkit-queues.webkit.org/results/382217
Created attachment 264794 [details] Patch for landing
Comment on attachment 264794 [details] Patch for landing Clearing flags on attachment: 264794 Committed r192027: <http://trac.webkit.org/changeset/192027>
<rdar://problem/22235751>