Add download button to MediaDocument on Android.

third_party/WebKit/Source/core/html/MediaDocument.cpp

Index: third_party/WebKit/Source/core/html/MediaDocument.cpp
diff --git a/third_party/WebKit/Source/core/html/MediaDocument.cpp b/third_party/WebKit/Source/core/html/MediaDocument.cpp
index 525d869dbb2ff380d5110f98d9e9b3d3e17ac593..36aa501592d2b7ab87bdcc71cfb7d90d652c38da 100644
--- a/third_party/WebKit/Source/core/html/MediaDocument.cpp
+++ b/third_party/WebKit/Source/core/html/MediaDocument.cpp
@@ -29,8 +29,10 @@
 #include "core/HTMLNames.h"
 #include "core/dom/ElementTraversal.h"
 #include "core/dom/RawDataDocumentParser.h"
+#include "core/events/EventListener.h"
 #include "core/events/KeyboardEvent.h"
 #include "core/frame/LocalFrame.h"
+#include "core/html/HTMLAnchorElement.h"
 #include "core/html/HTMLBodyElement.h"
 #include "core/html/HTMLHeadElement.h"
 #include "core/html/HTMLHtmlElement.h"
@@ -41,6 +43,7 @@
 #include "core/loader/FrameLoader.h"
 #include "core/loader/FrameLoaderClient.h"
 #include "platform/KeyboardCodes.h"
+#include "platform/text/PlatformLocale.h"
 
 namespace blink {
 
@@ -68,6 +71,42 @@ private:
     bool m_didBuildDocumentStructure;
 };
 
+class MediaEventListener : public EventListener {

[esprehn, 2016/03/11 00:35:32]

final

[qinmin, 2016/03/11 23:50:55]

Done. This class is no longer needed

+public:
+    static PassRefPtrWillBeRawPtr<MediaEventListener> create(MediaDocument* document)
+    {
+        return adoptRefWillBeNoop(new MediaEventListener(document));
+    }
+
+    static const MediaEventListener* cast(const EventListener* listener)

[esprehn, 2016/03/11 00:35:32]

this cast method doesn't seem used outside ==, just inline the type() == check
there.

[qinmin, 2016/03/11 23:50:55]

Done. Class removed

+    {
+        return listener->type() == MediaEventListenerType
+            ? static_cast<const MediaEventListener*>(listener) : 0;
+    }
+
+    bool operator==(const EventListener& other) const override;
+
+    DEFINE_INLINE_VIRTUAL_TRACE()
+    {
+        visitor->trace(m_doc);
+        EventListener::trace(visitor);
+    }
+
+private:
+    MediaEventListener(MediaDocument* document)

[esprehn, 2016/03/11 00:35:33]

explicit

[qinmin, 2016/03/11 23:50:55]

Class removed.

+        : EventListener(MediaEventListenerType)
+        , m_doc(document)

[esprehn, 2016/03/11 00:35:33]

m_document, don't abbreviate

[qinmin, 2016/03/11 23:50:55]

Class removed

+    {
+    }
+
+    virtual void handleEvent(ExecutionContext*, Event* event)
+    {
+        m_doc->updateLayout();

[esprehn, 2016/03/11 00:35:33]

you don't need the m_document thing, just do toDocument on the ExecutionContext
argument, then check isMediaDocument(), then
toMediaDocument(document)->doThing()

[qinmin, 2016/03/11 23:50:55]

Class removed

+    }
+
+    RawPtrWillBeMember<MediaDocument> m_doc;
+};
+
 void MediaDocumentParser::createDocumentStructure()
 {
     ASSERT(document());
@@ -98,6 +137,39 @@ void MediaDocumentParser::createDocumentStructure()
     media->appendChild(source.release());
 
     RefPtrWillBeRawPtr<HTMLBodyElement> body = HTMLBodyElement::create(*document());
+
+    if (RuntimeEnabledFeatures::mediaDocumentDownloadButtonEnabled()) {
+        RefPtrWillBeRawPtr<EventListener> listener = MediaEventListener::create(static_cast<MediaDocument*>(document()));

[esprehn, 2016/03/11 00:35:33]

toMediaDocument, don't static cast. If toMediaDocument doesn't exist lets add it
with the casting macros

[qinmin, 2016/03/11 23:50:55]

Line removed

+        media->addEventListener("resize", listener, false);

[esprehn, 2016/03/11 00:35:33]

forcing a layout for ever resize event doesn't make sense, don't do that

[qinmin, 2016/03/11 23:50:55]

removed

+
+        RefPtrWillBeRawPtr<HTMLAnchorElement> anchor = HTMLAnchorElement::create(*document());
+        anchor->setAttribute(downloadAttr, "");
+        anchor->setURL(document()->url());
+        anchor->setInnerText(document()->getCachedLocale(document()->contentLanguage()).queryString(WebLocalizedString::DownloadButtonLabel), ASSERT_NO_EXCEPTION);

[esprehn, 2016/03/11 00:35:32]

setTextContent, don't use setInnerText

[qinmin, 2016/03/11 23:50:55]

Done.

+        // TODO(qinmin): Move these css to MediaControlsAndroidNew.css when http://crbug.com/592080 is fixed.
+        anchor->setAttribute(
+            styleAttr,
+            "position: absolute;"
+            "display: block;"
+            "width: 120px;"
+            "height: 36px;"
+            "background: #4285F4;"
+            "font-family: Roboto;"
+            "font-size: 14px;"
+            "border-radius: 5px;"
+            "color: white;"
+            "font-weight: bold;"
+            "text-decoration:none;"

[esprehn, 2016/03/11 00:35:33]

missing space, also this is so much stuff. Is there really no .css file for
media documents? We should really add one instead

[qinmin, 2016/03/11 23:50:55]

Done. As I mentioned in the TODO above, these css stule can be moved to
MediaCOntrolsAndroidNew.css.
However, there is currently a layout bug(http://crbug.com/592080) preventing us
from doing this.
If these css style are moved to the .css file, then when the MediaDocument get
loaded, the layout is incorrect as if the css is not applied.
But when I reload the page, the css will be applied. So somehow we are not
applying the css correctly for MediaDocument, and I suspect the same happens for
ImageDocument, etc. I have talked to xianzhu and he thinks that we are probably
missing one layout if the css is external.
So I will leave this inside this file until that bug is addressed.

+            "text-transform: uppercase;"
+            "text-align: center;"
+            "line-height: 36px;"
+            "margin: auto;"
+            "top: 108px;"
+            "bottom: 0;"
+            "left: 0;"
+            "right: 0;");
+        body->appendChild(anchor.release());
+    }
     body->appendChild(media.release());
 
     rootElement->appendChild(head.release());
@@ -119,6 +191,13 @@ void MediaDocumentParser::appendBytes(const char*, size_t)
     finish();
 }
 
+bool MediaEventListener::operator==(const EventListener& listener) const
+{
+    if (const MediaEventListener* mediaEventListener = MediaEventListener::cast(&listener))
+        return m_doc == mediaEventListener->m_doc;
+    return false;
+}
+
 MediaDocument::MediaDocument(const DocumentInit& initializer)
     : HTMLDocument(initializer, MediaDocumentClass)
 {
@@ -151,4 +230,14 @@ void MediaDocument::defaultEventHandler(Event* event)
     }
 }
 
