Enabling autoplay and fullscreen for downloaded media

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 5d36b40caebdd1caa4e416f8f26e4811707f98ba..db66da4ebcccd9f14068b7e9e619ee724fc88c0b 100644
--- a/third_party/WebKit/Source/core/html/MediaDocument.cpp
+++ b/third_party/WebKit/Source/core/html/MediaDocument.cpp
@@ -27,13 +27,16 @@
 
 #include "bindings/core/v8/ExceptionState.h"
 #include "core/HTMLNames.h"
+#include "core/dom/DocumentUserGestureToken.h"
 #include "core/dom/ElementTraversal.h"
+#include "core/dom/GetRootNodeOptions.h"
 #include "core/dom/RawDataDocumentParser.h"
 #include "core/dom/shadow/ShadowRoot.h"
 #include "core/events/Event.h"
 #include "core/events/EventListener.h"
 #include "core/events/KeyboardEvent.h"
 #include "core/frame/LocalFrame.h"
+#include "core/frame/Settings.h"
 #include "core/frame/UseCounter.h"
 #include "core/html/HTMLAnchorElement.h"
 #include "core/html/HTMLBodyElement.h"
@@ -50,6 +53,7 @@
 #include "core/loader/FrameLoaderClient.h"
 #include "platform/Histogram.h"
 #include "platform/KeyboardCodes.h"
+#include "platform/UserGestureIndicator.h"
 #include "platform/text/PlatformLocale.h"
 
 namespace blink {
@@ -113,6 +117,29 @@ class MediaDownloadEventListener final : public EventListener {
   bool m_clicked;
 };
 
+class MediaLoadedEventListener final : public EventListener {
+ public:

[mlamouri (slow - plz ping), 2017/02/21 11:33:03]

`WTF_MAKE_NONCOPYABLE(MediaLoadedEventListener);` ?

[shaktisahu, 2017/02/21 16:51:57]

Done.

+  static MediaLoadedEventListener* create() {
+    return new MediaLoadedEventListener();
+  }

[mlamouri (slow - plz ping), 2017/02/21 11:33:03]

What's the benefit of using create over the ctor?

[shaktisahu, 2017/02/21 16:51:57]

hmm.. I wanted to keep it consistent with the other subclasses of EventListener
(see above).

+
+  bool operator==(const EventListener& other) const override {
+    return this == &other;
+  }
+
+ private:
+  MediaLoadedEventListener() : EventListener(CPPEventListenerType) {}

[mlamouri (slow - plz ping), 2017/02/21 11:33:03]

`= default;` instead of `{}`

[shaktisahu, 2017/02/21 16:51:57]

I think default works with only empty initialization list.

+
+  void handleEvent(ExecutionContext* context, Event* event) override {
+    HTMLVideoElement* media = Traversal<HTMLVideoElement>::firstWithin(
+        *event->target()->toNode()->getRootNode(GetRootNodeOptions()));

[mlamouri (slow - plz ping), 2017/02/21 11:33:03]

Why are you using Traversal<> here? Shouldn't `event->target()` be the video
element?

[shaktisahu, 2017/02/21 16:51:57]

Done.

+    UserGestureIndicator gesture(
+        DocumentUserGestureToken::create(&media->document()));
+    media->webkitEnterFullscreen();
+    media->play();

[mlamouri (slow - plz ping), 2017/02/21 11:33:03]

Maybe it would be better to start playback when we have transitionned to
fullscreen but that might be good enough :)

[shaktisahu, 2017/02/21 16:51:57]

I plan to revisit this depending on how the experience looks. Also I wanted to
see if I could get rid of the control panel that shows at the start, but may be
that's for later. For the released build, it looks smooth though, but there is
room for improvement.

+  }
+};
+
 void MediaDocumentParser::createDocumentStructure() {
   DCHECK(document());
   HTMLHtmlElement* rootElement = HTMLHtmlElement::create(*document());
@@ -163,6 +190,12 @@ void MediaDocumentParser::createDocumentStructure() {
   HTMLContentElement* content = HTMLContentElement::create(*document());
   div->appendChild(content);
 
+  if (document()->settings() &&
+      document()->settings()->getEmbeddedMediaExperienceEnabled()) {
+    EventListener* listener = MediaLoadedEventListener::create();
+    media->addEventListener(EventTypeNames::loadedmetadata, listener, false);

[mlamouri (slow - plz ping), 2017/02/21 11:33:03]

Would it make sense to do:
```
AddEventListenerOptions options;
options.setOnce(true);
AddEventListenerOptionsOrBoolean optionsOrBoolean;
options.setAddEventListenerOptions(options);
media->addEventListener(EventTypeNames::loadedMetadata, optionsOrBoolean);
```

It might be cleaner because the listener would be free'd up after being fired.
It would be nicer if we had an internal method that takes
AddEventListenerOptions but that's maybe for later :)

[shaktisahu, 2017/02/21 16:51:57]

Done.

+  }
+
   if (RuntimeEnabledFeatures::mediaDocumentDownloadButtonEnabled()) {
     HTMLAnchorElement* anchor = HTMLAnchorElement::create(*document());
     anchor->setAttribute(downloadAttr, "");