Media Controls: delegate 'volumechange' and 'focusin' handling to an EventListener.

third_party/WebKit/Source/core/html/shadow/MediaControlsMediaEventListener.cpp

Index: third_party/WebKit/Source/core/html/shadow/MediaControlsMediaEventListener.cpp
diff --git a/third_party/WebKit/Source/core/html/shadow/MediaControlsMediaEventListener.cpp b/third_party/WebKit/Source/core/html/shadow/MediaControlsMediaEventListener.cpp
new file mode 100644
index 0000000000000000000000000000000000000000..c607fce4e0be6578d1af1dad39884f906e9f7617
--- /dev/null
+++ b/third_party/WebKit/Source/core/html/shadow/MediaControlsMediaEventListener.cpp
@@ -0,0 +1,48 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "core/html/shadow/MediaControlsMediaEventListener.h"
+
+#include "core/events/Event.h"
+#include "core/html/HTMLMediaElement.h"
+#include "core/html/shadow/MediaControls.h"
+
+namespace blink {
+
+MediaControlsMediaEventListener::MediaControlsMediaEventListener(
+    MediaControls* mediaControls)
+    : EventListener(CPPEventListenerType), m_mediaControls(mediaControls) {
+  m_mediaControls->m_mediaElement->addEventListener(

[foolip, 2016/11/08 10:28:29]

Can you test what happens with this code?

<video></video>
<script>
var video = document.querySelector('video');
video.addEventListener('volumechange', function(event) {
  event.stopImmediatePropagation();
});
video.controls = true;
</script>

[mlamouri (slow - plz ping), 2016/11/11 05:29:17]

Added. Looks okay.

[foolip, 2016/11/11 09:50:20]

Surprising. Do you know why it works, are internal event handlers always handled
first?

[mlamouri (slow - plz ping), 2016/11/11 19:13:27]

Hmm, I think it's because we eagerly create the controls when an element is
added to the DOM even if it doesn't have a controls attribute. I was able to
write a test case where things didn't work out very well.

I'm not super familiar with event propagation but would handling the event on
the capture phase highly reduce the chances of this happening?

In long run (outside of this CL), I see two ways we could tackle this:
1. we suppose that a website blocking such events probably use its own control
and simply ignore the problem (I wonder what would happen on Safari because they
have controls in JS IIRC)
2. we add a new type of EventListener in our internal code like to make sure
they are not blocked by content event listeners

I'm in favour of 2 because we had this problem in a few situations and if we
want to reduce internal plumbing to handle certain things, we need to make sure
they are reliable. I will talk to some people to see what we can do. Though,
what do you think?

+      EventTypeNames::volumechange, this, false);
+  m_mediaControls->m_mediaElement->addEventListener(EventTypeNames::focusin,
+                                                    this, false);
+}
+
+bool MediaControlsMediaEventListener::operator==(
+    const EventListener& other) const {
+  return this == &other;
+}
+
+void MediaControlsMediaEventListener::handleEvent(
+    ExecutionContext* executionContext,
+    Event* event) {
+  if (event->type() == EventTypeNames::volumechange) {
+    m_mediaControls->onVolumeChange();
+    return;
+  }
+
+  if (event->type() == EventTypeNames::focusin) {
+    m_mediaControls->onFocusIn();
+    return;
+  }
+
+  NOTREACHED();
+}
+
+DEFINE_TRACE(MediaControlsMediaEventListener) {
+  EventListener::trace(visitor);
+  visitor->trace(m_mediaControls);
+}
+
+}  // namespace blink