Changeset 207690 in webkit


Ignore:
Timestamp:
Oct 21, 2016 11:53:05 AM (7 years ago)
Author:
Chris Dumez
Message:

[Web IDL] MediaControlsHost has invalid operation overloads
​https://bugs.webkit.org/show_bug.cgi?id=163793

Reviewed by Darin Adler.

MediaControlsHost has invalid operation overloads:

  • sortedTrackListForMenu()
  • displayNameForTrack()

The parameter is nullable for both overloads which is not valid IDL.

  • sortedTrackListForMenu(): The parameter is no longer nullable. This is a minor behavior change and it should be safe since this is Apple-specific and only called from mediaControlsApple.js which uses HTMLMediaElement.videoTracks and HTMLMediaElement.audioTracks as input, both of which are not nullable. Note that we could have also kept one of the parameters as nullable to not change behavior but allowing null does not seem useful here.
  • displayNameForTrack(): Use a union instead of overloading, no behavior change.
  • Modules/mediacontrols/MediaControlsHost.cpp:

(WebCore::MediaControlsHost::sortedTrackListForMenu):
(WebCore::MediaControlsHost::displayNameForTrack):

  • Modules/mediacontrols/MediaControlsHost.h:
  • Modules/mediacontrols/MediaControlsHost.idl:
