JDK-8202277 : WebView image capture fails with standalone FX due to dependency on javafx.swing
  • Type: Bug
  • Component: javafx
  • Sub-Component: web
  • Affected Version: 8,9,10,openjfx11
  • Priority: P2
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2018-04-25
  • Updated: 2020-01-31
  • Resolved: 2018-07-18
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 Other
8u202Fixed openjfx11Fixed
Related Reports
Relates :  
JDK-8198329 - Support FX build / test using JDK that doesn't include javafx.* modules
Relates :  
JDK-8209457 - [WebView] Canvas.toDataURL with image/jpeg MIME type fails
Description
The javafx.web module has a reflective dependency on the javafx.swing module. The UIClientImpl.toBufferedImage method calls a SwingFXUtils method in javafx.swing, via reflection, to convert an FX image into a BufferedImage. We need to eliminate this dependency.

To reproduce, run the attached program as follows using OpenJDK 10 + a build of the JavaFX standalone SDK.

Steps:

1. Set your JAVA_HOME and PATH to an OpenJDK 10 or 10.0.1 build. Verify this as follows:

$ java -version
openjdk version "10" 2018-03-20
OpenJDK Runtime Environment 18.3 (build 10+46)
OpenJDK 64-Bit Server VM 18.3 (build 10+46, mixed mode)


2. Apply the patch from JDK-8198329 to a local clone of jfx-dev

$ cd $JFX_ROOT/rt
<apply patch>


3. Build the standalone FX SDK:

$ cd $JFX_ROOT/rt
$ gradle sdk


4. Run the attached test program with the following options, which should be sufficient:

$ java --module-path="$JFX_ROOT/rt/build/sdk/lib" --add-modules=javafx.web CanvasTest

You will get the following exception:

java.lang.ClassNotFoundException: javafx.embed.swing.SwingFXUtils
	at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:582)
	at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:190)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:499)
	at java.base/java.lang.Class.forName0(Native Method)
	at java.base/java.lang.Class.forName(Class.java:291)
	at javafx.web/com.sun.javafx.webkit.UIClientImpl.toBufferedImage(UIClientImpl.java:401)
	at javafx.web/com.sun.javafx.webkit.prism.PrismImage.toData(PrismImage.java:68)
	at javafx.web/com.sun.javafx.webkit.prism.PrismImage.toDataURL(PrismImage.java:92)
	at javafx.web/com.sun.webkit.Timer.twkFireTimerEvent(Native Method)
	at javafx.web/com.sun.webkit.Timer.fireTimerEvent(Timer.java:83)
	at javafx.web/com.sun.webkit.Timer.notifyTick(Timer.java:64)
	at javafx.web/javafx.scene.web.WebEngine$PulseTimer.lambda$static$0(WebEngine.java:1196)
	at javafx.graphics/com.sun.javafx.application.PlatformImpl.lambda$runLater$10(PlatformImpl.java:424)
	at java.base/java.security.AccessController.doPrivileged(Native Method)
	at javafx.graphics/com.sun.javafx.application.PlatformImpl.lambda$runLater$11(PlatformImpl.java:423)
	at javafx.graphics/com.sun.glass.ui.InvokeLaterDispatcher$Future.run(InvokeLaterDispatcher.java:96)
	at javafx.graphics/com.sun.glass.ui.gtk.GtkApplication._runLoop(Native Method)
	at javafx.graphics/com.sun.glass.ui.gtk.GtkApplication.lambda$runLoop$11(GtkApplication.java:277)
	at java.base/java.lang.Thread.run(Thread.java:844)

Note that the exception prevents the image from being read and converted into a Data URL.


The fix will be to copy the SwingFXUtils.fromFXImage method into a private method in javafx.web and modify it to not use sun.awt.image.IntegerComponentRaster


Workaround:

The workaround is to add the swing module and also add one of the needed qualified exports as follows:

    --add-modules=javafx.swing --add-exports java.desktop/sun.awt.image=javafx.swing
