Bug 237828

Summary: [GStreamer] Avoid auto-selection of hole punching player
Product: WebKit Reporter: Enrique Ocaña <eocanha>
Component: MediaAssignee: Enrique Ocaña <eocanha>
Status: RESOLVED FIXED    
Severity: Normal CC: calvaris, cgarcia, eric.carlson, ews-watchlist, glenn, jer.noble, magomez, 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
none
Patch none

Description Enrique Ocaña 2022-03-14 06:46:40 PDT
The hole punch player private shouldn't be automatically selected when the mime type is empty or unknown.
Comment 1 Enrique Ocaña 2022-03-14 07:01:36 PDT
Created attachment 454588 [details]
Patch
Comment 2 Enrique Ocaña 2022-03-14 07:03:08 PDT
Created attachment 454589 [details]
Patch
Comment 3 Miguel Gomez 2022-03-14 07:44:40 PDT
Looks good to me Enrique!
Comment 4 Enrique Ocaña 2022-03-15 04:46:43 PDT
The editing/spelling/spellcheck-api-crash.html failure in the iOS-15-Simulator-WK2-Tests-EWS bot is unrelated to this patch and should be ignored. Actually, I've been seeing that failure in some other unrelated EWS patch submissions these days.
Comment 5 Xabier Rodríguez Calvar 2022-03-15 07:31:29 PDT
Comment on attachment 454589 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=454589&action=review

> Source/WebCore/platform/graphics/holepunch/MediaPlayerPrivateHolePunch.cpp:191
> +        if (mimeType.isEmpty() || !(mimeTypeCache().contains(mimeType)))

!(mimeTypeCache().contains(mimeType)) -> !mimeTypeCache().contains(mimeType)
Comment 6 Carlos Garcia Campos 2022-03-15 07:32:10 PDT
Comment on attachment 454589 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=454589&action=review

> Source/WebCore/platform/graphics/holepunch/MediaPlayerPrivateHolePunch.cpp:189
> +    if (m_player) {

Return early.

>> Source/WebCore/platform/graphics/holepunch/MediaPlayerPrivateHolePunch.cpp:191
>> +        if (mimeType.isEmpty() || !(mimeTypeCache().contains(mimeType)))
> 
> !(mimeTypeCache().contains(mimeType)) -> !mimeTypeCache().contains(mimeType)

Extra parentheses after !

> Source/WebCore/platform/graphics/holepunch/MediaPlayerPrivateHolePunch.h:79
> -    MediaPlayer::NetworkState networkState() const final { return MediaPlayer::NetworkState::Empty; };
> +    MediaPlayer::NetworkState networkState() const final;

We don't need to move this to the cpp, just return m_networkState here.
Comment 7 Enrique Ocaña 2022-03-15 10:50:13 PDT
Created attachment 454729 [details]
Patch
Comment 8 Enrique Ocaña 2022-03-15 12:32:42 PDT
Created attachment 454741 [details]
Patch
Comment 9 EWS 2022-03-16 03:17:21 PDT
Committed r291337 (248476@main): <https://commits.webkit.org/248476@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 454741 [details].
Comment 10 Radar WebKit Bug Importer 2022-03-16 03:18:16 PDT
<rdar://problem/90360852>