When a new Image is passed to an ImageLayerChromium, the contents of the layer aren't properly invalidated and the new image doesn't register. The same problem exists if the image contains an animation (e.g. an animated gif).
Created attachment 88911 [details] Patch
Comment on attachment 88911 [details] Patch 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?
Created attachment 89017 [details] Patch
(In reply to comment #2) > (From update of attachment 88911 [details]) > 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 Good point. Moved it to LayoutTests/compositing/images > - 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 Done. > - 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? You're right, the intervals were kind of arbitrary. I changed them to 0. The timeouts are needed to make sure that the compositor gets triggered before and after the change to make sure we're testing the right thing. Using zero-interval timeouts seems to be common practice in the rest of the layout tests. Also, using timeouts rather than layoutTestController specific functionality has the advantage that the test can be run directly in the browser.
Comment on attachment 89017 [details] Patch R=me
Comment on attachment 89017 [details] Patch Clearing flags on attachment: 89017 Committed r83558: <http://trac.webkit.org/changeset/83558>
All reviewed patches have been landed. Closing bug.