Re: RFR: 8298644 JNI call of getCurrentComponent on a wrong thread [v8]
On Tue, 10 Jan 2023 11:27:50 GMT, Artem Semenov wrote: >> [OutlineRowAccessibility currentAccessibleWithENV:] defines the >> getCurrentComponent method on the AccessibleContext instance of >> AccessibleJTreeNode class, however the call should go through CAccessibility >> so that it is executed on the Event Dispatch thread. > > Artem Semenov has updated the pull request incrementally with one additional > commit since the last revision: > > It is better to force the "ensureClassInitialized" as any other getters in > this class. To make sure the field is up-to-date even if it was initialized > on another thread. Marked as reviewed by kizune (Reviewer). - PR: https://git.openjdk.org/jdk/pull/11670
Re: RFR: 8298644 JNI call of getCurrentComponent on a wrong thread [v8]
On Tue, 10 Jan 2023 11:27:50 GMT, Artem Semenov wrote: >> [OutlineRowAccessibility currentAccessibleWithENV:] defines the >> getCurrentComponent method on the AccessibleContext instance of >> AccessibleJTreeNode class, however the call should go through CAccessibility >> so that it is executed on the Event Dispatch thread. > > Artem Semenov has updated the pull request incrementally with one additional > commit since the last revision: > > It is better to force the "ensureClassInitialized" as any other getters in > this class. To make sure the field is up-to-date even if it was initialized > on another thread. @prrace @azuev-java please review - PR: https://git.openjdk.org/jdk/pull/11670
Re: RFR: 8298644 JNI call of getCurrentComponent on a wrong thread [v8]
On Tue, 10 Jan 2023 11:27:50 GMT, Artem Semenov wrote: >> [OutlineRowAccessibility currentAccessibleWithENV:] defines the >> getCurrentComponent method on the AccessibleContext instance of >> AccessibleJTreeNode class, however the call should go through CAccessibility >> so that it is executed on the Event Dispatch thread. > > Artem Semenov has updated the pull request incrementally with one additional > commit since the last revision: > > It is better to force the "ensureClassInitialized" as any other getters in > this class. To make sure the field is up-to-date even if it was initialized > on another thread. Marked as reviewed by serb (Reviewer). - PR: https://git.openjdk.org/jdk/pull/11670
Re: RFR: 8298644 JNI call of getCurrentComponent on a wrong thread [v8]
On Tue, 10 Jan 2023 11:27:50 GMT, Artem Semenov wrote: >> [OutlineRowAccessibility currentAccessibleWithENV:] defines the >> getCurrentComponent method on the AccessibleContext instance of >> AccessibleJTreeNode class, however the call should go through CAccessibility >> so that it is executed on the Event Dispatch thread. > > Artem Semenov has updated the pull request incrementally with one additional > commit since the last revision: > > It is better to force the "ensureClassInitialized" as any other getters in > this class. To make sure the field is up-to-date even if it was initialized > on another thread. Marked as reviewed by avu (Committer). - PR: https://git.openjdk.org/jdk/pull/11670
Re: RFR: 8298644 JNI call of getCurrentComponent on a wrong thread [v8]
> [OutlineRowAccessibility currentAccessibleWithENV:] defines the > getCurrentComponent method on the AccessibleContext instance of > AccessibleJTreeNode class, however the call should go through CAccessibility > so that it is executed on the Event Dispatch thread. Artem Semenov has updated the pull request incrementally with one additional commit since the last revision: It is better to force the "ensureClassInitialized" as any other getters in this class. To make sure the field is up-to-date even if it was initialized on another thread. - Changes: - all: https://git.openjdk.org/jdk/pull/11670/files - new: https://git.openjdk.org/jdk/pull/11670/files/2841c5e4..9e4f87c4 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=11670&range=07 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=11670&range=06-07 Stats: 12 lines in 1 file changed: 6 ins; 6 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/11670.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11670/head:pull/11670 PR: https://git.openjdk.org/jdk/pull/11670
Re: RFR: 8298644 JNI call of getCurrentComponent on a wrong thread [v7]
> [OutlineRowAccessibility currentAccessibleWithENV:] defines the > getCurrentComponent method on the AccessibleContext instance of > AccessibleJTreeNode class, however the call should go through CAccessibility > so that it is executed on the Event Dispatch thread. Artem Semenov has updated the pull request incrementally with one additional commit since the last revision: It is better to force the "ensureClassInitialized" as any other getters in this class. To make sure the field is up-to-date even if it was initialized on another thread. - Changes: - all: https://git.openjdk.org/jdk/pull/11670/files - new: https://git.openjdk.org/jdk/pull/11670/files/ed0752e8..2841c5e4 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=11670&range=06 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=11670&range=05-06 Stats: 16 lines in 1 file changed: 8 ins; 8 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/11670.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11670/head:pull/11670 PR: https://git.openjdk.org/jdk/pull/11670
Re: RFR: 8298644 JNI call of getCurrentComponent on a wrong thread [v6]
> [OutlineRowAccessibility currentAccessibleWithENV:] defines the > getCurrentComponent method on the AccessibleContext instance of > AccessibleJTreeNode class, however the call should go through CAccessibility > so that it is executed on the Event Dispatch thread. Artem Semenov has updated the pull request incrementally with one additional commit since the last revision: It is better to force the "ensureClassInitialized" as any other getters in this class. To make sure the field is up-to-date even if it was initialized on another thread. - Changes: - all: https://git.openjdk.org/jdk/pull/11670/files - new: https://git.openjdk.org/jdk/pull/11670/files/6efb419e..ed0752e8 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=11670&range=05 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=11670&range=04-05 Stats: 46 lines in 1 file changed: 23 ins; 23 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/11670.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11670/head:pull/11670 PR: https://git.openjdk.org/jdk/pull/11670
Re: RFR: 8298644 JNI call of getCurrentComponent on a wrong thread [v5]
> [OutlineRowAccessibility currentAccessibleWithENV:] defines the > getCurrentComponent method on the AccessibleContext instance of > AccessibleJTreeNode class, however the call should go through CAccessibility > so that it is executed on the Event Dispatch thread. Artem Semenov has updated the pull request incrementally with two additional commits since the last revision: - It is better to force the "ensureClassInitialized" as any other getters in this class. To make sure the field is up-to-date even if it was initialized on another thread. - Note that this class declares interfaces at the top, and then field/get/set at the bottom of the file. - Changes: - all: https://git.openjdk.org/jdk/pull/11670/files - new: https://git.openjdk.org/jdk/pull/11670/files/bddabea5..6efb419e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=11670&range=04 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=11670&range=03-04 Stats: 26 lines in 1 file changed: 11 ins; 14 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/11670.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11670/head:pull/11670 PR: https://git.openjdk.org/jdk/pull/11670
Re: RFR: 8298644 JNI call of getCurrentComponent on a wrong thread [v4]
On Thu, 29 Dec 2022 20:52:52 GMT, Sergey Bylokhov wrote:
>> Artem Semenov has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains four commits:
>>
>> - Merge master
>> - update accessor
>> - Please split the long lines to have 80 chars per line.
>> - 8298644 JNI call of getCurrentComponent on a wrong thread
>
> src/java.desktop/share/classes/sun/swing/SwingAccessor.java line 60:
>
>> 58:
>> 59: public static AccessibleComponentAccessor
>> getAccessibleComponentAccessor() {
>> 60: return accessibleComponentAccessor;
>
> It is better to force the "ensureClassInitialized" as any other getters in
> this class. To make sure the field is up-to-date even if it was initialized
> on another thread.
Done
> src/java.desktop/share/classes/sun/swing/SwingAccessor.java line 71:
>
>> 69: * For example, the renderer of a list element, a table cell, or a
>> tree node
>> 70: */
>> 71: public interface AccessibleComponentAccessor {
>
> Note that this class declares interfaces at the top, and then field/get/set
> at the bottom of the file.
done
-
PR: https://git.openjdk.org/jdk/pull/11670
Re: RFR: 8298644 JNI call of getCurrentComponent on a wrong thread [v4]
On Thu, 29 Dec 2022 14:23:40 GMT, Artem Semenov wrote:
>> [OutlineRowAccessibility currentAccessibleWithENV:] defines the
>> getCurrentComponent method on the AccessibleContext instance of
>> AccessibleJTreeNode class, however the call should go through CAccessibility
>> so that it is executed on the Event Dispatch thread.
>
> Artem Semenov has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains four commits:
>
> - Merge master
> - update accessor
> - Please split the long lines to have 80 chars per line.
> - 8298644 JNI call of getCurrentComponent on a wrong thread
src/java.desktop/share/classes/sun/swing/SwingAccessor.java line 60:
> 58:
> 59: public static AccessibleComponentAccessor
> getAccessibleComponentAccessor() {
> 60: return accessibleComponentAccessor;
It is better to force the "ensureClassInitialized" as any other getters in this
class. To make sure the field is up-to-date even if it was initialized on
another thread.
src/java.desktop/share/classes/sun/swing/SwingAccessor.java line 71:
> 69: * For example, the renderer of a list element, a table cell, or a
> tree node
> 70: */
> 71: public interface AccessibleComponentAccessor {
Note that this class declares interfaces at the top, and then field/get/set at
the bottom of the file.
-
PR: https://git.openjdk.org/jdk/pull/11670
Re: RFR: 8298644 JNI call of getCurrentComponent on a wrong thread [v4]
On Thu, 29 Dec 2022 14:23:40 GMT, Artem Semenov wrote: >> [OutlineRowAccessibility currentAccessibleWithENV:] defines the >> getCurrentComponent method on the AccessibleContext instance of >> AccessibleJTreeNode class, however the call should go through CAccessibility >> so that it is executed on the Event Dispatch thread. > > Artem Semenov has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains four commits: > > - Merge master > - update accessor > - Please split the long lines to have 80 chars per line. > - 8298644 JNI call of getCurrentComponent on a wrong thread Looks good for me - Marked as reviewed by avu (Committer). PR: https://git.openjdk.org/jdk/pull/11670
Re: RFR: 8298644 JNI call of getCurrentComponent on a wrong thread [v4]
> [OutlineRowAccessibility currentAccessibleWithENV:] defines the > getCurrentComponent method on the AccessibleContext instance of > AccessibleJTreeNode class, however the call should go through CAccessibility > so that it is executed on the Event Dispatch thread. Artem Semenov has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains four commits: - Merge master - update accessor - Please split the long lines to have 80 chars per line. - 8298644 JNI call of getCurrentComponent on a wrong thread - Changes: https://git.openjdk.org/jdk/pull/11670/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=11670&range=03 Stats: 85 lines in 4 files changed: 63 ins; 15 del; 7 mod Patch: https://git.openjdk.org/jdk/pull/11670.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11670/head:pull/11670 PR: https://git.openjdk.org/jdk/pull/11670
Re: RFR: 8298644 JNI call of getCurrentComponent on a wrong thread [v4]
On Sat, 24 Dec 2022 00:35:07 GMT, Sergey Bylokhov wrote: >> Thank you. >> I removed adding accesses for TableCell and ListChildren. If that's what you >> meant. >> I would like to keep the implementation of the Accesser itself, if possible. >> Because in the future it may come in handy in this form, if in the future >> you still need to add access to the currentComponent for ListChildren and >> TableCell. >> this approach allows us to make the implementation more generic, in case the >> potential tree is not inherited from Jtree. It also avoids reflection. > > Probably we can simplify it further. usually, the SwingAccessor is used to > store one accessor per class, which does not need to have an additional list > of functions. And if the a11y code needs to call different methods for > different types of AccessibleComponent then it uses "instanceof" or role > check inside the a11y code. Can we do the same here? It will have the same > functionality. Done - PR: https://git.openjdk.org/jdk/pull/11670
Re: RFR: 8298644 JNI call of getCurrentComponent on a wrong thread [v3]
> [OutlineRowAccessibility currentAccessibleWithENV:] defines the > getCurrentComponent method on the AccessibleContext instance of > AccessibleJTreeNode class, however the call should go through CAccessibility > so that it is executed on the Event Dispatch thread. Artem Semenov has updated the pull request incrementally with one additional commit since the last revision: update accessor - Changes: - all: https://git.openjdk.org/jdk/pull/11670/files - new: https://git.openjdk.org/jdk/pull/11670/files/8bad4ba2..4fd570e6 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=11670&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=11670&range=01-02 Stats: 108 lines in 6 files changed: 45 ins; 54 del; 9 mod Patch: https://git.openjdk.org/jdk/pull/11670.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11670/head:pull/11670 PR: https://git.openjdk.org/jdk/pull/11670
Re: RFR: 8298644 JNI call of getCurrentComponent on a wrong thread [v2]
On Fri, 23 Dec 2022 09:15:47 GMT, Artem Semenov wrote:
>> src/java.desktop/share/classes/sun/swing/AccessibleComponentAccessor.java
>> line 42:
>>
>>> 40: }
>>> 41:
>>> 42: public static Accessible getAccessible(AccessibleContext context) {
>>
>> Will the fix have more code if it will be implemented via SwingAccessor just
>> to access that "private" method?
>
> Thank you.
> I removed adding accesses for TableCell and ListChildren. If that's what you
> meant.
> I would like to keep the implementation of the Accesser itself, if possible.
> Because in the future it may come in handy in this form, if in the future you
> still need to add access to the currentComponent for ListChildren and
> TableCell.
> this approach allows us to make the implementation more generic, in case the
> potential tree is not inherited from Jtree. It also avoids reflection.
Probably we can simplify it further. usually, the SwingAccessor is used to
store one accessor per class, which does not need to have an additional list of
functions. And if the a11y code needs to call different methods for different
types of AccessibleComponent then it uses "instanceof" or role check inside the
a11y code. Can we do the same here? It will have the same functionality.
-
PR: https://git.openjdk.org/jdk/pull/11670
Re: RFR: 8298644 JNI call of getCurrentComponent on a wrong thread [v2]
On Thu, 22 Dec 2022 22:01:39 GMT, Sergey Bylokhov wrote:
>> Artem Semenov has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Please split the long lines to have 80 chars per line.
>
> src/java.desktop/share/classes/javax/swing/JList.java line 3234:
>
>> 3232: static {
>> 3233: AccessibleComponentAccessor.addAccessor(c ->
>> 3234: c instanceof
>> JList.AccessibleJList.AccessibleJListChild ?
>> ((JList.AccessibleJList.AccessibleJListChild) c).getCurrentComponent() :
>> null);
>
> Please split the long lines to have 80 chars per line.
Donw
> src/java.desktop/share/classes/sun/swing/AccessibleComponentAccessor.java
> line 42:
>
>> 40: }
>> 41:
>> 42: public static Accessible getAccessible(AccessibleContext context) {
>
> Will the fix have more code if it will be implemented via SwingAccessor just
> to access that "private" method?
Thank you.
I removed adding accesses for TableCell and ListChildren. If that's what you
meant.
I would like to keep the implementation of the Accesser itself, if possible.
Because in the future it may come in handy in this form, if in the future you
still need to add access to the currentComponent for ListChildren and TableCell.
this approach allows us to make the implementation more generic, in case the
potential tree is not inherited from Jtree. It also avoids reflection.
-
PR: https://git.openjdk.org/jdk/pull/11670
Re: RFR: 8298644 JNI call of getCurrentComponent on a wrong thread [v2]
> [OutlineRowAccessibility currentAccessibleWithENV:] defines the > getCurrentComponent method on the AccessibleContext instance of > AccessibleJTreeNode class, however the call should go through CAccessibility > so that it is executed on the Event Dispatch thread. Artem Semenov has updated the pull request incrementally with one additional commit since the last revision: Please split the long lines to have 80 chars per line. - Changes: - all: https://git.openjdk.org/jdk/pull/11670/files - new: https://git.openjdk.org/jdk/pull/11670/files/256a2d2e..8bad4ba2 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=11670&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=11670&range=00-01 Stats: 14 lines in 3 files changed: 2 ins; 11 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/11670.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11670/head:pull/11670 PR: https://git.openjdk.org/jdk/pull/11670
Re: RFR: 8298644 JNI call of getCurrentComponent on a wrong thread
On Wed, 14 Dec 2022 10:47:09 GMT, Artem Semenov wrote:
> [OutlineRowAccessibility currentAccessibleWithENV:] defines the
> getCurrentComponent method on the AccessibleContext instance of
> AccessibleJTreeNode class, however the call should go through CAccessibility
> so that it is executed on the Event Dispatch thread.
src/java.desktop/share/classes/javax/swing/JList.java line 3234:
> 3232: static {
> 3233: AccessibleComponentAccessor.addAccessor(c ->
> 3234: c instanceof
> JList.AccessibleJList.AccessibleJListChild ?
> ((JList.AccessibleJList.AccessibleJListChild) c).getCurrentComponent() :
> null);
Please split the long lines to have 80 chars per line.
src/java.desktop/share/classes/sun/swing/AccessibleComponentAccessor.java line
42:
> 40: }
> 41:
> 42: public static Accessible getAccessible(AccessibleContext context) {
Will the fix have more code if it will be implemented via SwingAccessor just to
access that "private" method?
-
PR: https://git.openjdk.org/jdk/pull/11670
RFR: 8298644 JNI call of getCurrentComponent on a wrong thread
[OutlineRowAccessibility currentAccessibleWithENV:] defines the getCurrentComponent method on the AccessibleContext instance of AccessibleJTreeNode class, however the call should go through CAccessibility so that it is executed on the Event Dispatch thread. - Commit messages: - 8298644 JNI call of getCurrentComponent on a wrong thread Changes: https://git.openjdk.org/jdk/pull/11670/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=11670&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8298644 Stats: 99 lines in 6 files changed: 80 ins; 14 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/11670.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11670/head:pull/11670 PR: https://git.openjdk.org/jdk/pull/11670
