This is a serious security regression. If you are not able to fix this quickly, please revert the change that introduced it.

If this doesn't affect a release branch, or has not been properly classified for severity, please update the Security_Impact or Security_Severity labels, and remove the ReleaseBlock label. To disable this altogether, apply ReleaseBlock-NA.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Khushal, you have been making changes in this area, can you please help to triage.

Automatically applying components based on information from OWNERS files. If this seems incorrect, please apply the Test-Predator-Wrong-Components label.

Users experienced this crash on the following builds:

Win Canary 63.0.3232.0 -  3.84 CPM, 30 reports, 29 clients (signature blink::DecodingImageGenerator::GetContentIdForFrame)

If this update was incorrect, please add "Fracas-Wrong" label to prevent future updates.

- Go/Fracas

This crash has high impact on Chrome's stability.
Signature: blink::DecodingImageGenerator::GetContentIdForFrame.
Channel: canary. Platform: win.
Labeling  issue 768975  with ReleaseBlock-Dev.


If this update was incorrect, please add "Fracas-Wrong" label to prevent future updates.

- Go/Fracas

Not seeing any crashes after 63.0.3232.0. I'm still investigating it, but I don't think this needs to block dev.

There are 104 reports on 63.0.3232.0 but declined rapidly after that. The clusterfuzz test case is also not reproducible. I'm going to wait for a few more days, and if it stays the same, close this bug.

I see it spike again on 63.0.3234.0. Will follow up with a few CHECKs to narrow down the problem.

Friendly ping for an update on this. This is ranked as #3 renderer crash on the latest Windows canary version: 63.0.3238.0 and #2 renderer crash on the latest Dev(63.0.3236.0).

A patch to add a CHECK is here: https://chromium-review.googlesource.com/c/chromium/src/+/711030. That should give a trace for where an incorrect index was used.

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

commit ef1008fb7b69f58d19caa563efb4e037aeee83f2
Author: Khushal <khushalsagar@chromium.org>
Date: Sat Oct 14 08:03:58 2017

cc: Assert valid frame index on PaintImage and DrawImage.

Ensure that the frame index used with PaintImage is less than the
number of frames available.

Bug:  768975 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I32d550415a3708001230ce8be05e52f19e65d127
Reviewed-on: https://chromium-review.googlesource.com/711030
Reviewed-by: vmpstr <vmpstr@chromium.org>
Commit-Queue: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508933}
[modify] https://crrev.com/ef1008fb7b69f58d19caa563efb4e037aeee83f2/cc/paint/draw_image.cc
[modify] https://crrev.com/ef1008fb7b69f58d19caa563efb4e037aeee83f2/cc/paint/paint_image_builder.cc

Just to update the latest behavior of the below crash:

Magic Signature: 'blink::DecodingImageGenerator::GetContentIdForFrame'

This is top#4 renderer crash on Canary-63.0.3239.6 seeing 19 instances & top#2 renderer crash on Dev-63.0.3236.0 seeing 221 instances.

Thanks..!

The patch in #15 that added the assert hasn't made it to canary yet. I'm hoping it will help narrow down where the bug is.

M63 beta promotion is coming VERY soon. Your bug is labelled as Beta ReleaseBlock, pls make sure to land the fix and get it merged into the release branch ASAP. Thank you.

Adding Stability-Sheriff-Desktop, since this appears to be having a large impact on the stability of Windows Dev (63.0.3236.0).

It looks like r508933 is in today's 64.0.3241.0	Canary.  We're still seeing crashes in blink::DecodingImageGenerator::GetContentIdForFrame and I don't yet see any CHECK failures in DrawImage or PaintImage.

Can you post an update when you get a chance?  Thanks!

Thank you Vlad for the excellent insight!

