Re: RFR: 8376420: Remove AppContext from javax/swing/ImageIcon.java [v2]
On Fri, 30 Jan 2026 20:50:14 GMT, Sergey Bylokhov wrote: > The field yes, but the component itself is not a constant. Does it matter? The component is needed to create the `MediaTracker` instance. Then, media tracker uses `checkImage` and `prepareImage` which delegate to the `Toolkit`. Can the application change anything in the `Component` instance which would prevent the `checkImage` and `prepareImage` method from working correctly? *Unlikely*. Either way, I'm for using a different component to create the `MediaTracker` instance instead of re-using the long-deprecated `component` field. It's cleaner and safer, isn't it? - PR Review Comment: https://git.openjdk.org/jdk/pull/29433#discussion_r2782658010
Re: RFR: 8376420: Remove AppContext from javax/swing/ImageIcon.java [v2]
On Thu, 29 Jan 2026 17:36:04 GMT, Alexey Ivanov wrote: >>> Does an app need direct access to the `MediaTracker` that's used to >>> download the image? Probably not, therefore a private member is better. >> >> The application will have this access via getter already, the problematic >> part here is access to the Component which is mutable so we cannot use it >> via final field. > >> > Does an app need direct access to the `MediaTracker` that's used to >> > download the image? Probably not, therefore a private member is better. >> >> The application will have this access via getter already > > I'm confused… Where's a getter in `ImageIcon` that an app can use to get a > `MediaTracker` instance? > > The `getTracker` method is *private*. > >> the problematic part here is access to the Component which is mutable so we >> cannot use it via final field. > > The `component` field is *final*. The field yes, but the component itself is not a constant. - PR Review Comment: https://git.openjdk.org/jdk/pull/29433#discussion_r2747942137
Re: RFR: 8376420: Remove AppContext from javax/swing/ImageIcon.java [v2]
On Wed, 28 Jan 2026 21:11:58 GMT, Phil Race wrote: >> Remove per-AppContext MediaTracker from ImageIcon.java > > Phil Race has updated the pull request incrementally with one additional > commit since the last revision: > > 8376420 Marked as reviewed by psadhukhan (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/29433#pullrequestreview-3727161366
Re: RFR: 8376420: Remove AppContext from javax/swing/ImageIcon.java [v2]
On Thu, 29 Jan 2026 17:37:01 GMT, Alexey Ivanov wrote: >> Component is an abstract class. > > Yeah, I missed it… I should've looked more thoroughly at the declaration of > the `Component` class. I wonder if creating a final static class extending `Component` is more efficient than creating two anonymous classes for the `component` file and for the media tracker. However, it's a micro-optimisation. - PR Review Comment: https://git.openjdk.org/jdk/pull/29433#discussion_r2742844283
Re: RFR: 8376420: Remove AppContext from javax/swing/ImageIcon.java [v2]
On Wed, 28 Jan 2026 20:31:00 GMT, Phil Race wrote:
>> src/java.desktop/share/classes/javax/swing/ImageIcon.java line 346:
>>
>>> 344: }
>>> 345:
>>> 346: private static final MediaTracker MEDIA_TRACKER = new
>>> MediaTracker(new Component() {});
>>
>> Is there a particular reason to create a new anonymous class extending
>> `Component` instead of simply instantiating?
>>
>>
>> new MediaTracker(new Component());
>>
>>
>> Is it only to follow what `createComponent` does?
>>
>> With the removal of `AppContext` and `SecurityManager`, is the anonymous
>> class still necessary?
>
> Component is an abstract class.
Yeah, I missed it… I should've looked more thoroughly at the declaration of the
`Component` class.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/29433#discussion_r2742778775
Re: RFR: 8376420: Remove AppContext from javax/swing/ImageIcon.java [v2]
On Wed, 28 Jan 2026 21:06:27 GMT, Sergey Bylokhov wrote: > > Does an app need direct access to the `MediaTracker` that's used to > > download the image? Probably not, therefore a private member is better. > > The application will have this access via getter already I'm confused… Where's a getter in `ImageIcon` that an app can use to get a `MediaTracker` instance? The `getTracker` method is *private*. > the problematic part here is access to the Component which is mutable so we > cannot use it via final field. The `component` field is *final*. - PR Review Comment: https://git.openjdk.org/jdk/pull/29433#discussion_r2742775613
Re: RFR: 8376420: Remove AppContext from javax/swing/ImageIcon.java [v2]
On Wed, 28 Jan 2026 21:11:58 GMT, Phil Race wrote: >> Remove per-AppContext MediaTracker from ImageIcon.java > > Phil Race has updated the pull request incrementally with one additional > commit since the last revision: > > 8376420 Marked as reviewed by aivanov (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/29433#pullrequestreview-3724137660
Re: RFR: 8376420: Remove AppContext from javax/swing/ImageIcon.java [v2]
On Wed, 28 Jan 2026 21:05:50 GMT, Phil Race wrote: >>> I am pretty sure this was AppContext related, since we could not return a >>> MediaTracker stored in one AppContext to another AppContext and now we are >>> doing exactly that, but ok not a big issue. >> >> Very much likely. >> >> Does an app need direct access to the `MediaTracker` that's used to download >> the image? Probably not, therefore a private member is better. >> >>> The only difference between the two is that the old MediaTracker component >>> was created using createComponent, which resets the AppContext, should we >>> drop that AppContext usage there as well? >> >> Indeed, the component created via `createComponent` doesn't store a >> reference to app context. >> >> https://github.com/prrace/jdk/blob/08f95226d2c638cae74f51836b7fc509ed5b0b8b/src/java.desktop/share/classes/javax/swing/ImageIcon.java#L116-L119 >> >> I guess it'll get removed when the `appContext` field gets removed from >> `Component`. >> >> On the other hand, the shared `MediaTracker` instance introduced in this PR >> could use `createComponent` to instead of creating a new instance of >> `Component`. > >> I am pretty sure this was AppContext related, since we could not return a >> MediaTracker stored in one AppContext to another AppContext and now we are >> doing exactly that, but ok not a big issue. >> > > Not a big issue because only tests create 2 ACs now and the tests and AC > itself will be removed as soon as I can get past these leaf issues. > > Going further back it still doesn't seem to be the case that it was an AC fix. > It was actually due to a security issue with ACC (AccessControlContext). > That fix is now no longer there because we removed SM code a couple of > releases ago. > Now we can simplify it even further. > > But it still doesn't seem like we need to resurrect using the visible field. > >> The only difference between the two is that the old MediaTracker component >> was created using createComponent, which resets the AppContext, should we >> drop that AppContext usage there as well? > > Yes, I don't know why I didn't do that. > Does an app need direct access to the `MediaTracker` that's used to download > the image? Probably not, therefore a private member is better. The application will have this access via getter already, the problematic part here is access to the Component which is mutable so we cannot use it via final field. - PR Review Comment: https://git.openjdk.org/jdk/pull/29433#discussion_r2738610762
Re: RFR: 8376420: Remove AppContext from javax/swing/ImageIcon.java [v2]
On Wed, 28 Jan 2026 20:09:42 GMT, Alexey Ivanov wrote: > I am pretty sure this was AppContext related, since we could not return a > MediaTracker stored in one AppContext to another AppContext and now we are > doing exactly that, but ok not a big issue. > Not a big issue because only tests create 2 ACs now and the tests and AC itself will be removed as soon as I can get past these leaf issues. Going further back it still doesn't seem to be the case that it was an AC fix. It was actually due to a security issue with ACC (AccessControlContext). That fix is now no longer there because we removed SM code a couple of releases ago. Now we can simplify it even further. But it still doesn't seem like we need to resurrect using the visible field. > The only difference between the two is that the old MediaTracker component > was created using createComponent, which resets the AppContext, should we > drop that AppContext usage there as well? Yes, I don't know why I didn't do that. - PR Review Comment: https://git.openjdk.org/jdk/pull/29433#discussion_r2738608359
Re: RFR: 8376420: Remove AppContext from javax/swing/ImageIcon.java [v2]
> Remove per-AppContext MediaTracker from ImageIcon.java Phil Race has updated the pull request incrementally with one additional commit since the last revision: 8376420 - Changes: - all: https://git.openjdk.org/jdk/pull/29433/files - new: https://git.openjdk.org/jdk/pull/29433/files/08f95226..477de10f Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=29433&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=29433&range=00-01 Stats: 12 lines in 1 file changed: 0 ins; 11 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/29433.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/29433/head:pull/29433 PR: https://git.openjdk.org/jdk/pull/29433
Re: RFR: 8376420: Remove AppContext from javax/swing/ImageIcon.java
On Wed, 28 Jan 2026 20:19:42 GMT, Alexey Ivanov wrote:
>> Remove per-AppContext MediaTracker from ImageIcon.java
>
> src/java.desktop/share/classes/javax/swing/ImageIcon.java line 346:
>
>> 344: }
>> 345:
>> 346: private static final MediaTracker MEDIA_TRACKER = new
>> MediaTracker(new Component() {});
>
> Is there a particular reason to create a new anonymous class extending
> `Component` instead of simply instantiating?
>
>
> new MediaTracker(new Component());
>
>
> Is it only to follow what `createComponent` does?
>
> With the removal of `AppContext` and `SecurityManager`, is the anonymous
> class still necessary?
Component is an abstract class.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/29433#discussion_r2738492964
Re: RFR: 8376420: Remove AppContext from javax/swing/ImageIcon.java
On Mon, 26 Jan 2026 23:00:16 GMT, Phil Race wrote:
> Remove per-AppContext MediaTracker from ImageIcon.java
src/java.desktop/share/classes/javax/swing/ImageIcon.java line 346:
> 344: }
> 345:
> 346: private static final MediaTracker MEDIA_TRACKER = new
> MediaTracker(new Component() {});
Is there a particular reason to create a new anonymous class extending
`Component` instead of simply instantiating?
new MediaTracker(new Component());
Is it only to follow what `createComponent` does?
With the removal of `AppContext` and `SecurityManager`, is the anonymous class
still necessary?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/29433#discussion_r2738456590
Re: RFR: 8376420: Remove AppContext from javax/swing/ImageIcon.java
On Wed, 28 Jan 2026 19:39:21 GMT, Sergey Bylokhov wrote: > I am pretty sure this was AppContext related, since we could not return a > MediaTracker stored in one AppContext to another AppContext and now we are > doing exactly that, but ok not a big issue. Very much likely. Does an app need direct access to the `MediaTracker` that's used to download the image? Probably not, therefore a private member is better. > The only difference between the two is that the old MediaTracker component > was created using createComponent, which resets the AppContext, should we > drop that AppContext usage there as well? Indeed, the component created via `createComponent` doesn't store a reference to app context. https://github.com/prrace/jdk/blob/08f95226d2c638cae74f51836b7fc509ed5b0b8b/src/java.desktop/share/classes/javax/swing/ImageIcon.java#L116-L119 I guess it'll get removed when the `appContext` field gets removed from `Component`. On the other hand, the shared `MediaTracker` instance introduced in this PR could use `createComponent` to instead of creating a new instance of `Component`. - PR Review Comment: https://git.openjdk.org/jdk/pull/29433#discussion_r2738423738
Re: RFR: 8376420: Remove AppContext from javax/swing/ImageIcon.java
On Wed, 28 Jan 2026 15:54:55 GMT, Alexey Ivanov wrote: >> Not as far as I can see >> https://hg.openjdk.org/jdk8/jdk8/jdk/rev/54a6e22b749c >> I think that it was visible to apps was the issue. > > I agree it's safer not to use the deprecated field *that's visible to apps*. I am pretty sure this was AppContext related, since we could not return a MediaTracker stored in one AppContext to another AppContext and now we are doing exactly that, but ok not a big issue. The only difference between the two is that the old MediaTracker component was created using createComponent, which resets the AppContext, should we drop that AppContext usage there as well? - PR Review Comment: https://git.openjdk.org/jdk/pull/29433#discussion_r2738293304
Re: RFR: 8376420: Remove AppContext from javax/swing/ImageIcon.java
On Tue, 27 Jan 2026 06:02:15 GMT, Phil Race wrote:
>> src/java.desktop/share/classes/javax/swing/ImageIcon.java line 346:
>>
>>> 344: }
>>> 345:
>>> 346: private static final MediaTracker MEDIA_TRACKER = new
>>> MediaTracker(new Component() {});
>>
>> This actually duplicates an existing declaration:
>> `protected static final MediaTracker tracker = new MediaTracker(component);`
>> I assume it was deprecated in Java 8 because of the AppContext issue? so now
>> we can just uses that one?
>
> Not as far as I can see
> https://hg.openjdk.org/jdk8/jdk8/jdk/rev/54a6e22b749c
> I think that it was visible to apps was the issue.
I agree it's safer not to use the deprecated field *that's visible to apps*.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/29433#discussion_r2737319689
Re: RFR: 8376420: Remove AppContext from javax/swing/ImageIcon.java
On Mon, 26 Jan 2026 23:00:16 GMT, Phil Race wrote: > Remove per-AppContext MediaTracker from ImageIcon.java Marked as reviewed by aivanov (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/29433#pullrequestreview-3717494348
Re: RFR: 8376420: Remove AppContext from javax/swing/ImageIcon.java
On Mon, 26 Jan 2026 23:24:43 GMT, Sergey Bylokhov wrote:
>> Remove per-AppContext MediaTracker from ImageIcon.java
>
> src/java.desktop/share/classes/javax/swing/ImageIcon.java line 346:
>
>> 344: }
>> 345:
>> 346: private static final MediaTracker MEDIA_TRACKER = new
>> MediaTracker(new Component() {});
>
> This actually duplicates an existing declaration:
> `protected static final MediaTracker tracker = new MediaTracker(component);`
> I assume it was deprecated in Java 8 because of the AppContext issue? so now
> we can just uses that one?
Not as far as I can see
https://hg.openjdk.org/jdk8/jdk8/jdk/rev/54a6e22b749c
I think that it was visible to apps was the issue.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/29433#discussion_r2730397737
Re: RFR: 8376420: Remove AppContext from javax/swing/ImageIcon.java
On Mon, 26 Jan 2026 23:00:16 GMT, Phil Race wrote:
> Remove per-AppContext MediaTracker from ImageIcon.java
src/java.desktop/share/classes/javax/swing/ImageIcon.java line 346:
> 344: }
> 345:
> 346: private static final MediaTracker MEDIA_TRACKER = new
> MediaTracker(new Component() {});
This actually duplicates an existing declaration:
`protected static final MediaTracker tracker = new MediaTracker(component);`
I assume it was deprecated in Java 8 because of the AppContext issue? so now we
can just uses that one?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/29433#discussion_r2729655436
