Skip to content

Commit

Permalink
JBR-5668: The implementation of a11y announcing for macOS crashes wit…
Browse files Browse the repository at this point in the history 
…h -Xcheck:jni.

- Create a global reference of the passed to EDT accessible object (the local reference) to use it in the AppKit thread ;
- Enable -Xcheck:jni in the tests ;
- Make the tests handle the problematic case .
  • Loading branch information
@NikitkoCent
NikitkoCent committed May 30, 2023
1 parent c229e13 commit cba981d
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 28 deletions.
48 changes: 34 additions & 14 deletions src/java.desktop/macosx/native/libawt_lwawt/awt/a11y/CommonComponentAccessibility.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1363,13 +1363,24 @@ - (BOOL)accessibilityPerformIncrement {
{
JNI_COCOA_ENTER(env);

NSString *text = JavaStringToNSString(env, str);
NSNumber *javaPriority = [NSNumber numberWithInt:priority];
NSString *const text = JavaStringToNSString(env, str);
NSNumber *const javaPriority = [NSNumber numberWithInt:priority];

// From JNI specification:
// > Local references are only valid in the thread in which they are created.
// > The native code must not pass local references from one thread to another.
//
// So we have to create a global ref and pass it to the AppKit thread.
const jobject jAccessibleGlobalRef = (jAccessible == NULL) ? NULL : (*env)->NewGlobalRef(env, jAccessible);

[ThreadUtilities performOnMainThreadWaiting:YES block:^{
nativeAnnounceAppKit(jAccessible, text, javaPriority);
nativeAnnounceAppKit(jAccessibleGlobalRef, text, javaPriority);
}];

if (jAccessibleGlobalRef != NULL) {
(*env)->DeleteGlobalRef(env, jAccessibleGlobalRef);
}

JNI_COCOA_EXIT(env);
}

Expand All @@ -1382,25 +1393,34 @@ void nativeAnnounceAppKit(
assert((text != NULL));
assert((javaPriority != NULL));

JNIEnv *env = [ThreadUtilities getJNIEnv];
JNIEnv* const env = [ThreadUtilities getJNIEnv];
if (env == NULL) { // unlikely
NSLog(@"%s: failed to get JNIEnv instance\n%@\n", __func__, [NSThread callStackSymbols]);
return;
return; // I believe it's dangerous to go on announcing in that case
}

id caller = nil;

DECLARE_CLASS(jc_Accessible, "javax/accessibility/Accessible");
// The meaning of this block is the following:
// if jAccessible is an instance of "javax/accessibility/Accessible"
// and sun.lwawt.macosx.CAccessible#getCAccessible(jAccessible) returns a non-null object
// then we obtain its private field sun.lwawt.macosx.CAccessible#ptr
// (which is a pointer to a native "part" of the accessible component)
// and assign it to caller
if (jAccessible != NULL) {
DECLARE_CLASS(jc_Accessible, "javax/accessibility/Accessible");

if ((*env)->IsInstanceOf(env, jAccessible, jc_Accessible) == JNI_TRUE) {
const jobject jCAX = [CommonComponentAccessibility getCAccessible:jAccessible withEnv:env];

if ( (jAccessible != NULL) && ((*env)->IsInstanceOf(env, jAccessible, jc_Accessible) == JNI_TRUE) ) {
GET_CACCESSIBLE_CLASS();
DECLARE_FIELD(jf_ptr, sjc_CAccessible, "ptr", "J");
if (jCAX != NULL) {
GET_CACCESSIBLE_CLASS();
DECLARE_FIELD(jf_ptr, sjc_CAccessible, "ptr", "J");

// try to fetch the jCAX from Java, and return autoreleased
const jobject jCAX = [CommonComponentAccessibility getCAccessible:jAccessible withEnv:env];
if (jCAX != NULL) {
caller = (CommonComponentAccessibility*)jlong_to_ptr((*env)->GetLongField(env, jCAX, jf_ptr));
(*env)->DeleteLocalRef(env, jCAX);
caller = (CommonComponentAccessibility*)jlong_to_ptr((*env)->GetLongField(env, jCAX, jf_ptr));

(*env)->DeleteLocalRef(env, jCAX);
}
}
}

Expand Down
39 changes: 25 additions & 14 deletions test/jdk/java/awt/a11y/AccessibleAnnouncerTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
* Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2022, JetBrains s.r.o.. All rights reserved.
* Copyright (c) 2022, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2022, 2023, JetBrains s.r.o.. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand All @@ -24,12 +24,15 @@

/*
* @test
* @modules java.desktop/sun.swing:+open
* @summary Test implementation of accessibility announcing
* @run main/manual AccessibleAnnouncerTest
* @key headful
* @modules java.desktop/sun.swing:+open
* @run main/othervm/manual -Xcheck:jni AccessibleAnnouncerTest false
* @run main/othervm/manual -Xcheck:jni AccessibleAnnouncerTest true
*/

import sun.swing.AccessibleAnnouncer;
import javax.accessibility.Accessible;
import javax.swing.JButton;
import javax.swing.JPanel;
import javax.swing.JTextField;
Expand All @@ -46,17 +49,24 @@

public class AccessibleAnnouncerTest extends AccessibleComponentTest {

private final boolean useFrameAsAnnouncer;

private AccessibleAnnouncerTest(final boolean useFrameAsAnnouncer) {
super();
this.useFrameAsAnnouncer = useFrameAsAnnouncer;
}


@java.lang.Override
public CountDownLatch createCountDownLatch() {
return new CountDownLatch(1);
}

private static void announce(final String str, final int priority) {
private static void announce(final Accessible accessible, final String str, final int priority) {
try {
AccessibleAnnouncer.announce(str, priority);
AccessibleAnnouncer.announce(accessible, str, priority);
} catch (final Exception e) {
e.printStackTrace();

}
}

Expand All @@ -77,15 +87,15 @@ void createTest() {
@Override
public void actionPerformed(ActionEvent e) {
String str = textField.getText();
announce(str, AccessibleAnnouncer.ANNOUNCE_WITH_INTERRUPTING_CURRENT_OUTPUT);
announce(useFrameAsAnnouncer ? frame : null, str, AccessibleAnnouncer.ANNOUNCE_WITH_INTERRUPTING_CURRENT_OUTPUT);
}
});

frame.setLayout(new FlowLayout());
frame.add(textField);
frame.add(button);
exceptionString = "Accessible announcer test failed!";
super.createUI(frame, "Accessible Anouncer test");
super.createUI(frame, "Accessible Anouncer test (useFrameAsAnnouncer=" + useFrameAsAnnouncer + ")");
}

void createPriorityTest() {
Expand All @@ -107,11 +117,11 @@ void createPriorityTest() {

@Override
public void actionPerformed(ActionEvent e) {
announce(firstMessage, AccessibleAnnouncer.ANNOUNCE_WITHOUT_INTERRUPTING_CURRENT_OUTPUT);
announce(useFrameAsAnnouncer ? frame : null, firstMessage, AccessibleAnnouncer.ANNOUNCE_WITHOUT_INTERRUPTING_CURRENT_OUTPUT);
try {
Thread.sleep(3000);
announce("You must not hear this message.", AccessibleAnnouncer.ANNOUNCE_WITHOUT_INTERRUPTING_CURRENT_OUTPUT);
announce(secondMessage, AccessibleAnnouncer.ANNOUNCE_WITH_INTERRUPTING_CURRENT_OUTPUT);
announce(useFrameAsAnnouncer ? frame : null, "You must not hear this message.", AccessibleAnnouncer.ANNOUNCE_WITHOUT_INTERRUPTING_CURRENT_OUTPUT);
announce(useFrameAsAnnouncer ? frame : null, secondMessage, AccessibleAnnouncer.ANNOUNCE_WITH_INTERRUPTING_CURRENT_OUTPUT);
} catch (Exception ex) {
ex.printStackTrace();
}
Expand All @@ -121,11 +131,12 @@ public void actionPerformed(ActionEvent e) {
frame.setLayout(new FlowLayout());
frame.add(button);
exceptionString = "Accessible announcer priority test failed!";
super.createUI(frame, "Accessible Anouncer test");
super.createUI(frame, "Accessible Anouncer test (useFrameAsAnnouncer=" + useFrameAsAnnouncer + ")");
}

public static void main(String[] args) throws Exception {
AccessibleAnnouncerTest test = new AccessibleAnnouncerTest();
final boolean useFrameAsAnnouncer = (args.length > 0) && ("true".equals(args[0]));
AccessibleAnnouncerTest test = new AccessibleAnnouncerTest(useFrameAsAnnouncer);

countDownLatch = test.createCountDownLatch();
SwingUtilities.invokeLater(test::createTest);
Expand Down

0 comments on commit cba981d

Please sign in to comment.