Hmm, I'm not able to reproduce. Maybe I'm not reproducing correctly? Is it the GIF on the first slide (attached)? Also I'm not sure what step (2) means. I think it's something like:
- click the down arrow next to "PRESENT"
- select "Presenter view"
- in the popup, click "SPEAKER NOTES
?

I'm using 66.0.3359.117 (Official Build) (64-bit). Oh wait - is this just on ToT and not in production? And I guess I need to enable compositor animation?

Are the two sharing the same ImageFrameGenerator? The generator has a cache of ImageDecoders, and if the cache has one, but it's locked, I think a new one will be created. But it still seems like both ImageDecoders should produce the same image. And they ought to be different SkImages, so the ImageFrameGenerator is writing to different blocks of memory.

Deleted: slide.gif 91.5 KB

	slide.gif 91.5 KB View Download

The feature is on by default, so you should be able to repro it directly on 66. The steps you mentioned sound about right. The important thing is to have the popup open and visible at the same time as the main page.

Yes, the 2 should be sharing the same ImageFrameGenerator. It comes from BitmapImage in blink which is shared across pages in the renderer. The issue is that disabling this caching of ImageDecoder fixes the issue. That's all the patch in #0 does, evicts the decoder after decoding a frame. That makes me think its some error with the state/caching of dependent frames in the ImageDecoder.

I have a theory that I'll look into later if/when I get time. Or you can look into it if you're already on this.

I suspect that if you mark each frame as immutable after it is decoded the problem will go away. (Do we need to also put a lock around decoding that frame? I think not but I forget. I seem to recall they are externally synchronized.)

When a frame is decoded but not displayed, it is marked as mutable. Only displayed frames are immutable. Mutable frames can have their buffers stolen for reuse.

It seems like the displayed frame is the wrong frame, as if the buffer was stolen and filled with another frame's content. Marking every decoded frame as immutable will prevent the frame's buffer from being stolen.

Maybe that external synchronization isn't happening in the new path?

Another data point that its coming from ImageDecoder, ensuring that we decode the complete dependency chain by removing the check for cached complete frames here: https://cs.chromium.org/chromium/src/third_party/blink/renderer/platform/image-decoders/image_decoder.cc?l=257, also fixes this.

Thanks for the suggestion Chris, I'll give it a shot. There is a mutex to ensure exclusive access to the decoder (https://cs.chromium.org/chromium/src/third_party/blink/renderer/platform/graphics/image_frame_generator.cc?type=cs&l=235), is that what you meant by synchronized?

Narrowed it down to the mutex getting released too early! Fix incoming.

Fix up for review: https://chromium-review.googlesource.com/c/chromium/src/+/1050619

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

commit d2b5331766438fcdc00e82366d6497be66aef9b4
Author: Khushal <khushalsagar@chromium.org>
Date: Wed May 09 01:53:45 2018

blink/images: Ensure thread-safe access to ImageDecoder frames.

ImageFrameGenerator ensures that accessing the cached ImageDecoders is
thread-safe using a mutex, locked for the duration of
ImageFrameGenerator::TryToResumeDecode. However, the bitmap returned by
this method holds a reference to the pixel buffer which can still be
mutated by the ImageDecoder. This can introduce a race between copying
the pixel buffer to the caller's memory and another thread writing to
it to decode a subsequent frame.

Ensure the above is avoided by holding the mutex for the duration of
the decode and copying the pixel buffer to the caller's memory if
necessary.

R=chrishtr@chromium.org
BUG= 840946 

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I110c54232385b5d335001f221a480371968f9551
Reviewed-on: https://chromium-review.googlesource.com/1050619
Commit-Queue: Khushal <khushalsagar@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557059}
[modify] https://crrev.com/d2b5331766438fcdc00e82366d6497be66aef9b4/third_party/blink/renderer/platform/graphics/image_frame_generator.cc

+govind. I'm going to verify the fix in canary tomorrow before requesting a merge to 67.