Comments
This should be backported to 8.
19-07-2018
Changeset: 74d558eb71f6 Author: psadhukhan Date: 2018-07-18 10:36 +0530 URL: http://hg.openjdk.java.net/openjfx/jfx-dev/rt/rev/74d558eb71f6
18-07-2018
webrev .7 looks good to me too. +1
17-07-2018
"webrev.7" looks good to me. +1
17-07-2018
[~arajkumar] thanks for the primer on webunit tests intricacies :-) http://cr.openjdk.java.net/~psadhukhan/fx/8202277/webrev.7/
17-07-2018
[~psadhukhan] [~kcr], Following is my version of test which reliably passes all the time without any delay, public void testSwingDependency() throws Exception { final ByteArrayOutputStream errStream = new ByteArrayOutputStream(); System.setErr(new PrintStream(errStream)); loadContent(htmlContent); System.setErr(err); Assert.assertFalse("ClassNotFoundException found", errStream.toString().contains("ClassNotFoundException")); } (The key is, you have to use TestBase::loadContent, which takes care of waiting for the page to complete the loading.)
17-07-2018
http://cr.openjdk.java.net/~psadhukhan/fx/8202277/webrev.6/ with assertFalse
17-07-2018
Moving assert inside submit does not seem to work. It does not fail. public void testSwingDependency() throws Exception { ByteArrayOutputStream bytes = new ByteArrayOutputStream(); System.setErr(new PrintStream(bytes)); submit(() -> { final WebView webView = new WebView(); final WebEngine webEngine = webView.getEngine(); webEngine.loadContent(htmlContent); String s = bytes.toString(); Assert.assertFalse("ClassNotFoundException found", s.contains("ClassNotFoundException")); }); // Thread.sleep(200); System.setErr(err); }
17-07-2018
That seems like a bug in TestBase::submit then. I'll take a look, but if it doesn't wait, then it can't reliably propagate exceptions, either (so moving the assert inside the submit wouldn't work).
17-07-2018
[~kcr], I see that TestBase::submit is not waiting for the load to complete.
17-07-2018
>>I tried flushing but it did not help. It might be due to missing _synchronisation_ between test main thread and fx main thread. Probably moving the assertion into the previous submit(..) might help. >> Not sure if you are mandating me to change to Assert.assertFalse as Assert.fail is doing its job. It is for readability. We should avoid using conditional branches in unittest and use assertions instead., just a general unit test guidelines.
17-07-2018
I tried flushing but it did not help. Can I commit the changeset? Not sure if you are mandating me to change to Assert.assertFalse as Assert.fail is doing its job.
17-07-2018
+ Thread.sleep(200); + System.setErr(err); >> Delay was added as without that neither File during FileOutputStream not bytearray was written to and both were empty Probably flushing the `err` might help you get rid of the issue. But with `ByteArrayOutputStream` you shouldn't see the problem. + if (s.contains("ClassNotFoundException")) { + Assert.fail("ClassNotFoundException found"); + } You could express the above snippet like below. Assert.assertFalse("ClassNotFoundException found", s.contains("ClassNotFoundException"));
17-07-2018
Thanks for the review. Modified webrev to use ByteArrayOutputStream as is used in other webview tests. Delay was added as without that neither File during FileOutputStream not bytearray was written to and both were empty http://cr.openjdk.java.net/~psadhukhan/fx/8202277/webrev.5/
17-07-2018
+ } + DataBufferInt db = (DataBufferInt)bimg.getRaster().getDataBuffer(); Fix the indentation. SwingDependencyTest.java: + Thread.sleep(200); What is the need of this delay? + FileOutputStream fout = new FileOutputStream(file); + System.setErr(new PrintStream(file)); Use ByteArrayOutputStream instead of FileOutputStream. Refer CallbakTest.java.(testBlockingPopupHandler)
17-07-2018
In that case, I can't think of a reason that the sleep would be needed, unless the call to toDataURL is done from a runLater.
17-07-2018
[~kcr] Actually HTML5 Canvas `toDataURL` method will be called from JavaScript main thread(which is FX main thread), and from WebKit platform ImageBuffer::toDataURL[1] we are already blocking the main thread to complete the ImageBuffer rendering before converting pixels to URL data. [1] http://hg.openjdk.java.net/openjfx/jfx-dev/rt/file/11877faf1ae6/modules/javafx.web/src/main/native/Source/WebCore/platform/graphics/java/ImageBufferJava.cpp#l459
17-07-2018
The sleep may be needed if there is rendering activity that will happen after the submit returns. Note that submit won't return until it has successfully loaded the web page. Moving the assert into the submit will, if anything, cause it to be done earlier, and in any event, won't help. This is a problem we have with tests that need to have something happen on (or triggered by) the rendering thread. I recommend keeping the sleep. Arun's suggestion about assertFalse is a good one.
17-07-2018
+1
16-07-2018
Modified webrev with this changes in http://cr.openjdk.java.net/~psadhukhan/fx/8202277/webrev.4/
16-07-2018
One more minor comment: The import of java.lang.reflect.Method in UIClientImpl.java is no longer used. You might want to remove the unused import (it's up to you).
16-07-2018
Looks good with one minor comment: Given that you initialize the 'err' variable during construction of the test class object, you don't need the null check that I mentioned earlier. In this case, you can declare the 'err' field as "final" and skip the 'if (err != null)' check (you still need the null check for 'file' in case the file cannot be constructed for some reason). Also, please get a second review from either Arun or Murali. +1 -- if you choose to fix the "err" variable to be final (and thus avoid the null check) no need for a new webrev.
16-07-2018
I'll review it shortly. To answer your question: > Just trying to understand, why do you say > System.setErr(System.err) is a no-op -- it will set it back to its current value The call to System.setErr(stream) sets System.err to the 'stream'. This may seem counter-intuitive since System.err is declared as final, but it really isn't final in the sense of being immutable. You can see this for yourself with a simple test program: System.out.println(System.err); System.setErr(myPrintStream); System.out.println(System.err);
16-07-2018
Please find modified webrev http://cr.openjdk.java.net/~psadhukhan/fx/8202277/webrev.3/ Just trying to understand, why do you say System.setErr(System.err) is a no-op -- it will set it back to its current value I see in System.java public static void setErr(PrintStream err) { checkIO(); setErr0(err); } it does not check for err equal to System.setErr or not. Irrespective, it calls setErr0 to set the stream.
16-07-2018
* System.setErr(System.err) is a no-op -- it will set it back to its current value. What you need to do is save it in an instance variable and then restore it from that variable. * You should check that 'file' is not-null before deleting it. similarly when you add the save / restore of the err printsteam, you should check that for non-null before restoring it * Typo in your assert statement: ClassNotFoundExcpeption should be ClassNotFoundException * The test file has trailing whitespace * Minor: the 'break' after the Assert.fail is unnecessary, since that statement will never be reached.
13-07-2018
webrev addressing the review comments http://cr.openjdk.java.net/~psadhukhan/fx/8202277/webrev.2/
13-07-2018
7. If there is at least one line, the following is an infinite loop, since you never change that variable in the loop: 94 while (hasNextLine) {
13-07-2018
Comments on test: 1. Spelling error in name of test class and test: 'dependancy' should be 'dependency' 2. In three places you use a try/catch and ignore exception. Rather than that, please add "throws Exception" to the test method and remove the try/catch, so that any exception will cause the test to fail. 3. Line 74: err is an unused variable. 4. You should save / restore the System.err print stream after closing 'fout'. Otherwise this could affect the verbose output of test failures, as well as subsequent tests. You will also want to restore it in an '@After' method in case there is an exception that prevents the stream from being reset. 5. Line 96: you are missing a space after the 'if' 6. You can remove the print to system.out -- the failure from the assert.fail is sufficient.
13-07-2018
Please review this modified webrev with unit test http://cr.openjdk.java.net/~psadhukhan/fx/8202277/webrev.1/
13-07-2018
Hard to say. You have an extra '/' in your JAVA_HOME and JDK_HOME, but I doubt that would cause gradle clean to fail. It looks more like a problem where you are using a cygwin version of JDK_HOME or JAVA_HOME instead of a Windows version, but your "echo" commands look OK.
05-07-2018
I am facing some build issue with openjdk-10. Are there anything more setup that needs to be done for openjdk? $ which java /cygdrive/c/openjdk-10/bin/java PRSADHUK-IN+prsadhuk@PRSADHUK-IN /cygdrive/d/openjfx10/jfx-dev/rt $ java -version openjdk version "10" 2018-03-20 OpenJDK Runtime Environment 18.3 (build 10+46) OpenJDK 64-Bit Server VM 18.3 (build 10+46, mixed mode) PRSADHUK-IN+prsadhuk@PRSADHUK-IN /cygdrive/d/openjfx10/jfx-dev/rt $ echo $JDK_HOME C://openjdk-10 PRSADHUK-IN+prsadhuk@PRSADHUK-IN /cygdrive/d/openjfx10/jfx-dev/rt $ echo $JAVA_HOME C://openjdk-10 $ $JAVA_HOME/bin/java -version openjdk version "10" 2018-03-20 OpenJDK Runtime Environment 18.3 (build 10+46) OpenJDK 64-Bit Server VM 18.3 (build 10+46, mixed mode) PRSADHUK-IN+prsadhuk@PRSADHUK-IN /cygdrive/d/openjfx10/jfx-dev/rt $ gradle clean > Configure project :buildSrc Defining Closed Properties > Task :buildSrc:compileJava NO-SOURCE > Task :buildSrc:compileGroovy UP-TO-DATE > Task :buildSrc:processResources NO-SOURCE > Task :buildSrc:classes UP-TO-DATE > Task :buildSrc:jar UP-TO-DATE > Task :buildSrc:assemble UP-TO-DATE > Task :buildSrc:compileTestJava NO-SOURCE > Task :buildSrc:compileTestGroovy NO-SOURCE > Task :buildSrc:processTestResources NO-SOURCE > Task :buildSrc:testClasses UP-TO-DATE > Task :buildSrc:test NO-SOURCE > Task :buildSrc:check UP-TO-DATE > Task :buildSrc:build UP-TO-DATE > Configure project : Closed build Defining Closed Properties FAILURE: Build failed with an exception. * Where: Build file 'D:\openjfx10\jfx-dev\rt\build.gradle' line: 584 * What went wrong: A problem occurred evaluating root project 'rt'. > Missing or incorrect path to 'java': '\cygdrive\c\openjdk-10\bin\java.exe'. Perhaps bad JDK_HOME? /cygdrive/c/openjdk-10
05-07-2018
I am not able to reproduce this issue now. I have $ java -version java version "10" 2018-03-20 Java(TM) SE Runtime Environment 18.3 (build 10+46) Java HotSpot(TM) 64-Bit Server VM 18.3 (build 10+46, mixed mode) $ java --module-path="D:/openjfx10/jfx-dev/rt/build/sdk/lib" --add-modules=javafx.web CanvasTest Loaded I believe JDK-8198329 patch is already in so I do not need to do anything. I have backed out the patch I have from UICLientImpl.java I submitted earlier, but I have the qualified exports in the build.gradle and JDK-8195811 patch
05-07-2018
>I am not able to reproduce this issue now. I have >$ java -version >java version "10" 2018-03-20 ... This is because you are running Oracle JDK 10 and not OpenJDK 10.
05-07-2018
There is white space issue with webrev.
25-06-2018
I guess we already have a unit test for this case javafx.web/src/test/java/test/javafx/scene/web/CanvasTest.java
16-06-2018
The fix looks fine to me. Can you please provide a unit test? It should be fairly easy to convert the attached test program to a unit test. If it can reproduce the bug without needing to show a stage, then it can go in modules/javafx.web, otherwise tests/system.
15-06-2018
Request to review http://cr.openjdk.java.net/~psadhukhan/fx/8202277/webrev/
15-06-2018