JDK-8153148 : Defer image decoding until WebCore requests ImageFrame
  • Type: Bug
  • Component: javafx
  • Sub-Component: web
  • Affected Version: 9
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2016-03-31
  • Updated: 2020-01-31
  • Resolved: 2016-04-13
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 8 JDK 9
8u102Fixed 9Fixed
Related Reports
Duplicate :  
JDK-8163252 - JavaFx WebView uses too much heap for web pages with lots of images
Relates :  
JDK-8089049 - [WebView] zooming google maps hangs
Description
Currently we are decoding the images as soon as it is downloaded. Though it helps rendering, but it hurts memory badly. Also causes OOM with pages which has quite a few images.

Load the testcase http://cr.openjdk.java.net/~arajkumar/img_test/00/ in HelloWebView to produce the OOM.
Comments
Changeset: 3f15f2e59063 Author: arajkumar Date: 2016-04-13 12:36 -0700 URL: http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/3f15f2e59063
13-04-2016
+1 on the .03 version I will push the fix later today.
13-04-2016
>> 1) Do you think existing unit tests are sufficient to test this? I will be hard to test since it is not a behavioral change with respect to rendering. I couldn't find unit tests related to image rendering. >> 2) In WCImageDecoderImpl::imageSizeAvilable, I have a suggestion: Addressed in http://cr.openjdk.java.net/~arajkumar/8153148/webrev.03 Thank you @Kevin.
13-04-2016
The changes look fine. One comment and one question: 1) Do you think existing unit tests are sufficient to test this? 2) In WCImageDecoderImpl::imageSizeAvilable, I have a suggestion: > return imageHeight != 0 && imageWidth != 0; It might be more robust to check for > 0 rather than != 0 although if you can ensure that the values can never be set to a negative number then it doesn't really matter. +1
12-04-2016
http://cr.openjdk.java.net/~arajkumar/8153148/webrev.02 Updated the implementation to support both lazy decoding and progressive rendering. Basically the new implementation matches the previous behavior in terms of rendering, but decoding happens only if frames are requested from WebCore. Progressive rendering is achieved using the existing javafx Service concurrency primitive, but will be started only if WebCore requests frame.
07-04-2016
http://cr.openjdk.java.net/~arajkumar/8153148/webrev.01 Full Image Decoding should happen only after receiving complete image data from WebCore. Prior to webrev.01, decoding happens as soon as getImageFrame(..) is called & frames == null. But sometimes getImageFrame(..) might be called with partial data also.
07-04-2016
>> Before I review the fix itself (which I will do tomorrow) I did some testing and discovered a regression in that images are not always fully loaded. I see that the code that used to reload the image was eliminated. Maybe related to that? I will check that. >> Also, this patch now causes images to be loaded synchronously on the FX Application thread rather than on a background thread. Maybe that is a necessary consequence of waiting to start the load until you need it, but it could hurt performance. + } else if (data != null) { // null dataPortion means data completion - destroyLoader(); if (data.length > dataSize) { resizeDataArray(dataSize); } - setFrames(loadFrames()); // full data received - decode frames immediately - framesDecoded = true; - } Earlier also we were doing sync decoding after receiving full image data by discording frames decoded by Decoding Worker. Thanks @Kevin
07-04-2016
Before I review the fix itself (which I will do tomorrow) I did some testing and discovered a regression in that images are not always fully loaded. I see that the code that used to reload the image was eliminated. Maybe related to that? Also, this patch now causes images to be loaded synchronously on the FX Application thread rather than on a background thread. Maybe that is a necessary consequence of waiting to start the load until you need it, but it could hurt performance.
07-04-2016
http://cr.openjdk.java.net/~arajkumar/8153148/webrev.00/
01-04-2016