RFR: 8313177: Web Workers timeout with Webkit 616.1 [v2]

[Kevin Rushforth]

On Mon, 31 Jul 2023 08:16:12 GMT, Jay Bhaskar <jbhaskar at openjdk.org> wrote:

>> Issue: Some web worker tests fail to finish.
>> Root Cause:
>> sharedTimerFiredInternal function from ThreadTimers class does not require an isMainThread check, The check was introduced during the initial webkit stabilization.
>
> Jay Bhaskar has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add test case for worker timeout

The fix and test both look good. I confirm that it fixes the problem and that the new test fails without the fix and passes with the fix.

I left a couple minor comments on the test, but I'll approve it anyway. If you choose to fix them, I'll reapprove.

modules/javafx.web/src/test/java/test/javafx/scene/web/WebWorkerTest.java line 50:

> 48:     @After
> 49:     public void after() {
> 50:     }

Minor: you don't need this empty method.

modules/javafx.web/src/test/java/test/javafx/scene/web/WebWorkerTest.java line 64:

> 62:         } catch (InterruptedException e) {
> 63:             // Handle the exception if the thread is interrupted while sleeping
> 64:         }

Minor: if you add `throws InterruptedException` to the test method you don't need a try/catch here.

modules/javafx.web/src/test/java/test/javafx/scene/web/WebWorkerTest.java line 69:

> 67:             WebView view = getView();
> 68:             String res = (String) view.getEngine().executeScript("document.getElementById('result').innerText;");
> 69:             assertEquals("4",res);

Minor: space after the `,`

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

Marked as reviewed by kcr (Lead).

PR Review: https://git.openjdk.org/jfx/pull/1191#pullrequestreview-1555894189
PR Review Comment: https://git.openjdk.org/jfx/pull/1191#discussion_r1279944478
PR Review Comment: https://git.openjdk.org/jfx/pull/1191#discussion_r1280000329
PR Review Comment: https://git.openjdk.org/jfx/pull/1191#discussion_r1280000445