+enne, vmiura FYI.

For this one it looks like the page is doing a src change. I could get some cases on android as well, when the high resolution image was large enough to go through the async decode path.

I can look into plumbing an element id inside PaintImage to disable checkering the same element again, which would address this case. But do we feel that this is the correct direction to take?

I'm a little hesitant about making a distinction where an element which a src change is treated differently from a new element. This would certainly address this issue, but doesn't address issues on things like image search. 

The reason for the hesitation is that I'm not keen on recommending that people use the same element but just change the src as a workaround.

Maybe that's the right heuristic though...

And would be another way where we diverge in behaviour from Firefox.

In this particular case, I don't know that matching bad behavior seems like something we should feel compelled to do.

Do we have any options to handle the case where the image element is replaced entirely?

> In this particular case, I don't know that matching bad behavior seems like something we should feel compelled to do.

I guess it would be interesting to see if there are cases where src changes and checker imaging would benefit it. I can see cases where you append blank images and then change the src onload or something like that. Since checker images is an smoothness optimization at the cost of showing blank instead of images, it's really up to us when to enable this.

With the current heuristics, I think Khushal found that we're considering about 10% of images as allowed to be checkered.

> Do we have any options to handle the case where the image element is replaced entirely?

We've hooked up the img decode api to veto checker imaging, if that's what you mean. That means if the author calls decode on the image, then that image is guaranteed not to checker. When the api lands, then that would be one way to work around the checker imaging.

So we had chatted about trying to detect cases where the page would be relying on the image to be displayed synchronously with the rest of the changes and the narrowest signal we could come up with was an on-load handler. If an image is appended without it, it would create cases where we could render blank anyway if wasn't loaded.

One thought was to not checker on images with an on-load handler. I tested it and this does address both the amazon and image search flickers, but might be too heavy a hammer. Lets UMA how many cases we have to veto for this reason and go from there?

It looks like there was some offline discussion that might have covered this but I missed it. Can you describe the sequence of events that's leading to the flickering on Amazon?


Just noting a quirk of loading: The document load event waits for all images to load before firing. It may be worth also measuring if pages are listening to the document load instead of individual image loads.

On google image search and amazon gallery scrolling, the page first fetches a low resolution placeholder for an image, and eventually replaces it with a high res version. In the compositor, if we decide to defer an image for async decoding, we render blank for an image until the decode finishes. So the transition goes: low-res-placeholder -> blank -> high res image, instead of low-res-placeholder -> high res image in the regular case where the frame is blocked on the image decode.

The transition to blank during the decode causes a flicker. These cases only happen when the page is trying to transition the same image with different versions. Using a document load listener would probably not be used for this.

I'm a worried that a heuristic like this will appear as inconsistent to developers. It is surprising that event handlers would affect flickering. In the future we could have stackoverflow posts recommending adding event handlers to reduce flicker in chrome, or removing event handlers for more speed.

It's still a little weird to have flicker differences when changing src, but not having async decoding when changing from one image to another is likely what the user wants.

We need to provide some escape hatch to developers to disable this behaviour in cases like these. I understand that an onload handler might not be the most intuitive way for it, the only reason for considering it was that it is most likely to compatible with existing pages. 

Async decoding was always meant to be an intervention which we expected would result in issues and for cases where an image should be displayed synchronously once it is added to the page, the plan was to encourage the use of the decode api instead of onload. It achieves the same objective of ensuring that the image is as close to ready as possible when we get to raster it but provides the control to the develolper. The way it has been set up is that you could still call decode on the image and add it before the promise resolves, in which case we will display it synchronously, but it would be a reliable workaround.

So the question is, is the decode api a reasonable alternative to provide to developers to avoid this behaviour? And if we provide a reasonable time-frame for developers to adopt it once its shipped, are we comfortable with async decoding resulting in such flickers on pages that don't update?

