Accessibility on macOS: clean up reference counting logic

Simplify the "populate" functions to always create a new array, rather
than trying to reuse the existing array. Use (auto)release to free the
old arrays.

When creating place holder cells for a new row (where we know which row
array to add the cell to), shortcut the code to not find the row array.

Don't release cell elements that we got from the cache.

Consistently fix the row array size if it's not matching the table's.

Assert some more invariants and add logging output.

Fixes: QTBUG-122751
Pick-to: 6.7
Change-Id: Ie122a4754f6a3f4c19c5412a7c9e589f2861d4ae
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>
(cherry picked from commit 1f7e926411d1e0d958c37dc597f24101d410c46b)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Volker Hilsheimer 2024-08-20 16:42:33 +02:00 committed by Qt Cherry-pick Bot
parent c802de110b
commit 8633139e25

View File

@ -125,6 +125,10 @@ static void convertLineOffset(QAccessibleTextInterface *text, int *line, int *of
if (iface->tableInterface()) {
[self updateTableModel];
} else if (const auto *cell = iface->tableCellInterface()) {
// Called with role when populating a row with placeholder cells. The caller
// inserts us into the correct array, so we don't have to look for it.
if (role == NSAccessibilityCellRole)
return self;
// If we create an element for a table cell, initialize it with row/column
// and insert it into the corresponding row's columns array.
m_rowIndex = cell->rowIndex();
@ -156,8 +160,12 @@ static void convertLineOffset(QAccessibleTextInterface *text, int *line, int *of
<< "Table representation column count is out of sync:"
<< rowElement->columns.count << "!=" << tableInterface->columnCount();
}
rowElement->columns = [rowElement populateTableRow:rowElement->columns
count:tableInterface->columnCount()];
if (rowElement->columns) {
[rowElement->columns autorelease];
rowElement->columns = nil;
}
rowElement->columns = [rowElement populateTableRow:tableInterface->columnCount()];
[rowElement->columns retain];
}
qCDebug(lcAccessibilityTable) << "Creating cell representation for"
@ -203,10 +211,17 @@ static void convertLineOffset(QAccessibleTextInterface *text, int *line, int *of
return [self elementWithId:anId];
}
// called by QAccessibleCache::removeAccessibleElement
- (void)invalidate {
axid = 0;
rows = nil;
columns = nil;
if (rows) {
[rows autorelease];
rows = nil;
}
if (columns) {
[columns autorelease];
columns = nil;
}
synthesizedRole = nil;
NSAccessibilityPostNotification(self, NSAccessibilityUIElementDestroyedNotification);
@ -238,19 +253,10 @@ static void convertLineOffset(QAccessibleTextInterface *text, int *line, int *of
return synthesizedRole != nil;
}
- (NSMutableArray *)populateTableArray:(NSMutableArray *)array role:(NSAccessibilityRole)role count:(int)count
- (NSMutableArray *)populateTableArray:(NSAccessibilityRole)role count:(int)count
{
if (QAccessibleInterface *iface = self.qtInterface) {
if (array && int(array.count) != count) {
[array release];
array = nil;
}
if (!array) {
array = [NSMutableArray<QMacAccessibilityElement *> arrayWithCapacity:count];
[array retain];
} else {
[array removeAllObjects];
}
auto *array = [NSMutableArray<QMacAccessibilityElement *> arrayWithCapacity:count];
Q_ASSERT(array);
for (int n = 0; n < count; ++n) {
// columns will have same axid as table (but not inserted in cache)
@ -272,37 +278,33 @@ static void convertLineOffset(QAccessibleTextInterface *text, int *line, int *of
return nil;
}
- (NSMutableArray *)populateTableRow:(NSMutableArray *)array count:(int)count
- (NSMutableArray *)populateTableRow:(int)count
{
Q_ASSERT(synthesizedRole == NSAccessibilityRowRole);
if (array && int(array.count) != count) {
[array release];
array = nil;
}
qCDebug(lcAccessibilityTable) << "Populating table row" << m_rowIndex
<< "with" << count << "placeholder cells";
// When macOS asks for the children of a row, then we populate the row's column
// array with synthetic elements as place holders. This way, we don't have to
// create QAccessibleInterfaces for every cell before they are really needed.
// We don't add those synthetic elements into the cache, and we give them the
// same axid as the table. This way, we can get easily to the table, and from
// there to the QAccessibleInterface for the cell, when we have to eventually
// associate such an interface with the element (at which point it is no longer
// a placeholder).
auto *array = [NSMutableArray<QMacAccessibilityElement *> arrayWithCapacity:count];
Q_ASSERT(array);
if (!array) {
array = [NSMutableArray<QMacAccessibilityElement *> arrayWithCapacity:count];
[array retain];
// When macOS asks for the children of a row, then we populate the row's column
// array with synthetic elements as place holders. This way, we don't have to
// create QAccessibleInterfaces for every cell before they are really needed.
// We don't add those synthetic elements into the cache, and we give them the
// same axid as the table. This way, we can get easily to the table, and from
// there to the QAccessibleInterface for the cell, when we have to eventually
// associate such an interface with the element (at which point it is no longer
// a placeholder).
for (int n = 0; n < count; ++n) {
// columns will have same axid as table (but not inserted in cache)
QMacAccessibilityElement *cell =
[[QMacAccessibilityElement alloc] initWithId:axid role:NSAccessibilityCellRole];
if (cell) {
cell->m_rowIndex = m_rowIndex;
cell->m_columnIndex = n;
[array addObject:cell];
}
for (int n = 0; n < count; ++n) {
// columns will have same axid as table (but not inserted in cache)
QMacAccessibilityElement *cell =
[[QMacAccessibilityElement alloc] initWithId:axid role:NSAccessibilityCellRole];
if (cell) {
cell->m_rowIndex = m_rowIndex;
cell->m_columnIndex = n;
[array addObject:cell];
[cell release];
}
}
Q_ASSERT(array);
return array;
}
@ -313,8 +315,18 @@ static void convertLineOffset(QAccessibleTextInterface *text, int *line, int *of
Q_ASSERT(!self.isManagedByParent);
qCDebug(lcAccessibilityTable) << "Updating table representation with"
<< table->rowCount() << table->columnCount();
rows = [self populateTableArray:rows role:NSAccessibilityRowRole count:table->rowCount()];
columns = [self populateTableArray:columns role:NSAccessibilityColumnRole count:table->columnCount()];
if (rows) {
[rows autorelease];
rows = nil;
}
rows = [self populateTableArray:NSAccessibilityRowRole count:table->rowCount()];
[rows retain];
if (columns) {
[columns autorelease];
columns = nil;
}
columns = [self populateTableArray:NSAccessibilityColumnRole count:table->columnCount()];
[columns retain];
}
}
}
@ -350,7 +362,6 @@ static void convertLineOffset(QAccessibleTextInterface *text, int *line, int *of
if (cellElement != self) {
// for the same cell position
Q_ASSERT(cellElement->m_rowIndex == m_rowIndex && cellElement->m_columnIndex == m_columnIndex);
[cellElement release];
}
}
@ -467,8 +478,16 @@ static void convertLineOffset(QAccessibleTextInterface *text, int *line, int *of
// axid matches the parent table axid so that we can easily find the parent table
// children of row are cell/any items
Q_ASSERT(m_rowIndex >= 0);
const int numColumns = table->columnCount();
columns = [self populateTableRow:columns count:numColumns];
Q_ASSERT(rows == nil);
const unsigned int numColumns = table->columnCount();
if (!columns || columns.count != numColumns) {
if (columns) {
[columns autorelease];
columns = nil;
}
columns = [self populateTableRow:numColumns];
[columns retain];
}
return NSAccessibilityUnignoredChildren(columns);
}
}
@ -995,13 +1014,27 @@ static void convertLineOffset(QAccessibleTextInterface *text, int *line, int *of
- (NSArray *) accessibilityRows {
if (!synthesizedRole && rows) {
QAccessibleInterface *iface = self.qtInterface;
if (iface && iface->tableInterface())
QAccessibleTableInterface *tableInterface = iface ? iface->tableInterface() : nullptr;
if (tableInterface) {
const unsigned int rowCount = tableInterface->rowCount();
if (rows.count != rowCount) {
qCDebug(lcAccessibilityTable) << "Updating table rows with" << rowCount << "rows";
if (rows) {
[rows autorelease];
rows = nil;
}
rows = [self populateTableArray:NSAccessibilityRowRole
count:rowCount];
[rows retain];
}
return NSAccessibilityUnignoredChildren(rows);
}
}
return nil;
}
- (NSArray *) accessibilityColumns {
// we only implement this for a table, not for rows
if (!synthesizedRole && columns) {
QAccessibleInterface *iface = self.qtInterface;
if (iface && iface->tableInterface())