Title: [207690] trunk/Source/WebCore
Revision
207690
Author
cdu...@apple.com
Date
2016-10-21 11:53:05 -0700 (Fri, 21 Oct 2016)

Log 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:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (207689 => 207690)


--- trunk/Source/WebCore/ChangeLog	2016-10-21 18:45:39 UTC (rev 207689)
+++ trunk/Source/WebCore/ChangeLog	2016-10-21 18:53:05 UTC (rev 207690)
@@ -1,3 +1,30 @@
+2016-10-21  Chris Dumez  <cdu...@apple.com>
+
+        [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:
+
 2016-10-21  Jeremy Jones  <jere...@apple.com>
 
         Implement basic pointer lock behavior for WebKit and WebKit2.

Modified: trunk/Source/WebCore/Modules/mediacontrols/MediaControlsHost.cpp (207689 => 207690)


--- trunk/Source/WebCore/Modules/mediacontrols/MediaControlsHost.cpp	2016-10-21 18:45:39 UTC (rev 207689)
+++ trunk/Source/WebCore/Modules/mediacontrols/MediaControlsHost.cpp	2016-10-21 18:53:05 UTC (rev 207690)
@@ -84,31 +84,25 @@
 {
 }
 
-Vector<RefPtr<TextTrack>> MediaControlsHost::sortedTrackListForMenu(TextTrackList* trackList)
+Vector<RefPtr<TextTrack>> MediaControlsHost::sortedTrackListForMenu(TextTrackList& trackList)
 {
-    if (!trackList)
-        return Vector<RefPtr<TextTrack>>();
-
     Page* page = m_mediaElement->document().page();
     if (!page)
-        return Vector<RefPtr<TextTrack>>();
+        return { };
 
-    return page->group().captionPreferences().sortedTrackListForMenu(trackList);
+    return page->group().captionPreferences().sortedTrackListForMenu(&trackList);
 }
 
-Vector<RefPtr<AudioTrack>> MediaControlsHost::sortedTrackListForMenu(AudioTrackList* trackList)
+Vector<RefPtr<AudioTrack>> MediaControlsHost::sortedTrackListForMenu(AudioTrackList& trackList)
 {
-    if (!trackList)
-        return Vector<RefPtr<AudioTrack>>();
-
     Page* page = m_mediaElement->document().page();
     if (!page)
-        return Vector<RefPtr<AudioTrack>>();
+        return { };
 
-    return page->group().captionPreferences().sortedTrackListForMenu(trackList);
+    return page->group().captionPreferences().sortedTrackListForMenu(&trackList);
 }
 
-String MediaControlsHost::displayNameForTrack(TextTrack* track)
+String MediaControlsHost::displayNameForTrack(const Optional<TextOrAudioTrack>& track)
 {
     if (!track)
         return emptyString();
@@ -117,21 +111,11 @@
     if (!page)
         return emptyString();
 
-    return page->group().captionPreferences().displayNameForTrack(track);
+    return WTF::visit([&page](auto& track) {
+        return page->group().captionPreferences().displayNameForTrack(track.get());
+    }, track.value());
 }
 
-String MediaControlsHost::displayNameForTrack(AudioTrack* track)
-{
-    if (!track)
-        return emptyString();
-
-    Page* page = m_mediaElement->document().page();
-    if (!page)
-        return emptyString();
-
-    return page->group().captionPreferences().displayNameForTrack(track);
-}
-
 TextTrack* MediaControlsHost::captionMenuOffItem()
 {
     return TextTrack::captionMenuOffItem();

Modified: trunk/Source/WebCore/Modules/mediacontrols/MediaControlsHost.h (207689 => 207690)


--- trunk/Source/WebCore/Modules/mediacontrols/MediaControlsHost.h	2016-10-21 18:45:39 UTC (rev 207689)
+++ trunk/Source/WebCore/Modules/mediacontrols/MediaControlsHost.h	2016-10-21 18:53:05 UTC (rev 207690)
@@ -29,6 +29,7 @@
 
 #include <bindings/ScriptObject.h>
 #include <wtf/RefCounted.h>
+#include <wtf/Variant.h>
 #include <wtf/Vector.h>
 #include <wtf/text/WTFString.h>
 
@@ -52,10 +53,12 @@
     static const AtomicString& alwaysOnKeyword();
     static const AtomicString& manualKeyword();
 
-    Vector<RefPtr<TextTrack>> sortedTrackListForMenu(TextTrackList*);
-    Vector<RefPtr<AudioTrack>> sortedTrackListForMenu(AudioTrackList*);
-    String displayNameForTrack(TextTrack*);
-    String displayNameForTrack(AudioTrack*);
+    Vector<RefPtr<TextTrack>> sortedTrackListForMenu(TextTrackList&);
+    Vector<RefPtr<AudioTrack>> sortedTrackListForMenu(AudioTrackList&);
+
+    using TextOrAudioTrack = WTF::Variant<RefPtr<TextTrack>, RefPtr<AudioTrack>>;
+    String displayNameForTrack(const Optional<TextOrAudioTrack>&);
+
     TextTrack* captionMenuOffItem();
     TextTrack* captionMenuAutomaticItem();
     AtomicString captionDisplayMode();

Modified: trunk/Source/WebCore/Modules/mediacontrols/MediaControlsHost.idl (207689 => 207690)


--- trunk/Source/WebCore/Modules/mediacontrols/MediaControlsHost.idl	2016-10-21 18:45:39 UTC (rev 207689)
+++ trunk/Source/WebCore/Modules/mediacontrols/MediaControlsHost.idl	2016-10-21 18:53:05 UTC (rev 207690)
@@ -34,10 +34,9 @@
     ImplementationLacksVTable,
     NoInterfaceObject,
 ] interface MediaControlsHost {
-    sequence<TextTrack> sortedTrackListForMenu(TextTrackList? trackList);
-    sequence<AudioTrack> sortedTrackListForMenu(AudioTrackList? trackList);
-    DOMString displayNameForTrack(TextTrack? track);
-    DOMString displayNameForTrack(AudioTrack? track);
+    sequence<TextTrack> sortedTrackListForMenu(TextTrackList trackList);
+    sequence<AudioTrack> sortedTrackListForMenu(AudioTrackList trackList);
+    DOMString displayNameForTrack((TextTrack or AudioTrack)? track);
     readonly attribute TextTrack captionMenuOffItem;
     readonly attribute TextTrack captionMenuAutomaticItem;
     readonly attribute DOMString captionDisplayMode;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to