Tthe issue is that in the case of animated images, blink only repaints elements which are onscreen. This means its possible to have a case where in the same recording 2 PaintImages have different frame indices. The frame index we select during playback gets locked in this map (https://cs.chromium.org/chromium/src/cc/tiles/tile_manager.cc?q=tile_manager.cc&dr&l=900) when we are scheduling raster work and we would keep the latest value we see from all PaintImages on a tile in this map.

So in the case where we have 2 PaintImages with the same id on a tile which is partially offscreen such that one instance of the image is onscreen and the other is not, we would keep the frame index from one of them. And if the other PaintImage had a generator with a lower frame count, we would give it an invalid frame index.

ClusterFuzz testcase 6722466612510720 is flaky and no longer crashes, so closing issue.

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

Re-opening as the investigation is still going on and crashes with magic signature 'blink::DecodingImageGenerator::GetContentIdForFrame' still seen.

Link to the builds:
===================
https://crash.corp.google.com/browse?q=custom_data.ChromeCrashProto.magic_signature_1.name%3D%27blink%3A%3ADecodingImageGenerator%3A%3AGetContentIdForFrame%27%20AND%20product.name%3D%27Chrome%27&sql_dialect=googlesql&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D  

Fix in review: https://chromium-review.googlesource.com/c/chromium/src/+/723765

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

commit 26e025feb74e49f320a1ca9cdafdfbd04827c723
Author: Khushal <khushalsagar@chromium.org>
Date: Tue Oct 17 22:25:07 2017

cc: Clamp the frame index used during playback in cc.

The PlaybackImageProvider uses the same frame index for all PaintImages
on a tile during playback. Since updating of offscreen animated images
is throttled in blink, this can lead to cases where an instance of the
PaintImage in the recording may not have the data to decode the
requested frame index. In such cases, clamp the frame index to last
index available in that PaintImage.

Also remove CHECKs added for debugging this issue.

R=vmpstr@chromium.org

Bug:  768975 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Iff9dd9b918aaf26731853d96c4762b2da864b58f
Reviewed-on: https://chromium-review.googlesource.com/723765
Commit-Queue: Khushal <khushalsagar@chromium.org>
Reviewed-by: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509569}
[modify] https://crrev.com/26e025feb74e49f320a1ca9cdafdfbd04827c723/cc/paint/draw_image.cc
[modify] https://crrev.com/26e025feb74e49f320a1ca9cdafdfbd04827c723/cc/paint/paint_image_builder.cc
[modify] https://crrev.com/26e025feb74e49f320a1ca9cdafdfbd04827c723/cc/raster/playback_image_provider.cc
[modify] https://crrev.com/26e025feb74e49f320a1ca9cdafdfbd04827c723/cc/raster/playback_image_provider_unittest.cc
[modify] https://crrev.com/26e025feb74e49f320a1ca9cdafdfbd04827c723/cc/test/skia_common.cc
[modify] https://crrev.com/26e025feb74e49f320a1ca9cdafdfbd04827c723/cc/test/skia_common.h

+awhalley@ (Security TPM)

I'll let some feedback come from canary before merging.

khushalsagar@, We will monitor the crash data from tonight's canary & udpate the issue once it is pushed out.

Thanks!  Glad to see the progress-- I'll remove this from the stability sheriff queue.

Not seeing any crash instances on Latest Canary build # 64.0.3243.0 on windows [live for almost 8 hours] & Mac OSX [live for 12 hours]

Magic Signature : 'blink::DecodingImageGenerator::GetContentIdForFrame'

Windows:
https://crash.corp.google.com/browse?q=custom_data.ChromeCrashProto.magic_signature_1.name%3D%27blink%3A%3ADecodingImageGenerator%3A%3AGetContentIdForFrame%27%20AND%20product.name%3D%27Chrome%27%20AND%20custom_data.ChromeCrashProto.ptype%3D%27renderer%27&sql_dialect=googlesql&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D#-samplereports,productversion:1000

Mac OSX:
https://crash.corp.google.com/browse?q=custom_data.ChromeCrashProto.magic_signature_1.name%3D%27blink%3A%3ADecodingImageGenerator%3A%3AGetContentIdForFrame%27%20AND%20product.name%3D%27Chrome_Mac%27%20AND%20custom_data.ChromeCrashProto.ptype%3D%27renderer%27&sql_dialect=googlesql&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D#-samplereports,productversion:1000

Thank you pucchakayala@.

khushalsagar@, pls request a merge to M63 if you think it is ready for merge. 

The quick fix I have above introduced a correctness issue on a test page. Fixing that and will merge that instead.

The real fix: https://chromium-review.googlesource.com/c/chromium/src/+/726742

Not seeing any crash instances on latest canary builds # 64.0.3243.0 & # 64.0.3244.0

khushalsagar@, could you please request the merge in to M63 branch & merge it by end of this week so that it gets picked correctly for the beta candidate next week. 

The change in #37 should land soon. I think it will be better to merge that instead.

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

Your change meets the bar and is auto-approved for M63. Please go ahead and merge the CL to branch 3239 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Please mark security bugs as fixed as soon as the fix lands, and before requesting merges. This update is based on the merge- labels applied to this issue. Please reopen if this update was incorrect.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Please merge your change to M63 branch 3239 before 11:30 AM Monday (10/23), so we can take it in for next M63 Dev/Beta release. Thank you.

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

Not seeing crashes on any latest M64 Builds post 64.0.3242.0 on Win & Mac OSX

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot