[webkit-reviews] review granted: [Bug 30435] Update media layout tests to be codec aware and a V8 binding fix. : [Attachment 45857] Round 7

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted Andrew Scherkus
<scherkus at chromium.org>'s request for review:
Bug 30435: Update media layout tests to be codec aware and a V8 binding fix.
https://bugs.webkit.org/show_bug.cgi?id=30435

Attachment 45857: Round 7
https://bugs.webkit.org/attachment.cgi?id=45857&action=review

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
(In reply to comment #47)
> (In reply to comment #46)
> > (From update of attachment 45833 [details] [details])
> > 
> > > +        * platform/mac-leopard/media/video-no-audio-expected.checksum:
New results due to changing source video.
> > > +        * platform/mac-leopard/media/video-no-audio-expected.png: Ditto.

> > > +        * platform/mac-leopard/media/video-no-audio-expected.txt: Ditto.

> > 
> > You should check these in for SnowLeopard as well.
> 
> platform/mac-snowleopard/media doesn't exist... where you referring to
> platform/mac/media?
> 
  Yes, SnowLeopard is the current mac platform.

> > > -    waitForEvent("loadstart", function () {
testExpected("relativeURL(audio.currentSrc)", "content/test.wav"); });
> > > +    waitForEvent("loadstart", function () {
testExpected("relativeURL(audio.currentSrc)", audioFile); });
> > 
> > Eric's comment is correct. While all ports support wav at the moment, the
spec
> > doesn't require it so you should change this to use findMediaFile(). 
> 
> I'm confused... isn't that what my patch is doing in
> LayoutTests/media/audio-play-event.html ?
>
  Yes, but I copied the wrong line from the diff. The problem is in
audio-play-event.html where you are calling "run()" with the name of a platform
specific media file. "run()" logs the name of the file passed to it so the test
result has the platform specific name.

r=me with these changes