[Blink>MediaSession] Use setActionCallback() instead of event listeners for media control actions

third_party/WebKit/Source/modules/mediasession/MediaSession.cpp

Index: third_party/WebKit/Source/modules/mediasession/MediaSession.cpp
diff --git a/third_party/WebKit/Source/modules/mediasession/MediaSession.cpp b/third_party/WebKit/Source/modules/mediasession/MediaSession.cpp
index a1d0e462e4638bdc5dfb0cec0dc7fe2aeccd38a0..9e190b25fdc0c9e300e3a1a9d3e3724731521a37 100644
--- a/third_party/WebKit/Source/modules/mediasession/MediaSession.cpp
+++ b/third_party/WebKit/Source/modules/mediasession/MediaSession.cpp
@@ -4,12 +4,11 @@
 
 #include "modules/mediasession/MediaSession.h"
 
+#include "bindings/modules/v8/MediaSessionActionCallback.h"
 #include "core/dom/Document.h"
 #include "core/dom/DocumentUserGestureToken.h"
 #include "core/dom/ExecutionContext.h"
-#include "core/events/Event.h"
 #include "core/frame/LocalFrame.h"
-#include "modules/EventTargetModules.h"
 #include "modules/mediasession/MediaMetadata.h"
 #include "modules/mediasession/MediaMetadataSanitizer.h"
 #include "platform/UserGestureIndicator.h"
@@ -24,19 +23,29 @@ namespace {
 using ::blink::mojom::blink::MediaSessionAction;
 
 const AtomicString& mojomActionToEventName(MediaSessionAction action) {
+  DEFINE_STATIC_LOCAL(const AtomicString, playActionName, ("play"));
+  DEFINE_STATIC_LOCAL(const AtomicString, pauseActionName, ("pause"));
+  DEFINE_STATIC_LOCAL(const AtomicString, previousTrackActionName,
+                      ("previoustrack"));
+  DEFINE_STATIC_LOCAL(const AtomicString, nextTrackActionName, ("nexttrack"));
+  DEFINE_STATIC_LOCAL(const AtomicString, seekBackwardActionName,
+                      ("seekbackward"));
+  DEFINE_STATIC_LOCAL(const AtomicString, seekForwardActionName,
+                      ("seekforward"));
+
   switch (action) {
     case MediaSessionAction::PLAY:
-      return EventTypeNames::play;
+      return playActionName;
     case MediaSessionAction::PAUSE:
-      return EventTypeNames::pause;
+      return pauseActionName;
     case MediaSessionAction::PREVIOUS_TRACK:
-      return EventTypeNames::previoustrack;
+      return previousTrackActionName;
     case MediaSessionAction::NEXT_TRACK:
-      return EventTypeNames::nexttrack;
+      return nextTrackActionName;
     case MediaSessionAction::SEEK_BACKWARD:
-      return EventTypeNames::seekbackward;
+      return seekBackwardActionName;
     case MediaSessionAction::SEEK_FORWARD:
-      return EventTypeNames::seekforward;
+      return seekForwardActionName;
     default:
       NOTREACHED();
   }
@@ -44,18 +53,18 @@ const AtomicString& mojomActionToEventName(MediaSessionAction action) {
 }
 
 WTF::Optional<MediaSessionAction> eventNameToMojomAction(
-    const AtomicString& eventName) {
-  if (EventTypeNames::play == eventName)
+    const String& eventName) {

[mlamouri (slow - plz ping), 2016/12/20 13:25:57]

s/eventName/actionName/?

[Zhiqiang Zhang (Slow), 2016/12/20 13:53:06]

Done.

+  if ("play" == eventName)
     return MediaSessionAction::PLAY;
-  if (EventTypeNames::pause == eventName)
+  if ("pause" == eventName)
     return MediaSessionAction::PAUSE;
-  if (EventTypeNames::previoustrack == eventName)
+  if ("previoustrack" == eventName)
     return MediaSessionAction::PREVIOUS_TRACK;
-  if (EventTypeNames::nexttrack == eventName)
+  if ("nexttrack" == eventName)
     return MediaSessionAction::NEXT_TRACK;
-  if (EventTypeNames::seekbackward == eventName)
+  if ("seekbackward" == eventName)
     return MediaSessionAction::SEEK_BACKWARD;
-  if (EventTypeNames::seekforward == eventName)
+  if ("seekforward" == eventName)
     return MediaSessionAction::SEEK_FORWARD;
 
   NOTREACHED();
@@ -140,12 +149,20 @@ void MediaSession::onMetadataChanged() {
       m_metadata, getExecutionContext()));
 }
 
-const WTF::AtomicString& MediaSession::interfaceName() const {
-  return EventTargetNames::MediaSession;
-}
+void MediaSession::setActionCallback(const String& action,
+                                     MediaSessionActionCallback* callback) {
+  m_actionCallbacks.set(
+      action, TraceWrapperMember<MediaSessionActionCallback>(this, callback));
 
-ExecutionContext* MediaSession::getExecutionContext() const {
-  return ContextLifecycleObserver::getExecutionContext();
+  mojom::blink::MediaSessionService* service = getService();
+  if (!service)
+    return;
+  auto mojomAction = eventNameToMojomAction(action);
+  DCHECK(mojomAction.has_value());
+  if (callback)
+    service->EnableAction(mojomAction.value());
+  else
+    service->DisableAction(mojomAction.value());

[mlamouri (slow - plz ping), 2016/12/20 13:25:57]

Would it make sense to have timer in order to bundle multiple action
removed/added in the same event loop?

It's okay to do in a follow-up.

[Zhiqiang Zhang (Slow), 2016/12/20 13:53:06]

We are still passing individual actions through mojo. If we do a timer, we might
need to pass the whole set of actions instead. WDYT?

 }
 
 mojom::blink::MediaSessionService* MediaSession::getService() {
@@ -171,43 +188,37 @@ mojom::blink::MediaSessionService* MediaSession::getService() {
   return m_service.get();
 }
 
-bool MediaSession::addEventListenerInternal(
-    const AtomicString& eventType,
-    EventListener* listener,
-    const AddEventListenerOptionsResolved& options) {
-  if (mojom::blink::MediaSessionService* service = getService()) {
-    auto mojomAction = eventNameToMojomAction(eventType);
-    DCHECK(mojomAction.has_value());
-    service->EnableAction(mojomAction.value());
-  }
-  return EventTarget::addEventListenerInternal(eventType, listener, options);
-}
-
-bool MediaSession::removeEventListenerInternal(
-    const AtomicString& eventType,
-    const EventListener* listener,
-    const EventListenerOptions& options) {
-  if (mojom::blink::MediaSessionService* service = getService()) {
-    auto mojomAction = eventNameToMojomAction(eventType);
-    DCHECK(mojomAction.has_value());
-    service->DisableAction(mojomAction.value());
-  }
-  return EventTarget::removeEventListenerInternal(eventType, listener, options);
-}
-
 void MediaSession::DidReceiveAction(
     blink::mojom::blink::MediaSessionAction action) {
   DCHECK(getExecutionContext()->isDocument());
   Document* document = toDocument(getExecutionContext());
   UserGestureIndicator gestureIndicator(
       DocumentUserGestureToken::create(document));
-  dispatchEvent(Event::create(mojomActionToEventName(action)));
+
+  auto iter = m_actionCallbacks.find(mojomActionToEventName(action));
+  if (iter == m_actionCallbacks.end())
+    return;
+
+  iter->value->call(this);
+}
+
+void MediaSession::setV8ReferencesForCallbacks(
+    v8::Isolate* isolate,
+    const v8::Persistent<v8::Object>& wrapper) {
+  for (auto callback : m_actionCallbacks.values())
+    callback->setWrapperReference(isolate, wrapper);
 }
 
 DEFINE_TRACE(MediaSession) {
   visitor->trace(m_metadata);
-  EventTargetWithInlineData::trace(visitor);
+  visitor->trace(m_actionCallbacks);
   ContextLifecycleObserver::trace(visitor);
 }
 
+DEFINE_TRACE_WRAPPERS(MediaSession) {
+  for (auto callback : m_actionCallbacks.values()) {
+    visitor->traceWrappers(callback);
+  }

[mlamouri (slow - plz ping), 2016/12/20 13:25:57]

style: you can drop the { }

[Zhiqiang Zhang (Slow), 2016/12/20 13:53:06]

Done.

+}
+
 }  // namespace blink