<AWT Dev> RFR: 8263846: Bad JNI lookup getFocusOwner in accessibility code on Mac OS X

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

<AWT Dev> RFR: 8263846: Bad JNI lookup getFocusOwner in accessibility code on Mac OS X

Alexander Zuev-3
8263846: Bad JNI lookup getFocusOwner in accessibility code on Mac OS X

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

Commit messages:
 - 8263846: Bad JNI lookup getFocusOwner in accessibility code on Mac OS X

Changes: https://git.openjdk.java.net/jdk/pull/3099/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3099&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8263846
  Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3099.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3099/head:pull/3099

PR: https://git.openjdk.java.net/jdk/pull/3099
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> RFR: 8263846: Bad JNI lookup getFocusOwner in accessibility code on Mac OS X

Alexander Zvegintsev-2
On Fri, 19 Mar 2021 23:10:46 GMT, Alexander Zuev <[hidden email]> wrote:

> 8263846: Bad JNI lookup getFocusOwner in accessibility code on Mac OS X

Since there is no test, I assume that the JBS issue should have `noreg-hard` label.

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

Marked as reviewed by azvegint (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3099
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> RFR: 8263846: Bad JNI lookup getFocusOwner in accessibility code on Mac OS X

Sergey Bylokhov-2
In reply to this post by Alexander Zuev-3
On Fri, 19 Mar 2021 23:10:46 GMT, Alexander Zuev <[hidden email]> wrote:

> 8263846: Bad JNI lookup getFocusOwner in accessibility code on Mac OS X

src/java.desktop/macosx/native/libawt_lwawt/awt/JavaComponentAccessibility.m line 1459:

> 1457: {
> 1458:     JNIEnv *env = [ThreadUtilities getJNIEnv];
> 1459:     GET_CACCESSIBILITY_CLASS_RETURN(nil);

Is it caused by the JDK-8257853? Should we update the accessibilityIndexOfChild as well?

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

PR: https://git.openjdk.java.net/jdk/pull/3099
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> RFR: 8263846: Bad JNI lookup getFocusOwner in accessibility code on Mac OS X [v2]

Alexander Zuev-3
In reply to this post by Alexander Zvegintsev-2
On Fri, 19 Mar 2021 23:41:34 GMT, Alexander Zvegintsev <[hidden email]> wrote:

> Since there is no test, I assume that the JBS issue should have `noreg-hard` label.

Yes, as with most of platform-specific accessibility issues just configuring a system to be able to invoke this native functionality is quite hard. I will put the corresponding keyword into the bug.

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

PR: https://git.openjdk.java.net/jdk/pull/3099
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> RFR: 8263846: Bad JNI lookup getFocusOwner in accessibility code on Mac OS X [v2]

Alexander Zuev-3
In reply to this post by Alexander Zuev-3
> 8263846: Bad JNI lookup getFocusOwner in accessibility code on Mac OS X

Alexander Zuev has updated the pull request incrementally with one additional commit since the last revision:

  Fixing accessibilityIndexOfChild in the same way

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

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3099/files
  - new: https://git.openjdk.java.net/jdk/pull/3099/files/bc186078..46b6e417

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3099&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3099&range=00-01

  Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3099.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3099/head:pull/3099

PR: https://git.openjdk.java.net/jdk/pull/3099
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> RFR: 8263846: Bad JNI lookup getFocusOwner in accessibility code on Mac OS X [v2]

Alexander Zuev-3
In reply to this post by Sergey Bylokhov-2
On Sat, 20 Mar 2021 02:13:46 GMT, Sergey Bylokhov <[hidden email]> wrote:

>> Alexander Zuev has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   Fixing accessibilityIndexOfChild in the same way
>
> src/java.desktop/macosx/native/libawt_lwawt/awt/JavaComponentAccessibility.m line 1459:
>
>> 1457: {
>> 1458:     JNIEnv *env = [ThreadUtilities getJNIEnv];
>> 1459:     GET_CACCESSIBILITY_CLASS_RETURN(nil);
>
> Is it caused by the JDK-8257853? Should we update the accessibilityIndexOfChild as well?

It made it visible however it is just a good practice not to assume that class reference is already initialized before using it. So, yes, while i was not seeing any failures in accessibilityIndexOfChild  i might fix it as well.

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

PR: https://git.openjdk.java.net/jdk/pull/3099
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> RFR: 8263846: Bad JNI lookup getFocusOwner in accessibility code on Mac OS X [v2]

Phil Race
In reply to this post by Alexander Zuev-3
On Sat, 20 Mar 2021 14:01:51 GMT, Alexander Zuev <[hidden email]> wrote:

>> 8263846: Bad JNI lookup getFocusOwner in accessibility code on Mac OS X
>
> Alexander Zuev has updated the pull request incrementally with one additional commit since the last revision:
>
>   Fixing accessibilityIndexOfChild in the same way

src/java.desktop/macosx/native/libawt_lwawt/awt/JavaComponentAccessibility.m line 802:

> 800:
> 801:     JNIEnv *env = [ThreadUtilities getJNIEnv];
> 802:     GET_CACCESSIBILITY_CLASS_RETURN(0);

This isn't necessary. Because the line below takes care of it. The definition looks like
#define GET_ACCESSIBLEINDEXINPARENT_STATIC_METHOD_RETURN(ret) \
    GET_CACCESSIBILITY_CLASS_RETURN(ret); \
    GET_STATIC_METHOD_RETURN(sjm_getAccessibleIndexInParent, sjc_CAccessibility, "getAccessibleIndexInParent", \
                             "(Ljavax/accessibility/Accessible;Ljava/awt/Component;)I", ret);

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

PR: https://git.openjdk.java.net/jdk/pull/3099
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> RFR: 8263846: Bad JNI lookup getFocusOwner in accessibility code on Mac OS X [v2]

Phil Race
In reply to this post by Alexander Zuev-3
On Sat, 20 Mar 2021 13:57:21 GMT, Alexander Zuev <[hidden email]> wrote:

>> src/java.desktop/macosx/native/libawt_lwawt/awt/JavaComponentAccessibility.m line 1459:
>>
>>> 1457: {
>>> 1458:     JNIEnv *env = [ThreadUtilities getJNIEnv];
>>> 1459:     GET_CACCESSIBILITY_CLASS_RETURN(nil);
>>
>> Is it caused by the JDK-8257853? Should we update the accessibilityIndexOfChild as well?
>
> It made it visible however it is just a good practice not to assume that class reference is already initialized before using it. So, yes, while i was not seeing any failures in accessibilityIndexOfChild  i might fix it as well.

If this was missed in the JNF work, add the label "jnf" to the bug.

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

PR: https://git.openjdk.java.net/jdk/pull/3099
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> RFR: 8263846: Bad JNI lookup getFocusOwner in accessibility code on Mac OS X [v3]

Alexander Zuev-3
In reply to this post by Alexander Zuev-3
> 8263846: Bad JNI lookup getFocusOwner in accessibility code on Mac OS X

Alexander Zuev has updated the pull request incrementally with one additional commit since the last revision:

  Reverting the change for accessibilityIndexOfChild since macro used
  there takes care of the class initialization.

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

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3099/files
  - new: https://git.openjdk.java.net/jdk/pull/3099/files/46b6e417..e0c3b9dd

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3099&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3099&range=01-02

  Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3099.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3099/head:pull/3099

PR: https://git.openjdk.java.net/jdk/pull/3099
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> RFR: 8263846: Bad JNI lookup getFocusOwner in accessibility code on Mac OS X [v2]

Alexander Zuev-3
In reply to this post by Phil Race
On Sat, 20 Mar 2021 17:06:38 GMT, Phil Race <[hidden email]> wrote:

>> Alexander Zuev has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   Fixing accessibilityIndexOfChild in the same way
>
> src/java.desktop/macosx/native/libawt_lwawt/awt/JavaComponentAccessibility.m line 802:
>
>> 800:
>> 801:     JNIEnv *env = [ThreadUtilities getJNIEnv];
>> 802:     GET_CACCESSIBILITY_CLASS_RETURN(0);
>
> This isn't necessary. Because the line below takes care of it. The definition looks like
> #define GET_ACCESSIBLEINDEXINPARENT_STATIC_METHOD_RETURN(ret) \
>     GET_CACCESSIBILITY_CLASS_RETURN(ret); \
>     GET_STATIC_METHOD_RETURN(sjm_getAccessibleIndexInParent, sjc_CAccessibility, "getAccessibleIndexInParent", \
>                              "(Ljavax/accessibility/Accessible;Ljava/awt/Component;)I", ret);

Ah, ok, was missed that. Will revert this change.

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

PR: https://git.openjdk.java.net/jdk/pull/3099
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> RFR: 8263846: Bad JNI lookup getFocusOwner in accessibility code on Mac OS X [v3]

Alexander Zuev-3
In reply to this post by Phil Race
On Sat, 20 Mar 2021 17:07:41 GMT, Phil Race <[hidden email]> wrote:

>> It made it visible however it is just a good practice not to assume that class reference is already initialized before using it. So, yes, while i was not seeing any failures in accessibilityIndexOfChild  i might fix it as well.
>
> If this was missed in the JNF work, add the label "jnf" to the bug.

Done.

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

PR: https://git.openjdk.java.net/jdk/pull/3099
Reply | Threaded
Open this post in threaded view
|

Re: <AWT Dev> RFR: 8263846: Bad JNI lookup getFocusOwner in accessibility code on Mac OS X [v3]

Phil Race
In reply to this post by Alexander Zuev-3
On Sat, 20 Mar 2021 17:19:50 GMT, Alexander Zuev <[hidden email]> wrote:

>> 8263846: Bad JNI lookup getFocusOwner in accessibility code on Mac OS X
>
> Alexander Zuev has updated the pull request incrementally with one additional commit since the last revision:
>
>   Reverting the change for accessibilityIndexOfChild since macro used
>   there takes care of the class initialization.

Marked as reviewed by prr (Reviewer).

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

PR: https://git.openjdk.java.net/jdk/pull/3099
Reply | Threaded
Open this post in threaded view
|

<AWT Dev> Integrated: 8263846: Bad JNI lookup getFocusOwner in accessibility code on Mac OS X

Alexander Zuev-3
In reply to this post by Alexander Zuev-3
On Fri, 19 Mar 2021 23:10:46 GMT, Alexander Zuev <[hidden email]> wrote:

> 8263846: Bad JNI lookup getFocusOwner in accessibility code on Mac OS X

This pull request has now been integrated.

Changeset: 118a49fc
Author:    Alexander Zuev <[hidden email]>
URL:       https://git.openjdk.java.net/jdk/commit/118a49fc
Stats:     1 line in 1 file changed: 1 ins; 0 del; 0 mod

8263846: Bad JNI lookup getFocusOwner in accessibility code on Mac OS X

Reviewed-by: azvegint, prr

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

PR: https://git.openjdk.java.net/jdk/pull/3099