Add media controls to MediaSession in Blink

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

 // Copyright 2015 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.
 
 #ifndef MediaSession_h
 #define MediaSession_h
 
 #include "bindings/core/v8/ScriptWrappable.h"
+#include "core/events/EventTarget.h"
 #include "modules/ModulesExport.h"
 #include "platform/heap/Handle.h"
 #include "public/platform/modules/mediasession/media_session.mojom-blink.h"
 #include <memory>
 
 namespace blink {
 
 class MediaMetadata;
 class ScriptState;
 
-class MODULES_EXPORT MediaSession final
-    : public GarbageCollectedFinalized<MediaSession>,
-      public ScriptWrappable {
+class MODULES_EXPORT MediaSession final : public EventTargetWithInlineData {
   DEFINE_WRAPPERTYPEINFO();
 
  public:
-  static MediaSession* create();
+  static MediaSession* create(ScriptState*);
 
-  void setMetadata(ScriptState*, MediaMetadata*);
-  MediaMetadata* metadata(ScriptState*) const;
+  void setMetadata(MediaMetadata*);
+  MediaMetadata* metadata() const;
+
+  // EventTarget implementation.
+  const WTF::AtomicString& interfaceName() const override;
+  ExecutionContext* getExecutionContext() const override;
+
+  // Event getters and setters. Not using DEFINE_ATTRIBUTE_EVENT_LISTENER macro

[whywhat, 2016/10/17 15:32:35]

I thought the common way to do it was to override
attachEventListener/removeEventListener from EventTarget and handle custom
actions there.

Unless there're other precedents or reasons why doing a custom thing is better,
I'd prefer you use the usual macro (imagine if the macro is modified to support
some shiny new thing - how will the change owner know to update this class?)

[whywhat, 2016/10/17 15:41:46]

s/attachEventListener/addEventListener
Another downside is that Web authors would rather use addEventListener instead
of assigning one directly and this would not go via setOnplaypause at all (while
the opposite is true if you use the standard macro.).

[Zhiqiang Zhang (Slow), 2016/10/17 17:19:08]

Thanks. Done.
Though there might be a drawback when the page sets the listener to a new value,
since MediaSessionService will receive an unset and a set signal. Then our media
buttons might flicker, but I think it's probably fine.

[whywhat, 2016/10/17 17:32:06]

That's already the case with add/removeEventListener the page will more likely
use anyway if it's not careful with the order (setting event handler as an
attribute is a deprecated practice AFAIK) :/ So it's a limitation of the spec
really. Perhaps we could bundle requests to the Mojo service to avoid the issue.

Or we could discuss it at the spec level again (whether we need something extra
to the event handlers for the page to define the supported events) or just wait
to see if it becomes a problem - I'm not sure the pages would change the event
handlers often rather than from null to something and back.

+  // since there are additional actions in setters.
+  EventListener* onplay();
+  EventListener* onpause();
+  EventListener* onplaypause();
+  EventListener* onprevioustrack();
+  EventListener* onnexttrack();
+  EventListener* onseekforward();
+  EventListener* onseekbackward();
+
+  void setOnplay(EventListener*);
+  void setOnpause(EventListener*);
+  void setOnplaypause(EventListener*);
+  void setOnprevioustrack(EventListener*);
+  void setOnnexttrack(EventListener*);
+  void setOnseekforward(EventListener*);
+  void setOnseekbackward(EventListener*);
 
   DECLARE_VIRTUAL_TRACE();
 
  private:
   friend class MediaSessionTest;
 
-  MediaSession();
+  MediaSession(ScriptState*);
 
   // Returns null when the ExecutionContext is not document.
   mojom::blink::MediaSessionService* getService(ScriptState*);
 
+  RefPtr<ScriptState> m_scriptState;
   Member<MediaMetadata> m_metadata;
   mojom::blink::MediaSessionServicePtr m_service;
 };
 
 }  // namespace blink
 
 #endif  // MediaSession_h