Before we approve merge to M67, please answer followings:

* Is this M67 regression? Is it critical?
* Is this change having enough automation tests coverage and safe to merge to M67?
* Any other important details to justify the merge.

Please note M67 is already in Beta, so merge bar is very high. Thank you.

* Is this M67 regression? Is it critical?
No the regression was present in M66, but it was introduced by a recent feature that causes the buggy code-path to be hit a lot more often. The regression leads to quite visible visual glitches if the same image is animating across multiple windows. The only way the user was able to get around it for M66 was by disabling the feature. In M67, that feature has been enabled by default and the old code path has been completely removed so there is no way to avoid the bug.

* Is this change having enough automation tests coverage and safe to merge to M67?
I'm adding a test right now, it wasn't super obvious for how to test this. But its a very small change and completely safe to merge.

* Any other important details to justify the merge.
I just tested this on canary and verified that it fixes the bug.

Ok, pls request a merge to M67. Also wanted to check, is this feature behind finch and can be easily disabled if anything goes wrong?

The feature (CompositorImageAnimation) has been on since M64. We only recently got a report about this bug. And since M67, its enabled by default with all client-side code removed.

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

commit b8736df3c3b9ba5fa92998809a2624b8e569f72f
Author: Khushal <khushalsagar@chromium.org>
Date: Wed May 09 19:50:39 2018

blink/images: Ensure thread-safe access to ImageDecoder frames.

ImageFrameGenerator ensures that accessing the cached ImageDecoders is
thread-safe using a mutex, locked for the duration of
ImageFrameGenerator::TryToResumeDecode. However, the bitmap returned by
this method holds a reference to the pixel buffer which can still be
mutated by the ImageDecoder. This can introduce a race between copying
the pixel buffer to the caller's memory and another thread writing to
it to decode a subsequent frame.

Ensure the above is avoided by holding the mutex for the duration of
the decode and copying the pixel buffer to the caller's memory if
necessary.

R=​chrishtr@chromium.org
BUG= 840946 

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I110c54232385b5d335001f221a480371968f9551
Reviewed-on: https://chromium-review.googlesource.com/1050619
Commit-Queue: Khushal <khushalsagar@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#557059}(cherry picked from commit d2b5331766438fcdc00e82366d6497be66aef9b4)
Reviewed-on: https://chromium-review.googlesource.com/1052967
Reviewed-by: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#537}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/b8736df3c3b9ba5fa92998809a2624b8e569f72f/third_party/blink/renderer/platform/graphics/image_frame_generator.cc

This bug requires manual review: M67 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

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

Cl listed at #15 was approved for merge via offline chat.

The NextAction date has arrived: 2018-05-10

Tested this issue on Windows 10, Mac 10.13.3 & Debian Rodate using chrome-67.0.3396.48 as per C#0 & C#1.

Steps:
1. launched Chrome
2. Login with corp credentials
3. Navigate to below URL
https://docs.google.com/presentation/d/1JLSgEesItdvx60RMrFWY6d-AJ5rw8KhZtjZ4C503hhA/edit#slide=id.g379045cd27_21_7432
4. Click on 'Down arrow' beside 'Present' On top right corner of the window
5. Select 'Presenter view' & then 'Presenter view' popup will get opened successfully
6. Click on 'Speaker notes' in the popup dialog box
7. Observe 'Gif' 

On Reported version->
    Observed flickering on gif while loading from the bottom right corner to top left corner in both (Actual window -Gif & popup window- Gif)

On Fixed version->
   
   No flickering Observed on gif while loading from the bottom right corner to top left corner in both (Actual window -Gif & popup window- Gif)

Please find the attached screencast for reference & confirm on the fix.
Thanks..!

Deleted: 840946-Fixed version.mp4 3.6 MB

	840946-Fixed version.mp4 3.6 MB View Download

Deleted: 840946-Reported-M66.mp4 5.4 MB

	840946-Reported-M66.mp4 5.4 MB View Download