Bug 21930 – Add MediaPlayerPrivateChromium to MediaPlayer

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Andrew Scherkus]

Adds in a MediaPlayerPrivate implementation for Chromium using PLATFORM(CHROMIUM) guard.

[Andrew Scherkus]

Created attachment 24723 [details]
patch

[Sam Weinig]

Comment on attachment 24723 [details]
patch

This needs a ChangeLog and should really also include the new MediaPlayerPrivateChromium.h file as well.  I no we have let quite a few of these header #include changes, but I am beginning to think we should not.

[Darin Fisher (:fishd, Google)]

Sam, please see Brett's post to webkit-dev on the subject:
https://lists.webkit.org/pipermail/webkit-dev/2008-October/005526.html

[Sam Weinig]

Ok, but it still needs a changelog, which was my main complaint :)

[Andrew Scherkus]

Yes that was my mistake :)

Uploading a new patch as we speak!

[Andrew Scherkus]

Created attachment 24750 [details]
Includes ChangeLog entry and I fixed the order (we no longer define WIN/MAC/etc. so it doesn't have to be first)

[Darin Adler]

Comment on attachment 24750 [details]
Includes ChangeLog entry and I fixed the order (we no longer define WIN/MAC/etc. so it doesn't have to be first)

> Index: ChangeLog
> ===================================================================
> --- ChangeLog	(revision 37973)
> +++ ChangeLog	(working copy)
> @@ -1,3 +1,12 @@
> [phone]  Andrew Scherkus  <[e-mail]>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Add MediaPlayerPrivate header include for Chromium platform.
> +
> +        * WebCore\platform\graphics\MediaPlayer.cpp
> +:
> +

I'm not sure how you're generating ChangeLog files but there are three problems with this one:

    1) The path uses \ rather than / -- we use / in our ChangeLog even though Windows is one of the platforms we support.
    2) The colon is on a separate line after the filename.
    3) Doesn't contain a link to this bug on bugs.webkit.org; we prefer to do that.

The person who lands this patch will need to fix these problems.

r=me

[Andrew Scherkus]

Thanks for pointing out the ChangeLog mistakes.  I used prepare-ChangeLog under cygwin, but I believe the non-cygwin svn was used by mistake.  I'll fix it up for future submissions.

[Darin Fisher (:fishd, Google)]

http://trac.webkit.org/changeset/37980