Use OptionSet for MediaProducer::MediaStateFlags
Created attachment 427543 [details] Patch
Comment on attachment 427543 [details] Patch bot failure is unrelated
Comment on attachment 427543 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=427543&action=review > Source/WebCore/Modules/airplay/WebMediaSessionManager.cpp:278 > + MediaProducer::MediaStateFlags updateConfigurationFlags { MediaProducer::MediaState::RequiresPlaybackTargetMonitoring, MediaProducer::MediaState::HasPlaybackTargetAvailabilityListener, MediaProducer::MediaState::HasAudioOrVideo }; Not new to this refactoring, but this could be `static constexpr` > Source/WebCore/Modules/airplay/WebMediaSessionManager.cpp:282 > + MediaProducer::MediaStateFlags playingToTargetFlags { MediaProducer::MediaState::IsPlayingToExternalDevice, MediaProducer::MediaState::IsPlayingVideo }; Ditto > Source/WebCore/html/HTMLMediaElement.cpp:7696 > + if (hasMediaStreamSrcObject() && mediaState().containsAny(MediaProducer::MediaState::IsPlayingAudio) && document().mediaState().containsAny(MediaProducer::MediaState::HasActiveAudioCaptureDevice)) { Nit: although `OptionSet::contains()` and `OptionSet::constainsAny()` are the same, it is slightly odd all throughout this patch you sometimes use `constainsAny` and other times use 'contains` to test for just one value.
(In reply to Eric Carlson from comment #3) > Comment on attachment 427543 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=427543&action=review > > > Source/WebCore/Modules/airplay/WebMediaSessionManager.cpp:278 > > + MediaProducer::MediaStateFlags updateConfigurationFlags { MediaProducer::MediaState::RequiresPlaybackTargetMonitoring, MediaProducer::MediaState::HasPlaybackTargetAvailabilityListener, MediaProducer::MediaState::HasAudioOrVideo }; > > Not new to this refactoring, but this could be `static constexpr` Will fix. > > Source/WebCore/Modules/airplay/WebMediaSessionManager.cpp:282 > > + MediaProducer::MediaStateFlags playingToTargetFlags { MediaProducer::MediaState::IsPlayingToExternalDevice, MediaProducer::MediaState::IsPlayingVideo }; > > Ditto OK > > Source/WebCore/html/HTMLMediaElement.cpp:7696 > > + if (hasMediaStreamSrcObject() && mediaState().containsAny(MediaProducer::MediaState::IsPlayingAudio) && document().mediaState().containsAny(MediaProducer::MediaState::HasActiveAudioCaptureDevice)) { > > Nit: although `OptionSet::contains()` and `OptionSet::constainsAny()` are > the same, it is slightly odd all throughout this patch you sometimes use > `constainsAny` and other times use 'contains` to test for just one value. contains take a MediaState, containsAny takes a MediaStateFlags. I try to follow the pattern here. containsAny can also take a MediaState since a MediaStateFlags can be implicitly constructed so we could go with containsAny all over the place.
Created attachment 427642 [details] Patch for landing
Committed r276952 (237285@main): <https://commits.webkit.org/237285@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 427642 [details].
<rdar://problem/77496454>