Bug 53737 – Layout Test (media/video-zoom.html) fails on Mac (crbug.com/70252)

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Ami Fischman]

Layout Test (media/video-zoom.html) fails on Mac (crbug.com/70252)

[Ami Fischman]

Created attachment 81145 [details]
Patch

[Ami Fischman]

Reviewer: please flip commit-queue+ bit when ready.
(I'm a chromium/webkit noob and didn't realize I should pass --request-commit to webkit-patch upload)

[Eric Seidel (no email)]

Comment on attachment 81145 [details]
Patch

Your ChangeLog should explain why.  Is this no longer flaky?  Why are these rebaselines differnet from other ports?

[Ami Fischman]

Created attachment 81382 [details]
Patch

[Ami Fischman]

Thanks for the quick review Eric.  I'm new here so I appreciate you making expectations/process explicit.

I expanded the ChangeLog entry in the updated patch.

I only changed the one port's baseline b/c that's what
./webkit/tools/layout_tests/run_webkit_tests.sh  --new-baseline --build-directory=`pwd`/ninja media/video-zoom.html
thought was necessary.  If that's not what you were getting at with "Why are these rebaselines differnet from other ports?", can you be more explicit about what you're looking for?

[Eric Seidel (no email)]

So these are all the scrollbar drawing bug?  (I'm pretty sure Mihai filed a Radar with Apple about such.)

[Ami Fischman]

> So these are all the scrollbar drawing bug?

image_diff says nothing changed other than the scrollbar, yes.
Are you saying this patch shouldn't be committed b/c the checked-in expectation is actually correct and the scrollbar showing up differently is a reported bug?

[Mihai Parparita]

(In reply to comment #7)
> > So these are all the scrollbar drawing bug?
> 
> image_diff says nothing changed other than the scrollbar, yes.
> Are you saying this patch shouldn't be committed b/c the checked-in expectation is actually correct and the scrollbar showing up differently is a reported bug?

This test has chromium-specific baselines since (apparently) we render scaled media elements differently from the mac port. When I landed my scrollbar change to make chromium-mac consistent with mac (http://trac.webkit.org/changeset/74892), I had to rebaseline such tests (since they had chromium-mac baselines checked in with the old chromium-specific scrollbar drawing). However, I missed this one since it was marked as failing at that time (for flakyness reasons which are apparently no longer true).

[Mihai Parparita]

(In reply to comment #7)
> Are you saying this patch shouldn't be committed b/c the checked-in expectation is actually correct and the scrollbar showing up differently is a reported bug?

Just to be clear, the scrollbar change is for the better, in that the chromium-mac scrollbars now match the mac ones. If you delete the video-zoom-expected.png/checksum files in chromium-mac and run this test with the mac pixel baselines, you'll see that the the only diffs are in the media element itself.

[Ami Fischman]

Thanks for the info Mihai.
I guess it's back to you, Eric, for review.

[Eric Seidel (no email)]

Comment on attachment 81382 [details]
Patch

rs=me.

[WebKit Commit Bot]

Comment on attachment 81382 [details]
Patch

Clearing flags on attachment: 81382

Committed r77897: <http://trac.webkit.org/changeset/77897>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

http://trac.webkit.org/changeset/77897 might have broken GTK Linux 32-bit Release

[Ami Fischman]

FTR: Build breakage was unrelated; GTK Linux 32-bit Release build was unbroken by http://build.webkit.org/changes/27435