[webkit-reviews] review denied: [Bug 66472] [chromium] Add backend compositor support for rescaling (zooming) textures during zoom animation. : [Attachment 105804] Patch

[bugzilla-daemon at webkit.org]

James Robinson <jamesr at chromium.org> has denied W. James MacLean
<wjmaclean at chromium.org>'s request for review:
Bug 66472: [chromium] Add backend compositor support for rescaling (zooming)
textures during zoom animation.
https://bugs.webkit.org/show_bug.cgi?id=66472

Attachment 105804: Patch
https://bugs.webkit.org/attachment.cgi?id=105804&action=review

------- Additional Comments from James Robinson <jamesr at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=105804&action=review


Thanks for the test.  The code looks pretty sane (although you've got a lot of
merge conflicts now. pardon our dust!)

I'm not really sure what is going on with the test, though.  What would the
test display if the test failed?  Normally we try to construct tests to that
you see red if it fails and green if it passes, if that's feasible, and
otherwise we try to describe the output you should see in the test (in an HTML
comment or the like).  I can't tell if that test is passing or failing just
from looking at it.  It would also be good to have tests for >1 zooms as well
as <1.	I'm also a little confused about whether the non-initialized compositor
blue is showing up in the test or if that's part of the test page.  We probably
shouldn't be showing the uninitialized blue ever, right?

> LayoutTests/platform/chromium/compositing/zoom-animator-scale-test.html:9
> +	  if (window.layoutTestController)
> +	    window.layoutTestController.waitUntilDone();

please also call layoutTestController.dumpAsText(true) to indicate that the
pixel output is the only thing needed