We've had a long discussion with pdr@ and chrishtr@. I believe that there is a general opposition to this feature altogether, because of

1. flicker on image search and amazon.
2. flicker on image reload.

2 can be fixed by detecting a reload instead of navigation and not checkering any content on that. 1 is harder to address, because it's affecting sites that display a low resolution content which is then replaced with a high resolution content. Amazon seemingly does this using a src change, whereas img search does this by replacing the element. This means that detecting a src change is not enough. This means it is hard if not impossible for us to detect this specific type of pattern.

onload addresses 2 for both sites, but #10 raised a point that it is inconsistent. Personally, I feel that it is a targeted fix for problems that we know of. I don't really like thinking in terms of "there will be 1000s of cases that break" unless I see cases that break. 

Another solution is to disable checker imaging for any images that have resulted from javascript code. This is a bigger hammer that addresses onload and probably a bunch of other cases. I think that's fine, since the target sites this feature is trying to address are a long tail of poorly written sites that have large images but little or no script. 

I think the next steps here are to investigate the performance benefits and resolve outstanding issues that prevent good performance (ie scheduling throttling), and implement a fix that addresses 1 and 2 above.

To re-iterate, I think this feature should still go ahead after the bugs are addressed.

Thanks Vlad, I think that is a fair summary and I think your proposed next steps are good ones.

In looking into this, I found WebKit is also landing async image decoding (overall tracking bug at https://bugs.webkit.org/show_bug.cgi?id=155322). Support for this in second major browser leads me to believe we should continue investigating this. Using similar heuristics would also be nice on web developers.

WebKit's async decode feature seems to have caused a similar flash regression (https://bugs.webkit.org/show_bug.cgi?id=174451). I made a simple repro case (https://pr.gg/flicker.html) and found that Safari 10.1.1 does not flicker but Safari 11.0 preview does flicker. The proposed WebKit fix seems to only do async decoding on the first draw of a tile. Could a similar approach work in blink?

Thanks for putting together the repro case! I've also confirmed that this flickers on firefox as expected. 

I think the most important task here is for us to understand the performance of this change. Depending on the outcome, the discussion is slightly different.

If there are clear benefits of the change, then we'll go ahead with fixing the flickering where we can (probably similar to WebKit's fix to do it only on first draw). 

If we discover some underlying issue preventing good benefits, then we should see if those are fixable. If not, then we can reconsider this change altogether.

The Webkit workaround seems to be using repaint counts on GraphicsLayers. We had considered something similar of veto-ing rects on a layer where images have been displayed but given that the layerization can also potentially change, that won't be consistent.

I think my preferred fix would be to make it so that changing the image src of a visible image prevents checkerboarding for that frame, but that putting in a new image doesn't. That fixes the amazon case and what I suspect is the common case of what developers would expect in terms of flicker. It doesn't fix the google image search case, but we can lobby them to fix that on their end with the decode API.

The WebKit solution seems unpredictable to me. I don't know how we'd explain that to web developers. It also seems like we would do sync raster of images more often than in the solution I propose here.

Regarding onload events, I would shy away from using that as a signal for a couple reasons:
1. It will cover way more cases than we need to avoid flickering. So will lead to worse performance in some cases.
2. There's a general opposition in web platform design to having registering an event handler have side effects because of fear of confusing heisenbugs where observing the behavior changes it.

Could we do this just on first page load or first paint? It's a conservative approach but we can still get good data.

When I recommended disabling async decodes on src changes, I didn't realize that it would still leave pages like images.google.com broken. I think it is likely other pages are using similar patterns and it would be best to not knowingly add flashes without proof of the upside.

To sum up the discussion with pdr@ and chrishtr@ on this today. The conclusion was to limit checkering an image element only if it was in the DOM (or may be the Layout tree?) and at least one frame was committed while no data was available for this image. If we could have already checkerboarded this image since it was unavailable, then it is fair to also delay it until it is decoded.

