I've attached a test case that will repro the issue. Switch on layer borders to offset things so you can get this exact case: 2 images on the same tile, one onscreen and one offscreen, and you'll see that the offscreen image wouldn't be repainted.

Even without them being on the same tile, this becomes an issue when the image animation is running in cc because a lot of code assumes that recordings in a commit have the same generator/the same amount of data so it should be safe to use the same frame index across all tiles on a tree. If this situation is happening during loading, then its possible the the updated PaintImage from the onscreen image has more data than the offscreen one. We'll try to use the same frame index for the offscreen image and that would cause an error with the generator.

I think this needs to be fixed in blink, we shouldn't delay painting if the ImageChanged notification resulted from the image data being updated. Right now the layout code doesn't have this distinction as to whether the notification is from the animation advancing or more data, but it should be easy to plumb from ImageResourceContent to ImageResourceObserver::ImageChanged. Yes, this does mean that we would invalidate offscreen instances of an image on data updates but that wouldn't be any different from what happens with static images.

Are there any alternate ideas?

Deleted: image_animation_bug.html 412 bytes

image_animation_bug.html 412 bytes View Download

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/58237fb768d3c6951396f6d162958526847f7924

commit 58237fb768d3c6951396f6d162958526847f7924
Author: Khushal <khushalsagar@chromium.org>
Date: Fri Oct 20 09:34:52 2017

cc: Only populate image to frame index map with cc image animations.

Currently we select the same frame index to use across all PaintImages
on a tile, which comes from blink on the PaintImage when image
animations are driven by blink. If this tile is partially offscreen
such that one image instance is offscreen, then blink never updates
the recording for it. So depending on the order in which we visit
images on the tile, we may select the frame index for the offscreen
instance of the image, which will effectively pause the animation also
for the visible instance of the image on this tile.

In order to avoid this, only populate the map when compositor image
animations are enabled. This can still lead to cases where if cc is
selecting the frame index to use, we attempt to use an index with an
instance of PaintImage that does not have data for that frame, which
will result in a crash in the generator when we query the FrameKey for
this frame. But the correct fix for this needs to be in blink, to
update the recording when the data for the image changes and ensure
all instances of PaintImage received in a commit share the same
generator.

R=vmpstr@chromium.org

Bug:  768975 ,  775558 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Iae302875166d667172018d0835370792bbd7a27c
Reviewed-on: https://chromium-review.googlesource.com/726742
Commit-Queue: Khushal <khushalsagar@chromium.org>
Reviewed-by: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510388}
[modify] https://crrev.com/58237fb768d3c6951396f6d162958526847f7924/cc/raster/playback_image_provider.cc
[modify] https://crrev.com/58237fb768d3c6951396f6d162958526847f7924/cc/raster/playback_image_provider_unittest.cc
[modify] https://crrev.com/58237fb768d3c6951396f6d162958526847f7924/cc/tiles/tile_manager.cc
[modify] https://crrev.com/58237fb768d3c6951396f6d162958526847f7924/cc/tiles/tile_manager_settings.h
[modify] https://crrev.com/58237fb768d3c6951396f6d162958526847f7924/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/58237fb768d3c6951396f6d162958526847f7924/cc/trees/layer_tree_settings.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/618173b110e090930ea879180fca035d5e5f5a85

commit 618173b110e090930ea879180fca035d5e5f5a85
Author: Khushal <khushalsagar@chromium.org>
Date: Mon Oct 23 05:14:46 2017

cc: Only populate image to frame index map with cc image animations.

Currently we select the same frame index to use across all PaintImages
on a tile, which comes from blink on the PaintImage when image
animations are driven by blink. If this tile is partially offscreen
such that one image instance is offscreen, then blink never updates
the recording for it. So depending on the order in which we visit
images on the tile, we may select the frame index for the offscreen
instance of the image, which will effectively pause the animation also
for the visible instance of the image on this tile.

In order to avoid this, only populate the map when compositor image
animations are enabled. This can still lead to cases where if cc is
selecting the frame index to use, we attempt to use an index with an
instance of PaintImage that does not have data for that frame, which
will result in a crash in the generator when we query the FrameKey for
this frame. But the correct fix for this needs to be in blink, to
update the recording when the data for the image changes and ensure
all instances of PaintImage received in a commit share the same
generator.

R=vmpstr@chromium.org
TBR=khushalsagar@chromium.org

(cherry picked from commit 58237fb768d3c6951396f6d162958526847f7924)

