[Webkit-unassigned] [Bug 145530] Add stub implementation of MediaSession, part of Media Session spec

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=145530

--- Comment #4 from Matt Rajca <mrajca at apple.com> ---
Comment on attachment 254028
  --> https://bugs.webkit.org/attachment.cgi?id=254028
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=254028&action=review

>> Source/WebCore/Modules/mediasession/MediaSession.h:32
>> +#include <wtf/RefCounted.h>
> 
> If we include "MediaRemoteControls.h" it seems unlikely we need to include this also.

Removed.

>> Source/WebCore/Modules/mediasession/MediaSession.h:37
>> +class MediaSession : public RefCounted<MediaSession> {
> 
> I suggest marking this class final.

I agree.

>> Source/WebCore/Modules/mediasession/MediaSession.h:46
>> +    MediaSession(ScriptExecutionContext&, Kind);
> 
> Does this compile? Does the bindings generator really turn those strings into MediaSession::Kind values?

Not if we compile JSMediaSession with the project. Fixed.

>> Source/WebCore/Modules/mediasession/MediaSession.h:47
>> +    virtual ~MediaSession();
> 
> Why virtual? There doesn’t seem to be any polymorphism in this class, so it seems we should avoid marking it virtual.

Done, and annotated the interface in the IDL file with ImplementationLacksVTable.

>> Source/WebCore/Modules/mediasession/MediaSession.h:50
>> +    MediaRemoteControls *controls() const { return m_controls.get(); }
> 
> Wrong position of the * character here.

Fixed.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150602/d99f6ecc/attachment.html>