+void MediaDocument::updateLayout()

[esprehn, 2016/03/11 00:35:33]

updateLayout is a real method you're shadowing, you need a different name. This
should just be handleResize or something.

[qinmin, 2016/03/11 23:50:55]

removed

+{
+    HTMLAnchorElement* anchorElement = Traversal<HTMLAnchorElement>::firstWithin(*this);
+    HTMLMediaElement* mediaElement = Traversal<HTMLMediaElement>::firstWithin(*this);
+
+    int top = mediaElement->offsetTop() + mediaElement->offsetHeight() + 36;

[esprehn, 2016/03/11 00:35:33]

this doesn't seem right, what is 36? This seems really hacky setting the top and
marginTop like this.

[qinmin, 2016/03/11 23:50:55]

This should actually be 32. It is to move the button 32 px below the
mediaElement to conform to the android material design.
See
https://docs.google.com/document/d/1SqmvJjxhmDvdgzqQEIls129ZYq2QFnvcSYbwbTfZ4...
for the UI design.
Anyway, moved this to the css above

+    anchorElement->setInlineStyleProperty(CSSPropertyTop, top, CSSPrimitiveValue::UnitType::Pixels);
+    anchorElement->setInlineStyleProperty(CSSPropertyMarginTop, 0, CSSPrimitiveValue::UnitType::Pixels);

[esprehn, 2016/03/11 00:35:32]

huh? why always set to 0?

[qinmin, 2016/03/11 23:50:55]

This is margin top. Since both the video and anchor position are absolute, so as
long as the button is 36px below the video, we don't need margin top.

+}
+
 } // namespace blink