Bug 173298 – Disable asynchronous image decoding for server push and streaming over HTTP contents

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: Images

[Said Abou-Hallawa]

For webcam pages, the MIME type is "multipart/x-mixed-replace". For this page, the server will push new data when it is available to the client. For the webcam case, new CachedImage and BitmapImage are created for the new image encoded data. If the BitmapImage is displayed asynchronously, there will a flickering between displaying the old and the new BitmapImage. Therefore, asynchronous image decoding has to be disabled in this case.

[Said Abou-Hallawa]

Created attachment 312741 [details]
Patch

[Said Abou-Hallawa]

<rdar://problem/32689659>

[Said Abou-Hallawa]

<rdar://problem/31246421>

[zalan]

Comment on attachment 312741 [details]
Patch

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

> Source/WebCore/ChangeLog:4
> +        https://bugs.webkit.org/show_bug.cgi?id=173298

radar link?

> Source/WebCore/rendering/RenderBoxModelObject.cpp:886
> +            auto decodingMode = decodingModeForPaintedImage(paintInfo);
>              context.drawTiledImage(*image, geometry.destRect(), toLayoutPoint(geometry.relativePhase()), geometry.tileSize(), geometry.spaceSize(), ImagePaintingOptions(compositeOp, bgLayer.blendMode(), decodingMode, ImageOrientationDescription(), interpolation));

You could just omit the local variable here.

> Source/WebCore/rendering/RenderElement.cpp:2208
> +    if (frame().loader().activeDocumentLoader()->isLoadingMultipartContent())
> +        return DecodingMode::Synchronous;

It seems a bit odd to change the decoding based on the minetype (though I understand the camera usecase) It also mismatches the discussion in the radar.

> Source/WebCore/rendering/RenderElement.h:223
> +    DecodingMode decodingModeForPaintedImage(const PaintInfo&) const;

Does it need to be public?

> Source/WebCore/rendering/RenderImage.cpp:588
> +    auto decodingMode = decodingModeForPaintedImage(paintInfo);
>      paintInfo.context().drawImage(*img, rect, ImagePaintingOptions(compositeOperator, BlendModeNormal, decodingMode, orientationDescription, interpolation));

You could just omit the local variable here.

[Simon Fraser (smfr)]

Comment on attachment 312741 [details]
Patch

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

Is there a way we can keep the older image around for longer in this case, rather than falling back to sync decoding?

> Source/WebCore/rendering/RenderElement.cpp:2207
> +    if (frame().loader().activeDocumentLoader()->isLoadingMultipartContent())

Is this right? You're assuming that the image is the main resource of that loader, but I don't think that's valid. Maybe this is an image in some kind of multipart text/html?

[Simon Fraser (smfr)]

Also, it might be a pain but can we make a http test and a CGI script that fakes the image stream? We get bugs on this feature almost every cycle, and it would be good to have some testing.

[Said Abou-Hallawa]

*** This bug has been marked as a duplicate of bug 174451 ***