Adding overflow menu to media player

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

Index: third_party/WebKit/Source/core/html/shadow/MediaControlElements.h
diff --git a/third_party/WebKit/Source/core/html/shadow/MediaControlElements.h b/third_party/WebKit/Source/core/html/shadow/MediaControlElements.h
index c9aa995280ae8e19d2a823355e5b4e0ec8ad926f..bf89d1c1271f4b7de83e591724200917d1da59c0 100644
--- a/third_party/WebKit/Source/core/html/shadow/MediaControlElements.h
+++ b/third_party/WebKit/Source/core/html/shadow/MediaControlElements.h
@@ -31,6 +31,7 @@
 #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 {
 
@@ -94,6 +95,8 @@ public:
     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&);
 
@@ -140,6 +143,8 @@ public:
 
     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&);
 
@@ -174,6 +179,38 @@ private:
 
 // ----------------------------
 
+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&);
@@ -184,7 +221,6 @@ public:
     // is supported by HTMLMediaElement, see https://crbug.com/137275
     void setPosition(double);
     void setDuration(double);
-
 private:
     explicit MediaControlTimelineElement(MediaControls&);
 
@@ -202,6 +238,8 @@ public:
 
     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&);
 
@@ -218,6 +256,8 @@ public:
 
     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();