Skip to content

Commit

Permalink
8273678: TableAccessibility and TableRowAccessibility miss autorelease
Browse files Browse the repository at this point in the history
Reviewed-by: ant, kizune, pbansal
  • Loading branch information
savoptik authored and Anton Tarasov committed Oct 25, 2021
1 parent 7cf68b1 commit 3221a14
Show file tree
Hide file tree
Showing 5 changed files with 153 additions and 95 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ public static CAccessible getCAccessible(final Accessible a) {
private static native void treeNodeExpanded(long ptr);
private static native void treeNodeCollapsed(long ptr);
private static native void selectedCellsChanged(long ptr);
private static native void tableContentCacheClear(long ptr);

private Accessible accessible;

Expand Down Expand Up @@ -124,6 +125,13 @@ public void propertyChange(PropertyChangeEvent e) {
selectionChanged(ptr);
} else if (name.equals(ACCESSIBLE_TABLE_MODEL_CHANGED)) {
valueChanged(ptr);
if (CAccessible.getSwingAccessible(CAccessible.this) != null) {
Accessible a = CAccessible.getSwingAccessible(CAccessible.this);
AccessibleContext ac = a.getAccessibleContext();
if ((ac != null) && (ac.getAccessibleRole() == AccessibleRole.TABLE)) {
tableContentCacheClear(ptr);
}
}
} else if (name.equals(ACCESSIBLE_ACTIVE_DESCENDANT_PROPERTY)) {
if (newValue instanceof AccessibleContext) {
activeDescendant = (AccessibleContext)newValue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,61 +49,6 @@ - (NSAccessibilityRole)accessibilityRole
return NSAccessibilityColumnRole;
}

- (NSArray *)accessibilityChildren
{
NSArray *children = [super accessibilityChildren];
if (children == NULL) {
JNIEnv *env = [ThreadUtilities getJNIEnv];
CommonComponentAccessibility *parent = [self accessibilityParent];
if (parent->fAccessible == NULL) return nil;
GET_CHILDRENANDROLES_METHOD_RETURN(nil);
jobjectArray jchildrenAndRoles = (jobjectArray)(*env)->CallStaticObjectMethod(env, sjc_CAccessibility, jm_getChildrenAndRoles,
parent->fAccessible, parent->fComponent, sun_lwawt_macosx_CAccessibility_JAVA_AX_ALL_CHILDREN, NO);
CHECK_EXCEPTION();
if (jchildrenAndRoles == NULL) return nil;

jsize arrayLen = (*env)->GetArrayLength(env, jchildrenAndRoles);
NSMutableArray *childrenCells = [NSMutableArray arrayWithCapacity:arrayLen/2];

NSUInteger childIndex = fIndex;

int inc = [(TableAccessibility *)[self accessibilityParent] accessibilityRowCount] * 2;
NSInteger i = childIndex * 2;
for(i; i < arrayLen; i += inc)
{
jobject /* Accessible */ jchild = (*env)->GetObjectArrayElement(env, jchildrenAndRoles, i);
jobject /* String */ jchildJavaRole = (*env)->GetObjectArrayElement(env, jchildrenAndRoles, i+1);

NSString *childJavaRole = nil;
if (jchildJavaRole != NULL) {
DECLARE_CLASS_RETURN(sjc_AccessibleRole, "javax/accessibility/AccessibleRole", nil);
DECLARE_FIELD_RETURN(sjf_key, sjc_AccessibleRole, "key", "Ljava/lang/String;", nil);
jobject jkey = (*env)->GetObjectField(env, jchildJavaRole, sjf_key);
CHECK_EXCEPTION();
childJavaRole = JavaStringToNSString(env, jkey);
(*env)->DeleteLocalRef(env, jkey);
}

CellAccessibility *child = [[CellAccessibility alloc] initWithParent:self
withEnv:env
withAccessible:jchild
withIndex:childIndex
withView:self->fView
withJavaRole:childJavaRole];
[childrenCells addObject:[[child retain] autorelease]];

(*env)->DeleteLocalRef(env, jchild);
(*env)->DeleteLocalRef(env, jchildJavaRole);

childIndex += (inc / 2);
}
(*env)->DeleteLocalRef(env, jchildrenAndRoles);
return childrenCells;
} else {
return children;
}
}

- (NSInteger)accessibilityIndex
{
return fIndex;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@
#import "CommonComponentAccessibility.h"

@interface TableAccessibility : CommonComponentAccessibility <NSAccessibilityTable>
{
NSMutableDictionary<NSNumber*, id> *rowCache;
}

- (BOOL)isAccessibleChildSelectedFromIndex:(int)index;
- (int) accessibleRowAtIndex:(int)index;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#import "JNIUtilities.h"
#import "CellAccessibility.h"
#import "sun_lwawt_macosx_CAccessibility.h"
#import "sun_lwawt_macosx_CAccessible.h"

static jclass sjc_CAccessibility = NULL;

Expand Down Expand Up @@ -124,22 +125,23 @@ - (BOOL) isAccessibleChildSelectedFromIndex:(int)index

- (TableRowAccessibility *)createRowWithIndex:(NSUInteger)index
{
return [[TableRowAccessibility alloc] initWithParent:self
withEnv:[ThreadUtilities getJNIEnv]
withAccessible:NULL
withIndex:index
withView:[self view]
withJavaRole:JavaAccessibilityIgnore];
}
if (rowCache == nil) {
int rowCount = [self accessibilityRowCount];
rowCache = [[NSMutableDictionary<NSNumber*, id> dictionaryWithCapacity:rowCount] retain];
}

- (ColumnAccessibility *)createColumnWithIndex:(NSUInteger)index
{
return [[ColumnAccessibility alloc] initWithParent:self
withEnv:[ThreadUtilities getJNIEnv]
withAccessible:NULL
withIndex:index
withView:self->fView
withJavaRole:JavaAccessibilityIgnore];
id row = [rowCache objectForKey:[NSNumber numberWithUnsignedInteger:index]];
if (row == nil) {
row = [[TableRowAccessibility alloc] initWithParent:self
withEnv:[ThreadUtilities getJNIEnv]
withAccessible:NULL
withIndex:index
withView:[self view]
withJavaRole:JavaAccessibilityIgnore];
[rowCache setObject:row forKey:[NSNumber numberWithUnsignedInteger:index]];
}

return row;
}

// NSAccessibilityElement protocol methods
Expand Down Expand Up @@ -189,26 +191,6 @@ - (id)accessibilityParent
return [super accessibilityParent];
}

- (nullable NSArray *)accessibilityColumns
{
int colCount = [self accessibilityColumnCount];
NSMutableArray *columns = [NSMutableArray arrayWithCapacity:colCount];
for (int i = 0; i < colCount; i++) {
[columns addObject:[self createColumnWithIndex:i]];
}
return [NSArray arrayWithArray:columns];
}

- (nullable NSArray *)accessibilitySelectedColumns
{
NSArray<NSNumber *> *indexes = [self getTableSelectedInfo:sun_lwawt_macosx_CAccessibility_JAVA_AX_COLS];
NSMutableArray *columns = [NSMutableArray arrayWithCapacity:[indexes count]];
for (NSNumber *i in indexes) {
[columns addObject:[self createColumnWithIndex:i.unsignedIntValue]];
}
return [NSArray arrayWithArray:columns];
}

- (NSInteger)accessibilityRowCount
{
return [[self getTableInfo:sun_lwawt_macosx_CAccessibility_JAVA_AX_ROWS] integerValue];
Expand Down Expand Up @@ -238,4 +220,28 @@ - (NSArray *)accessibilitySelectedCells
return [NSArray arrayWithArray:selectedCells];
}

- (void)clearCache {
for (NSNumber *key in [rowCache allKeys]) {
[[rowCache objectForKey:key] release];
}
[rowCache release];
rowCache = nil;
}

@end

/*
* Class: sun_lwawt_macosx_CAccessible
* Method: tableContentIndexDestroy
* Signature: (J)V
*/
JNIEXPORT void JNICALL Java_sun_lwawt_macosx_CAccessible_tableContentCacheClear
(JNIEnv *env, jclass class, jlong element)
{
JNI_COCOA_ENTER(env);
[ThreadUtilities performOnMainThread:@selector(clearCache)
on:(CommonComponentAccessibility *)jlong_to_ptr(element)
withObject:nil
waitUntilDone:NO];
JNI_COCOA_EXIT(env);
}
106 changes: 101 additions & 5 deletions test/jdk/java/awt/a11y/AccessibleJTableTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,15 @@
* @requires (os.family == "windows" | os.family == "mac")
*/

import javax.swing.JTable;
import javax.swing.JPanel;
import javax.swing.JScrollPane;
import javax.swing.SwingUtilities;
import javax.swing.*;
import javax.swing.event.TableModelEvent;
import javax.swing.event.TableModelListener;
import javax.swing.table.AbstractTableModel;
import javax.swing.table.TableModel;

import java.awt.FlowLayout;
import java.awt.*;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;

Expand Down Expand Up @@ -94,6 +97,65 @@ public void createUINamed() {
super.createUI(panel, "AccessibleJTableTest");
}

public void createUIWithChangingContent() {
INSTRUCTIONS = "INSTRUCTIONS:\n"
+ "Check a11y of dynamic JTable.\n\n"
+ "Turn screen reader on, and Tab to the table.\n"
+ "Add and remove rows and columns using the appropriate buttons and try to move around the table\n\n"
+ "If you hear changes in the table - ctrl+tab further and press PASS, otherwise press FAIL.\n";

JTable table = new JTable(new TestTableModel(3, 3));

JPanel panel = new JPanel();
panel.setLayout(new FlowLayout());
JScrollPane scrollPane = new JScrollPane(table);
panel.add(scrollPane);

JPanel buttonPanel = new JPanel(new GridLayout());
JButton addRow = new JButton("Add row");
addRow.addActionListener(new ActionListener() {
@Override
public void actionPerformed(ActionEvent e) {
table.setModel(new TestTableModel(table.getModel().getRowCount() + 1, table.getModel().getColumnCount()));
}
});

JButton addColumn = new JButton("Add column");
addColumn.addActionListener(new ActionListener() {
@Override
public void actionPerformed(ActionEvent e) {
table.setModel(new TestTableModel(table.getModel().getRowCount(), table.getModel().getColumnCount() + 1));
}
});

JButton removeRow = new JButton("Remove row");
removeRow.addActionListener(new ActionListener() {
@Override
public void actionPerformed(ActionEvent e) {
table.setModel(new TestTableModel(table.getModel().getRowCount() - 1, table.getModel().getColumnCount()));
}
});

JButton removeColumn = new JButton("Remove column");
removeColumn.addActionListener(new ActionListener() {
@Override
public void actionPerformed(ActionEvent e) {
table.setModel(new TestTableModel(table.getModel().getRowCount(), table.getModel().getColumnCount() - 1));
}
});

buttonPanel.add(addRow);
buttonPanel.add(addColumn);
buttonPanel.add(removeRow);
buttonPanel.add(removeColumn);

panel.add(buttonPanel);

panel.setFocusable(false);
exceptionString = "AccessibleJTable test failed!";
super.createUI(panel, "AccessibleJTableTest");
}

public static void main(String[] args) throws Exception {
AccessibleJTableTest test = new AccessibleJTableTest();

Expand All @@ -110,5 +172,39 @@ public static void main(String[] args) throws Exception {
if (!testResult) {
throw new RuntimeException(exceptionString);
}

countDownLatch = test.createCountDownLatch();
SwingUtilities.invokeAndWait(test::createUIWithChangingContent);
countDownLatch.await(15, TimeUnit.MINUTES);
if (!testResult) {
throw new RuntimeException(exceptionString);
}

}

private static class TestTableModel extends AbstractTableModel {
private final int rows;
private final int cols;

TestTableModel(final int r, final int c) {
super();
rows = r;
cols = c;
}

@Override
public int getRowCount() {
return rows >= 0 ? rows : 0;
}

@Override
public int getColumnCount() {
return cols >= 0 ? cols : 0;
}

@Override
public Object getValueAt(int rowIndex, int columnIndex) {
return String.valueOf((rowIndex + 1) * (columnIndex + 1));
}
}
}

3 comments on commit 3221a14

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@savoptik
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/backport jdk17u

@openjdk
Copy link

@openjdk openjdk bot commented on 3221a14 Nov 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@savoptik Could not automatically backport 3221a14f to openjdk/jdk17u due to conflicts in the following files:

  • src/java.desktop/macosx/classes/sun/lwawt/macosx/CAccessible.java

To manually resolve these conflicts run the following commands in your personal fork of openjdk/jdk17u:

$ git checkout -b savoptik-backport-3221a14f
$ git fetch --no-tags https://git.openjdk.java.net/jdk 3221a14f9eaf002d91597d84efdb125704710a4c
$ git cherry-pick --no-commit 3221a14f9eaf002d91597d84efdb125704710a4c
$ # Resolve conflicts
$ git add files/with/resolved/conflicts
$ git commit -m 'Backport 3221a14f9eaf002d91597d84efdb125704710a4c'

Once you have resolved the conflicts as explained above continue with creating a pull request towards the openjdk/jdk17u with the title Backport 3221a14f9eaf002d91597d84efdb125704710a4c.

Please sign in to comment.