Bug 21930

Summary:

Add MediaPlayerPrivateChromium to MediaPlayer

Product:

WebKit

Reporter:

Andrew Scherkus <scherkus>

Component:

Platform

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
patch	sam: review-
Includes ChangeLog entry and I fixed the order (we no longer define WIN/MAC/etc. so it doesn't have to be first)	darin: review+

Andrew Scherkus

Reported 2008-10-28 11:48:14 PDT

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

Attachments
patch (439 bytes, patch) 2008-10-28 11:52 PDT, Andrew Scherkus	sam: review-	Details Formatted Diff Diff
Includes ChangeLog entry and I fixed the order (we no longer define WIN/MAC/etc. so it doesn't have to be first) (931 bytes, patch) 2008-10-29 11:19 PDT, Andrew Scherkus	darin: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Andrew Scherkus

Comment 1 2008-10-28 11:52:18 PDT

Created attachment 24723 [details] patch

Sam Weinig

Comment 2 2008-10-28 23:34:16 PDT

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)

Comment 3 2008-10-29 11:00:08 PDT

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

Comment 4 2008-10-29 11:02:13 PDT

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

Andrew Scherkus

Comment 5 2008-10-29 11:04:29 PDT

Yes that was my mistake :) Uploading a new patch as we speak!

Andrew Scherkus

Comment 6 2008-10-29 11:19:01 PDT

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 7 2008-10-29 13:01:54 PDT

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 @@ > +2008-10-29 Andrew Scherkus <scherkus@chromium.org> > + > + 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

Comment 8 2008-10-29 13:17:12 PDT

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)

Comment 9 2008-10-29 14:45:28 PDT

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

Note You need to log in before you can comment on or make changes to this bug.