[Webkit-unassigned] [Bug 58346] Convert media controls hooks to a client interface

[bugzilla-daemon at webkit.org]

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


Dimitri Glazkov (Google) <dglazkov at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #89917|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #12 from Dimitri Glazkov (Google) <dglazkov at chromium.org>  2011-04-16 09:52:59 PST ---
(From update of attachment 89917)
View in context: https://bugs.webkit.org/attachment.cgi?id=89917&action=review

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:22965
> +				1F0D6E0E13590DE700236592 /* MediaControlBase.h in Headers */,

The XCode changes look like they haven't been sorted. Can you run sort-XCode-project-files on this?

> Source/WebCore/html/shadow/MediaControlBase.h:34
> +#include <HTMLDivElement.h>
> +#include <HTMLMediaElement.h>
> +#include <HTMLNames.h>

"", not <>

> Source/WebCore/html/shadow/MediaControlBase.h:38
> +class MediaControlBase : public HTMLDivElement {

I am sorry about flip-flopping here, but looking at callsites, it's clear that they are working with an interface, not a base class. So we should call this new thing MediaControls, not MediaControlBase.

> Source/WebCore/html/shadow/MediaControlBase.h:42
> +    // This function is to be implemented in your platform-specific media

port-specific.

> Source/WebCore/html/shadow/MediaControlBase.h:73
> +    MediaControlBase(HTMLMediaElement* mediaElement)
> +        : HTMLDivElement(HTMLNames::divTag, mediaElement->document()) { }

I know you want to keep this change to only have a header file, but you could save a couple of header includes if you just have a source file with it.

> Source/WebCore/html/shadow/MediaControlBase.h:81
> +#endif

extra break after first endif would be helpful.

> Source/WebCore/html/shadow/MediaControlRootElement.h:28
> +#ifndef MediaControlRootElement_h
> +#define MediaControlRootElement_h

Ha! Thanks for noticing this one.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.