Location:
trunk/Source/WebCore
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r207689 r207690  
     12016-10-21  Chris Dumez  <cdumez@apple.com>
     2
     3        [Web IDL] MediaControlsHost has invalid operation overloads
     4        https://bugs.webkit.org/show_bug.cgi?id=163793
     5
     6        Reviewed by Darin Adler.
     7
     8        MediaControlsHost has invalid operation overloads:
     9        - sortedTrackListForMenu()
     10        - displayNameForTrack()
     11
     12        The parameter is nullable for both overloads which is not valid IDL.
     13
     14        - sortedTrackListForMenu(): The parameter is no longer nullable. This is a minor
     15          behavior change and it should be safe since this is Apple-specific and only
     16          called from mediaControlsApple.js which uses HTMLMediaElement.videoTracks and
     17          HTMLMediaElement.audioTracks as input, both of which are not nullable.
     18          Note that we could have also kept one of the parameters as nullable to not
     19          change behavior but allowing null does not seem useful here.
     20        - displayNameForTrack(): Use a union instead of overloading, no behavior change.
     21
     22        * Modules/mediacontrols/MediaControlsHost.cpp:
     23        (WebCore::MediaControlsHost::sortedTrackListForMenu):
     24        (WebCore::MediaControlsHost::displayNameForTrack):
     25        * Modules/mediacontrols/MediaControlsHost.h:
     26        * Modules/mediacontrols/MediaControlsHost.idl:
     27
    1282016-10-21  Jeremy Jones  <jeremyj@apple.com>
    229

  • trunk/Source/WebCore/Modules/mediacontrols/MediaControlsHost.cpp

    r207423 r207690  
    8585}
    8686
    87 Vector<RefPtr<TextTrack>> MediaControlsHost::sortedTrackListForMenu(TextTrackList* trackList)
    88 {
    89     if (!trackList)
    90         return Vector<RefPtr<TextTrack>>();
    91 
    92     Page* page = m_mediaElement->document().page();
    93     if (!page)
    94         return Vector<RefPtr<TextTrack>>();
    95 
    96     return page->group().captionPreferences().sortedTrackListForMenu(trackList);
    97 }
    98 
    99 Vector<RefPtr<AudioTrack>> MediaControlsHost::sortedTrackListForMenu(AudioTrackList* trackList)
    100 {
    101     if (!trackList)
    102         return Vector<RefPtr<AudioTrack>>();
    103 
    104     Page* page = m_mediaElement->document().page();
    105     if (!page)
    106         return Vector<RefPtr<AudioTrack>>();
    107 
    108     return page->group().captionPreferences().sortedTrackListForMenu(trackList);
    109 }
    110 
    111 String MediaControlsHost::displayNameForTrack(TextTrack* track)
     87Vector<RefPtr<TextTrack>> MediaControlsHost::sortedTrackListForMenu(TextTrackList& trackList)
     88{
     89    Page* page = m_mediaElement->document().page();
     90    if (!page)
     91        return { };
     92
     93    return page->group().captionPreferences().sortedTrackListForMenu(&trackList);
     94}
     95
     96Vector<RefPtr<AudioTrack>> MediaControlsHost::sortedTrackListForMenu(AudioTrackList& trackList)
     97{
     98    Page* page = m_mediaElement->document().page();
     99    if (!page)
     100        return { };
     101
     102    return page->group().captionPreferences().sortedTrackListForMenu(&trackList);
     103}
     104
     105String MediaControlsHost::displayNameForTrack(const Optional<TextOrAudioTrack>& track)
    112106{
    113107    if (!track)
    … …  
    118112        return emptyString();
    119113
    120     return page->group().captionPreferences().displayNameForTrack(track);
    121 }
    122 
    123 String MediaControlsHost::displayNameForTrack(AudioTrack* track)
    124 {
    125     if (!track)
    126         return emptyString();
    127 
    128     Page* page = m_mediaElement->document().page();
    129     if (!page)
    130         return emptyString();
    131 
    132     return page->group().captionPreferences().displayNameForTrack(track);
     114    return WTF::visit([&page](auto& track) {
     115        return page->group().captionPreferences().displayNameForTrack(track.get());
     116    }, track.value());
    133117}
    134118

  • trunk/Source/WebCore/Modules/mediacontrols/MediaControlsHost.h

    r207423 r207690  
    3030#include <bindings/ScriptObject.h>
    3131#include <wtf/RefCounted.h>
     32#include <wtf/Variant.h>
    3233#include <wtf/Vector.h>
    3334#include <wtf/text/WTFString.h>
    … …  
    5354    static const AtomicString& manualKeyword();
    5455
    55     Vector<RefPtr<TextTrack>> sortedTrackListForMenu(TextTrackList*);
    56     Vector<RefPtr<AudioTrack>> sortedTrackListForMenu(AudioTrackList*);
    57     String displayNameForTrack(TextTrack*);
    58     String displayNameForTrack(AudioTrack*);
     56    Vector<RefPtr<TextTrack>> sortedTrackListForMenu(TextTrackList&);
     57    Vector<RefPtr<AudioTrack>> sortedTrackListForMenu(AudioTrackList&);
     58
     59    using TextOrAudioTrack = WTF::Variant<RefPtr<TextTrack>, RefPtr<AudioTrack>>;
     60    String displayNameForTrack(const Optional<TextOrAudioTrack>&);
     61
    5962    TextTrack* captionMenuOffItem();
    6063    TextTrack* captionMenuAutomaticItem();

  • trunk/Source/WebCore/Modules/mediacontrols/MediaControlsHost.idl

    r207423 r207690  
    3535    NoInterfaceObject,
    3636] interface MediaControlsHost {
    37     sequence<TextTrack> sortedTrackListForMenu(TextTrackList? trackList);
    38     sequence<AudioTrack> sortedTrackListForMenu(AudioTrackList? trackList);
    39     DOMString displayNameForTrack(TextTrack? track);
    40     DOMString displayNameForTrack(AudioTrack? track);
     37    sequence<TextTrack> sortedTrackListForMenu(TextTrackList trackList);
     38    sequence<AudioTrack> sortedTrackListForMenu(AudioTrackList trackList);
     39    DOMString displayNameForTrack((TextTrack or AudioTrack)? track);
    4140    readonly attribute TextTrack captionMenuOffItem;
    4241    readonly attribute TextTrack captionMenuAutomaticItem;
Note: See TracChangeset for help on using the changeset viewer.