The aim is to avoid checkering for cases where the page is ensuring that the image is loaded before appending it or if its already in the in memory cache (for instance in the case of a reload).

Reviving this bug now that we have more consensus on the launch plan for this. I want to go ahead with adding a src change veto to the default heuristic. pdr@/chrishtr@, sound good?

Sounds good to me!

Per other threads, yes it sounds good to add the src change veto to the heuristic for the
next phase of experimentation.

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

commit 512439cb84fd40519064a16aafab13d89963cca1
Author: Khushal <khushalsagar@chromium.org>
Date: Wed Oct 18 17:36:53 2017

images: Plumb async decode veto for src changes to cc.

If the image content for an image element is changed, displaying the
updated image asynchronously can result in visual content flickering.
Plumb this state change using the DecodingMode on PaintImage to cc.

R=chrishtr@chromium.org, vmpstr@chromium.org

Bug:  740953 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I1d89eb170e26f7348a247cd93d85f62efef6d40d
Reviewed-on: https://chromium-review.googlesource.com/714477
Commit-Queue: Khushal <khushalsagar@chromium.org>
Reviewed-by: Fredrik Söderquist <fs@opera.com>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509802}
[modify] https://crrev.com/512439cb84fd40519064a16aafab13d89963cca1/cc/paint/paint_image.cc
[modify] https://crrev.com/512439cb84fd40519064a16aafab13d89963cca1/cc/paint/paint_image.h
[modify] https://crrev.com/512439cb84fd40519064a16aafab13d89963cca1/third_party/WebKit/Source/core/html/HTMLImageElement.cpp
[modify] https://crrev.com/512439cb84fd40519064a16aafab13d89963cca1/third_party/WebKit/Source/core/html/HTMLImageElement.h
[modify] https://crrev.com/512439cb84fd40519064a16aafab13d89963cca1/third_party/WebKit/Source/core/html/canvas/ImageElementBase.cpp
[modify] https://crrev.com/512439cb84fd40519064a16aafab13d89963cca1/third_party/WebKit/Source/core/html/canvas/ImageElementBase.h
[modify] https://crrev.com/512439cb84fd40519064a16aafab13d89963cca1/third_party/WebKit/Source/core/paint/ImagePainter.cpp
[modify] https://crrev.com/512439cb84fd40519064a16aafab13d89963cca1/third_party/WebKit/Source/core/paint/SVGImagePainter.cpp
[modify] https://crrev.com/512439cb84fd40519064a16aafab13d89963cca1/third_party/WebKit/Source/core/paint/compositing/CompositedLayerMapping.cpp
[modify] https://crrev.com/512439cb84fd40519064a16aafab13d89963cca1/third_party/WebKit/Source/core/svg/SVGImageElement.cpp
[modify] https://crrev.com/512439cb84fd40519064a16aafab13d89963cca1/third_party/WebKit/Source/core/svg/SVGImageElement.h
[modify] https://crrev.com/512439cb84fd40519064a16aafab13d89963cca1/third_party/WebKit/Source/platform/graphics/Image.h

Fixed on latest canary.

Tested this issue on Mac 10.12.6 using chrome latest #64.0.3247.0 as per the URL mentioned in C#0.
No image flickering observed in the amazon website while scrolling down the page.

Same behavior observed on reported version-61.0.3118.0 also.
Please find the attached screencast for reference & please confirm on the fix and the expected behavior.

Thanks in advance..!

Deleted: 740953-latest.mp4 6.6 MB

	740953-latest.mp4 6.6 MB View Download

The expected behaviour is to not see the flicker. But note that the feature that introduced the flicker is behind a flag and may not be enabled for you. In order to ensure that it is enabled for testing, go to chrome://flags, search for AsyncImageDecoding and select Enabled. This will ask you to restart the browser. Also, pause a bit after scrolling to make sure that the images in the gallery become change (become crisper), without any flicker.