On Tue, 15 Dec 2020 16:57:13 GMT, Matthias Bläsing 
<github.com+2179736+matthiasblaes...@openjdk.org> wrote:

>> The code in WTF::scheduleDispatchFunctionsOnMainThread assumes, that
>> the java class com.sun.webkit.MainThread can be found be the JNI
>> function FindClass. This is only true if the class is loadable by the
>> system class loader.
>> 
>> One such case is when the OpenJFX modules are loaded from a new
>> ModuleLayer. To fix this, the reference to the class needs to be loaded
>> from when a JNI call from Java into native code is active. In that case
>> FindClass uses the classloader associated with that method.
>> 
>> The test code can be executed by running:
>> 
>> cd tests/manual/web/dataurl
>> ../../../../gradlew run
>
> Matthias Bläsing has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Updated according to review comments
>   
>   - The text and formatting was adjusted according to the raised concerns.
>   - The test was modified with a 15000ms timeout as is the case for other
>     tests in the systemTests project
>   - A safeguard was added in the launched child JVM, that terminates the
>     child after 15000ms is it did not exit normally
>   - The detection of a successful test was moved to a ChangeLister on the
>     load worker state. It is assumed, that the worker success state is
>     similar to the DOMContentLoaded event. The latter is fired after
>     synchronous javascript was executed and that would be late enough to
>     "see" the DOM update of the title.
>   
>   It was validated, that the test still fails without the fix and succeeds
>   with it.

The updates look good. Two additional comments on the test.

tests/system/src/testapp7/java/mymod/myapp7/DataUrlWithModuleLayer.java line 42:

> 40: public class DataUrlWithModuleLayer extends Application {
> 41:     public static final int ERROR_OK = 0;
> 42:     public static final int ERROR_ASSUMPTION_VIOLATED = 1;

We usually leave 1 unused, since that is what `Process.waitFor` returns if the 
application fails to launch (in case that happens it will be easier for someone 
to diagnose).

tests/system/src/testapp7/java/mymod/myapp7/DataUrlWithModuleLayer.java line 97:

> 95:                 new ChangeListener<State>() {
> 96:                     public void changed(ObservableValue ov, State 
> oldState, State newState) {
> 97:                         String title = webview.getEngine().getTitle();

Minor: you might want to move this inside the test for `SUCCEEDED` since it 
isn't valid until then.

-------------

PR: https://git.openjdk.java.net/jfx/pull/360

Reply via email to