Bug 186336 – TileFirstPaint strategy for async image decoding should be disabled for non root RenderLayers

• Status: RESOLVED
• Resolution: FIXED
• Component: Images

[Chris Dumez]

HTML 'load' event fires before images have rendered. This causes flickr for some apps / sites. I will attach a demo.

[Chris Dumez]

<rdar://problem/40808099>

[Chris Dumez]

Created attachment 342025 [details]
Repro case

Here is a very simple repro case.

Open test.html in Safari, then hit the reload button.

Expected result: There should be no red flash
Actual result: We see a red flash before the image shows

Works in Chrome.

The page has a red div with opacity 0 and a background image in CSS. When the 'load' event fires, opacity of the is changed to 1.

Was reported at WWDC today.

[Chris Dumez]

I confirmed it is a regression from async image decoding by updating RenderBoxModelObject::decodingModeForImageDraw(const Image&, const PaintInfo&) to always return DecodingMode::Synchronous. This made the bug go away.

I seem to remember there is a way for users to opt out of async image decoding for <img> elements. However, I do not think there is such thing for images loaded from CSS?

[Chris Dumez]

(paintInfo.paintBehavior & PaintBehaviorTileFirstPaint) is true in RenderBoxModelObject::decodingModeForImageDraw(const Image&, const PaintInfo&).

[Said Abou-Hallawa]

What is the spec of the load event regarding painting the image? Does firing the load event guarantees the image will be painted with the next window update? If this is the case we can disable the async image decoding for large images if the image element has a load event handler. The reason will be: in the image load event handler there might be a script which assumes the image will be painted synchronously.

Meanwhile there is an easy workaround which uses the HTMLImageElement::decode API. When the decode promise is resolved, the image has to be be painted in the next window update. The workaround test case is attached.

[Said Abou-Hallawa]

Created attachment 342178 [details]
image

[Said Abou-Hallawa]

Created attachment 342179 [details]
workaround

[Said Abou-Hallawa]

Created attachment 342590 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 342590 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=342590&action=review

> Source/WebCore/ChangeLog:14
> +        had to be generalized for large and animated images. The logic for making 
> +        the image decoding takes the required duration was moved from BitmapImage

"the logic for making the image decoding" doesn't make sense grammatically.

> Source/WebCore/rendering/RenderLayer.h:189
> +    bool isRoot() const { return !parent(); }

We already have isRenderViewLayer(), which should give the same answer.

[Said Abou-Hallawa]

Created attachment 342614 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 342614 [details]
Patch

Clearing flags on attachment: 342614

Committed r232802: <https://trac.webkit.org/changeset/232802>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.