Re: RFR: 8376423: Test javax/swing/plaf/metal/MetalUtils/bug6190373.java failed: ClassCastException: class java.lang.Character cannot be cast to class javax.swing.Painter [v3]

2026-01-29 Thread Alexey Ivanov
On Wed, 28 Jan 2026 21:56:14 GMT, Phil Race  wrote:

> I'm trying (as much as I can) to remove them only when they fail, so I can 
> relate it closely to a specific place where AppContext is removed.
> 
> So I'll not remove those additional tests here, but soon enough they'll go.

Thanks! I'd follow the same approach.

-

PR Comment: https://git.openjdk.org/jdk/pull/29437#issuecomment-3819155513


Re: RFR: 8376423: Test javax/swing/plaf/metal/MetalUtils/bug6190373.java failed: ClassCastException: class java.lang.Character cannot be cast to class javax.swing.Painter [v3]

2026-01-28 Thread Tejesh R
On Tue, 27 Jan 2026 21:14:47 GMT, Phil Race  wrote:

>> AppContext is removed from Swing LAF state. Tests which require it are 
>> deleted.
>
> Phil Race has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8376423

Marked as reviewed by tr (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/29437#pullrequestreview-3720657019


Re: RFR: 8376423: Test javax/swing/plaf/metal/MetalUtils/bug6190373.java failed: ClassCastException: class java.lang.Character cannot be cast to class javax.swing.Painter [v3]

2026-01-28 Thread Phil Race
On Wed, 28 Jan 2026 04:45:19 GMT, Tejesh R  wrote:

>> Phil Race has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   8376423
>
> Test with same name 
> [Test6657026.java](https://github.com/openjdk/jdk/pull/29437/changes#diff-d392211a6f8e0b0b65dbf68abd45ca2ddf89e6bd787be1cd1ed20d0c33eb9644)
>  referring to same bug ID 6657026 exist in other places too, should we remove 
> those test too ? 
> Example : same file name referring to same bugid exists in path 
> "open/test/jdk/javax/swing/ToolTipManager/ ", 
> "open/test/jdk/javax/swing/plaf/metal/MetalBorders/ ", etc.,

> @TejeshR13, you're right, there are other tests which reference JDK-6657026:
> 
> ```
> test/jdk/javax/swing/plaf/metal/MetalSliderUI/Test6657026.java
> test/jdk/javax/swing/plaf/metal/MetalInternalFrameUI/Test6657026.java
> test/jdk/javax/swing/plaf/metal/MetalBorders/Test6657026.java
> test/jdk/javax/swing/plaf/metal/MetalBumps/Test6657026.java
> test/jdk/javax/swing/plaf/basic/BasicSplitPaneUI/Test6657026.java
> test/jdk/javax/swing/ToolTipManager/Test6657026.java
> test/jdk/javax/swing/UIManager/Test6657026.java
> ```
> 
> I looked at a few at the top. They test that the data are bound to 
> `AppContext`. Yet none of these tests fails for me, even with the changes 
> from this PR.
> 
> Still, these tests should be removed sooner… or later.

A couple of these are on my short-term radar. 

> @TejeshR13, you're right, there are other tests which reference JDK-6657026:
> 
> ```
> test/jdk/javax/swing/plaf/metal/MetalSliderUI/Test6657026.java
> test/jdk/javax/swing/plaf/metal/MetalInternalFrameUI/Test6657026.java
> test/jdk/javax/swing/plaf/metal/MetalBorders/Test6657026.java
> test/jdk/javax/swing/plaf/metal/MetalBumps/Test6657026.java
> test/jdk/javax/swing/plaf/basic/BasicSplitPaneUI/Test6657026.java
> test/jdk/javax/swing/ToolTipManager/Test6657026.java
> test/jdk/javax/swing/UIManager/Test6657026.java
> ```
> 
> I looked at a few at the top. They test that the data are bound to 
> `AppContext`. Yet none of these tests fails for me, even with the changes 
> from this PR.
> 
> Still, these tests should be removed sooner… or later.

 test/jdk/javax/swing/UIManager/Test6657026.java is removed as part of this PR 
because it does fail.

as you say, the others do not fail although MetalBumps fails for me in a 
different (metal) fix I'm working on.

So I'm not sure they are testing what they should be.
I expect I'll remove all the metal tests in the Metal fix that'll be coming 
shortly.

The tooltip one is not one I'd noticed at all, but in general rather than just 
removing all app context tests up front, I'm trying (as much as I can) to 
remove them only when they fail, so I can relate it closely to a specific place 
where AppContext is removed.

So I'll not remove those additional tests here, but soon enough they'll go.

-

PR Comment: https://git.openjdk.org/jdk/pull/29437#issuecomment-3814135901


Re: RFR: 8376423: Test javax/swing/plaf/metal/MetalUtils/bug6190373.java failed: ClassCastException: class java.lang.Character cannot be cast to class javax.swing.Painter [v3]

2026-01-28 Thread Alexey Ivanov
On Wed, 28 Jan 2026 04:45:19 GMT, Tejesh R  wrote:

>> Phil Race has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   8376423
>
> Test with same name 
> [Test6657026.java](https://github.com/openjdk/jdk/pull/29437/changes#diff-d392211a6f8e0b0b65dbf68abd45ca2ddf89e6bd787be1cd1ed20d0c33eb9644)
>  referring to same bug ID 6657026 exist in other places too, should we remove 
> those test too ? 
> Example : same file name referring to same bugid exists in path 
> "open/test/jdk/javax/swing/ToolTipManager/ ", 
> "open/test/jdk/javax/swing/plaf/metal/MetalBorders/ ", etc.,

@TejeshR13, you're right, there are other tests which reference JDK-6657026:


test/jdk/javax/swing/plaf/metal/MetalSliderUI/Test6657026.java
test/jdk/javax/swing/plaf/metal/MetalInternalFrameUI/Test6657026.java
test/jdk/javax/swing/plaf/metal/MetalBorders/Test6657026.java
test/jdk/javax/swing/plaf/metal/MetalBumps/Test6657026.java
test/jdk/javax/swing/plaf/basic/BasicSplitPaneUI/Test6657026.java
test/jdk/javax/swing/ToolTipManager/Test6657026.java
test/jdk/javax/swing/UIManager/Test6657026.java


I looked at a few at the top. They test that the data are bound to 
`AppContext`. Yet none of these tests fails for me, even with the changes from 
this PR.

Still, these tests should be removed sooner… or later.

-

PR Comment: https://git.openjdk.org/jdk/pull/29437#issuecomment-3812146566


Re: RFR: 8376423: Test javax/swing/plaf/metal/MetalUtils/bug6190373.java failed: ClassCastException: class java.lang.Character cannot be cast to class javax.swing.Painter [v3]

2026-01-27 Thread Tejesh R
On Tue, 27 Jan 2026 21:14:47 GMT, Phil Race  wrote:

>> AppContext is removed from Swing LAF state. Tests which require it are 
>> deleted.
>
> Phil Race has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8376423

Test with same name 
[Test6657026.java](https://github.com/openjdk/jdk/pull/29437/changes#diff-d392211a6f8e0b0b65dbf68abd45ca2ddf89e6bd787be1cd1ed20d0c33eb9644)
 referring to same bug ID 6657026 exist in other places too, should we remove 
those test too ? 
Example : same file name referring to same bugid exists in path 
"open/test/jdk/javax/swing/ToolTipManager/ ", 
"open/test/jdk/javax/swing/plaf/metal/MetalBorders/ ", etc.,

-

PR Review: https://git.openjdk.org/jdk/pull/29437#pullrequestreview-3714560432


Re: RFR: 8376423: Test javax/swing/plaf/metal/MetalUtils/bug6190373.java failed: ClassCastException: class java.lang.Character cannot be cast to class javax.swing.Painter [v3]

2026-01-27 Thread Phil Race
On Tue, 27 Jan 2026 20:49:05 GMT, Alexey Ivanov  wrote:

>> The existing code only used that when creating the instance, not when 
>> retrieving an already created instance, so I don't see why we'd need to 
>> extend the lock to the retrieval.
>
> Indeed, the existing code use the lock only when creating the instance.
> 
> Yet there are quite a few places where `synchronized (classLock)` is used. In 
> particular, `maybeInitialize` method.
> 
> https://github.com/openjdk/jdk/blob/eb6e74b1fa794bf16f572d5dbce157d1cae4c505/src/java.desktop/share/classes/javax/swing/UIManager.java#L1461-L1468
> 
> The logic in `maybeInitialize` will remain thread-safe, yet 
> `SwingAccessor.isLafStateInitialized` can return a stale value of `false`. 
> Thus, `SwingAccessor.getLAFStateAccessor().lafStateIsInitialized()` in 
> `DefaultMetalTheme` can return `false` even after `UIManager` was 
> initialised. I guess this problem existed in the original code, too.
> 
> To ensure thread-safe access to the `initialized` flag, the method 
> `UIManager.isLafStateInitialized` has to access the flag inside the 
> `synchronized (classLock)` block.

OK. I've updated it.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/29437#discussion_r2733831629


Re: RFR: 8376423: Test javax/swing/plaf/metal/MetalUtils/bug6190373.java failed: ClassCastException: class java.lang.Character cannot be cast to class javax.swing.Painter [v3]

2026-01-27 Thread Alexey Ivanov
On Tue, 27 Jan 2026 21:11:03 GMT, Phil Race  wrote:

>> AppContext is removed from Swing LAF state. Tests which require it are 
>> deleted.
>
> Phil Race has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8376423

Marked as reviewed by aivanov (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/29437#pullrequestreview-3713470478


Re: RFR: 8376423: Test javax/swing/plaf/metal/MetalUtils/bug6190373.java failed: ClassCastException: class java.lang.Character cannot be cast to class javax.swing.Painter [v3]

2026-01-27 Thread Phil Race
> AppContext is removed from Swing LAF state. Tests which require it are 
> deleted.

Phil Race has updated the pull request incrementally with one additional commit 
since the last revision:

  8376423

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/29437/files
  - new: https://git.openjdk.org/jdk/pull/29437/files/8dcdc6fa..aeef9323

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=29437&range=02
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=29437&range=01-02

  Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/29437.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/29437/head:pull/29437

PR: https://git.openjdk.org/jdk/pull/29437