Bug:  768975 ,  775558 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Iae302875166d667172018d0835370792bbd7a27c
Reviewed-on: https://chromium-review.googlesource.com/726742
Commit-Queue: Khushal <khushalsagar@chromium.org>
Reviewed-by: vmpstr <vmpstr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#510388}
Reviewed-on: https://chromium-review.googlesource.com/732734
Reviewed-by: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#144}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/618173b110e090930ea879180fca035d5e5f5a85/cc/tiles/tile_manager.cc
[modify] https://crrev.com/618173b110e090930ea879180fca035d5e5f5a85/cc/tiles/tile_manager_settings.h
[modify] https://crrev.com/618173b110e090930ea879180fca035d5e5f5a85/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/618173b110e090930ea879180fca035d5e5f5a85/cc/trees/layer_tree_settings.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/206544e6b708b1cf2ae33ea218a16454e6457f4a

commit 206544e6b708b1cf2ae33ea218a16454e6457f4a
Author: Khushal <khushalsagar@chromium.org>
Date: Tue Oct 31 01:44:28 2017

blink: Don't defer image invalidations resulting from data update.

The optimization to defer invalidations for offscreen animated images
also defers the invalidation resulting from a data update. Since
updating the data for an image results in creating a new generator,
this can create the following scenario.

If multiple elements have the same image and only one of them is
onscreen, the cached DisplayList for only the onscreen element is
invalidated. If the offscreen elements are in the interest rect, the
recordings committed to the compositor will have PaintImages with
different generators, holding different lengths of encoded data.

The above scenario breaks the assumption in cc that all PaintImages
received in a commit should share the same generator and be able to
display the same frame. If one of the images does not have the
selected frame, because it is holding stale data from a cached
recording, this results in a crash when rasterizing this recording.

This change fixes that by plumbing whether the invalidation for an
ImageChanged notification can be deferred. This disallows defering
of the invalidation if it resulted from a data update to the Image.

Note that since an invalidation here also includes raster invalidation,
this will result in more prepaint invalidations during image load. But
this is already the case with static images, and avoiding this requires
a larger change in how image updates are pushed to the compositor.

R=chrishtr@chromium.org

Bug:  775558 
Change-Id: I9a6e166bf9f8db61758cac18040cca4612fc259a
Reviewed-on: https://chromium-review.googlesource.com/738622
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Commit-Queue: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512695}
[modify] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/core/css/CSSCrossfadeValue.cpp
[modify] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/core/css/CSSCrossfadeValue.h
[modify] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/core/css/CSSPaintValue.cpp
[modify] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/core/layout/LayoutBox.cpp
[modify] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/core/layout/LayoutBox.h
[modify] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/core/layout/LayoutBoxTest.cpp
[modify] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/core/layout/LayoutFrame.cpp
[modify] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/core/layout/LayoutFrame.h
[modify] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/core/layout/LayoutImage.cpp
[modify] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/core/layout/LayoutImage.h
[modify] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/core/layout/LayoutImageResource.cpp
[modify] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/core/layout/LayoutImageResource.h
[modify] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/core/layout/LayoutInline.cpp
[modify] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/core/layout/LayoutInline.h
[modify] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/core/layout/LayoutListMarker.cpp
[modify] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/core/layout/LayoutListMarker.h
[modify] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/core/layout/LayoutObject.cpp
[modify] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/core/layout/LayoutObject.h
[modify] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/core/layout/LayoutScrollbarPart.cpp
[modify] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/core/layout/LayoutScrollbarPart.h
[modify] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/core/layout/LayoutVideo.cpp
[modify] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/core/layout/LayoutVideo.h
[modify] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/core/layout/svg/LayoutSVGImage.cpp
[modify] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/core/layout/svg/LayoutSVGImage.h
[modify] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/core/loader/ImageLoader.cpp
[modify] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/core/loader/ImageLoader.h
[modify] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/core/loader/resource/ImageResourceContent.cpp
[modify] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/core/loader/resource/ImageResourceContent.h
[modify] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/core/loader/resource/ImageResourceObserver.h
[modify] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/core/loader/resource/ImageResourceTest.cpp
[modify] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/core/loader/resource/MockImageResourceObserver.cpp
[modify] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/core/loader/resource/MockImageResourceObserver.h
[modify] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/platform/BUILD.gn
[add] https://crrev.com/206544e6b708b1cf2ae33ea218a16454e6457f4a/third_party/WebKit/Source/platform/graphics/test/StubImage.h

My hope was that we could eventually remove the invalidation throttling code in blink, in the interest of avoiding unnecessary complexity if cc is taking care of it. But looks like all that code is shared with svg, which are still main thread driven, so that code is here to stay.

 Issue 779916  has been merged into this issue.

Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.

ClusterFuzz testcase 4593436106948608 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.