Adding overflow menu to media player

third_party/WebKit/Source/core/html/shadow/MediaControlElements.h

 /*
  * Copyright (C) 2008, 2009, 2010, 2011 Apple Inc. All rights reserved.
  * Copyright (C) 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
@@ skipping 13 matching lines @@
  * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
  * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
  * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #ifndef MediaControlElements_h
 #define MediaControlElements_h
 
 #include "core/html/shadow/MediaControlElementTypes.h"
+#include <set>

[whywhat, 2016/08/19 23:56:55]

nit: empty line between system and project includes; also I think the standard
includes precede the project ones.

#include <set>

#include "core/html/..."

[kdsilva, 2016/08/24 05:33:26]

If I change the ordering, I get a presubmit error. Thoughts?

[whywhat, 2016/08/26 20:57:26]

Ok, presubmit check is the ultimate source of truth then :)

 
 namespace blink {
 
 class TextTrack;
 
 // ----------------------------
 
 class MediaControlPanelElement final : public MediaControlDivElement {
 public:
     static MediaControlPanelElement* create(MediaControls&);
@@ skipping 43 matching lines @@
 
 // ----------------------------
 
 class MediaControlMuteButtonElement final : public MediaControlInputElement {
 public:
     static MediaControlMuteButtonElement* create(MediaControls&);
 
     bool willRespondToMouseClickEvents() override { return true; }
     void updateDisplayType() override;
 
+    String getText() override;

[whywhat, 2016/08/19 23:56:55]

This returns the user visible text, right? If so, the strings need to be
internationalized so that users get translated text if they're using Chrome with
a different language.

[kdsilva, 2016/08/24 05:33:26]

Done, I think... 
I followed what closed captions did, PTAL?

[whywhat, 2016/08/26 20:57:26]

looks good

+
 private:
     explicit MediaControlMuteButtonElement(MediaControls&);
 
     void defaultEventHandler(Event*) override;
 };
 
 // ----------------------------
 
 class MediaControlPlayButtonElement final : public MediaControlInputElement {
 public:
@@ skipping 26 matching lines @@
 // ----------------------------
 
 class MediaControlToggleClosedCaptionsButtonElement final : public MediaControlInputElement {
 public:
     static MediaControlToggleClosedCaptionsButtonElement* create(MediaControls&);
 
     bool willRespondToMouseClickEvents() override { return true; }
 
     void updateDisplayType() override;
 
+    String getText() override { return "Captions"; }

[whywhat, 2016/08/19 23:56:55]

I don't think we should inline string literals in headers - they might get
multiplied in the binaries making chrome bigger.

[kdsilva, 2016/08/24 05:33:26]

Done.

+
 private:
     explicit MediaControlToggleClosedCaptionsButtonElement(MediaControls&);
 
     void defaultEventHandler(Event*) override;
 };
 
 // ----------------------------
 
 class MediaControlTextTrackListElement final : public MediaControlDivElement {
 public:
@@ skipping 14 matching lines @@
     String getTextTrackLabel(TextTrack*);
     // Creates the track element in the list when a valid track is passed in and the "Off" item when the parameter is null.
     Element* createTextTrackListItem(TextTrack*);
 
     void showTextTrackAtIndex(unsigned);
     void disableShowingTextTracks();
 };
 
 // ----------------------------
 
+class MediaControlOverflowMenu final : public MediaControlInputElement {

[whywhat, 2016/08/19 23:56:55]

nit: please add some comments to the new classes and at least public methods
where it makes sense

[kdsilva, 2016/08/24 05:33:26]

Done.

+public:
+    static MediaControlOverflowMenu* create(MediaControls&);
+
+    bool willRespondToMouseClickEvents() override { return true; }
+
+private:
+    explicit MediaControlOverflowMenu(MediaControls&);
+
+    void defaultEventHandler(Event*) override;
+};
+
+// ----------------------------
+
+class MediaControlOverflowMenuListElement final : public MediaControlDivElement {
+public:
+    static MediaControlOverflowMenuListElement* create(MediaControls&);
+
+    // A set of the MediaControlElementTypes that should appear in the
+    // overflow menu.
+    void setOverflowMenuControls(std::set<MediaControlElementType>);

[whywhat, 2016/08/19 23:56:55]

can you pass the set by (const) reference?

[kdsilva, 2016/08/24 05:33:26]

Done.

+
+    void setVisible(bool);
+
+private:
+    explicit MediaControlOverflowMenuListElement(MediaControls&);
+
+    void defaultEventHandler(Event*) override;
+};
+
+// ----------------------------
+
 class MediaControlTimelineElement final : public MediaControlInputElement {
 public:
     static MediaControlTimelineElement* create(MediaControls&);
 
     bool willRespondToMouseClickEvents() override;
 
     // FIXME: An "earliest possible position" will be needed once that concept
     // is supported by HTMLMediaElement, see https://crbug.com/137275
     void setPosition(double);
     void setDuration(double);
-
 private:
     explicit MediaControlTimelineElement(MediaControls&);
 
     void defaultEventHandler(Event*) override;
     bool keepEventInNode(Event*) override;
 };
 
 // ----------------------------
 
 class MediaControlFullscreenButtonElement final : public MediaControlInputElement {
 public:
     static MediaControlFullscreenButtonElement* create(MediaControls&);
 
     bool willRespondToMouseClickEvents() override { return true; }
 
     void setIsFullscreen(bool);
 
+    String getText() override { return "Full Screen"; }

[whywhat, 2016/08/19 23:56:55]

nit: I believe Fullscreen is one word?

[kdsilva, 2016/08/24 05:33:26]

Done.

+
 private:
     explicit MediaControlFullscreenButtonElement(MediaControls&);
 
     void defaultEventHandler(Event*) override;
 };
 
 // ----------------------------
 
 class MediaControlCastButtonElement final : public MediaControlInputElement {
 public:
     static MediaControlCastButtonElement* create(MediaControls&, bool isOverlayButton);
 
     bool willRespondToMouseClickEvents() override { return true; }
 
     void setIsPlayingRemotely(bool);
 
+    String getText() override;
+
     // This will show a cast button if it is not covered by another element.
     // This MUST be called for cast button elements that are overlay elements.
     void tryShowOverlay();
 
 private:
     explicit MediaControlCastButtonElement(MediaControls&, bool isOverlayButton);
 
     const AtomicString& shadowPseudoId() const override;
     void defaultEventHandler(Event*) override;
     bool keepEventInNode(Event*) override;
@@ skipping 49 matching lines @@
 public:
     static MediaControlCurrentTimeDisplayElement* create(MediaControls&);
 
 private:
     explicit MediaControlCurrentTimeDisplayElement(MediaControls&);
 };
 
 } // namespace blink
 
 #endif // MediaControlElements_h