[webkit-reviews] review denied: [Bug 58181] [chromium] Image layers don't update when their contents change : [Attachment 88911] Patch

[bugzilla-daemon at webkit.org]

James Robinson <jamesr at chromium.org> has denied Vangelis Kokkevis
<vangelis at chromium.org>'s request for review:
Bug 58181: [chromium] Image layers don't update when their contents change
https://bugs.webkit.org/show_bug.cgi?id=58181

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

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

Code change looks good, test not quite

- the test should go in LayoutTests/compositing/, there's nothing
chromium-specific about it
- since the test is set up to show red if it fails and green if it passes
there's no need for text in the actual output.	remove it and put it in an HTML
comment in the test in case anyone is unsure
- the setTimeout()s seem very wrong - can you get the same result by using
layoutTestController.display() to force composites?

> LayoutTests/platform/chromium/compositing/image-content-change.html:29
> +		   }, 20);
> +	       }
> +	   }, 2000);

these timeouts are suspicious.	where do 20 and 2000 come from?  why does this
test have to take 2 seconds to complete?