a11y: Don't reassign unique ID to other object right away

For the case that a newly created and registered accessible
interface gets removed again from the cache before another one
gets registered, the next registered interface was
previously assigned the same "unique ID" again, which e.g. breaks
assistive technology when using caching
with AT-SPI, since that relies on the assumption
that the ID is actually unique for each object.
(But here, the new object was using the same object path
as the old one, so data from the old object would be
used for the new one.)

To prevent that from happening, increment the
counter for the next ID to try at the end of
QAccessibleCache::acquireId, so the next time
the method gets called, it doesn't try again
whether the same ID as used previously is
available again.

For consistency, also rename the variable used
for the counter from lastUsedId to nextId.

This also adds a corresponding test case.

Fixes: QTBUG-105962
Pick-to: 6.4
Change-Id: Iddf4f3b35c57895bcfbb623a5377edf8344ab6c2
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>

src/gui/accessible/qaccessiblecache.cpp

@@ -48,18 +48,18 @@ QAccessibleCache *QAccessibleCache::instance()
 QAccessible::Id QAccessibleCache::acquireId() const
 {
     static const QAccessible::Id FirstId = QAccessible::Id(INT_MAX) + 1;
-    static QAccessible::Id lastUsedId = FirstId;
+    static QAccessible::Id nextId = FirstId;
 
-    while (idToInterface.contains(lastUsedId)) {
+    while (idToInterface.contains(nextId)) {
         // (wrap back when when we reach UINT_MAX - 1)
         // -1 because on Android -1 is taken for the "View" so just avoid it completely for consistency
-        if (lastUsedId == UINT_MAX - 1)
-            lastUsedId = FirstId;
+        if (nextId == UINT_MAX - 1)
+            nextId = FirstId;
         else
-            ++lastUsedId;
+            ++nextId;
     }
 
-    return lastUsedId;
+    return nextId++;
 }
 
 QAccessibleInterface *QAccessibleCache::interfaceForId(QAccessible::Id id) const

tests/auto/other/qaccessibility/tst_qaccessibility.cpp

@@ -213,6 +213,7 @@ private slots:
     void treeTest();
     void tableTest();
 
+    void uniqueIdTest();
     void calendarWidgetTest();
     void dockWidgetTest();
     void comboBoxTest();
@@ -3387,6 +3388,25 @@ void tst_QAccessibility::tableTest()
     QTestAccessibility::clearEvents();
 }
 
+void tst_QAccessibility::uniqueIdTest()
+{
+    // Test that an ID isn't reassigned to another interface right away when an accessible interface
+    // that has just been created is removed from the cache and deleted before the next
+    // accessible interface is registered.
+    // For example for AT-SPI, that would result in the same object path being used, and thus
+    // data from the old and new interface can get confused due to caching.
+    QWidget widget1;
+    QAccessibleInterface *iface1 = QAccessible::queryAccessibleInterface(&widget1);
+    QAccessible::Id id1 = QAccessible::uniqueId(iface1);
+    QAccessible::deleteAccessibleInterface(id1);
+
+    QWidget widget2;
+    QAccessibleInterface *iface2 = QAccessible::queryAccessibleInterface(&widget2);
+    QAccessible::Id id2 = QAccessible::uniqueId(iface2);
+
+    QVERIFY(id1 != id2);
+}
+
 void tst_QAccessibility::calendarWidgetTest()
 {
 #if QT_CONFIG(calendarwidget)