1694865 - Separate Windows MSAA implementation into its own class hierarchy

Status: RESOLVED FIXED

(Core :: Disability Access APIs, task)

Description

[James Teh [:Jamie]]

Currently, MSAA and IA2 are implemented by inheritance into AccessibleWrap and friends. Although other OS do use the Wrap classes to provide some platform specific functionality, Windows is the only platform to implement all of the actual OS calls in the Wrap classes. While this has worked well enough until now, it becomes problematic in our aim to unify local and remote accessibles because we need both local and remote to use the MSAA/IA2 code.

If it were just AccessibleWrap, we could probably solve this by making AccessibleWrap a subclass of Accessible (instead of LocalAccessible). We'd then have LocalAccessible and RemoteAccessible subclass AccessibleWrap. However, this becomes problematic for HyperTextAccessible, TableAccessible, etc., since they will eventually be base interfaces supported by both local and remote.

Eventually, we also won't need the MSAA stuff in the content process. While having it there doesn't really do any harm, it's pointless and confusing.

So, we are going to create a separate class hierarchy: MsaaAccessible, MsaaDocAccessible, etc. These will only inherit the COM interfaces, not Gecko classes. There will be pointers to move between the two.

Comment 1

[James Teh [:Jamie]]

Attachment: Bug 1694865 part 1: Create new skeleton MsaaAccessible class.

This simply adds the new MsaaAccessible class, moves the inheritance of ia2Accessible, etc. to that class, and makes AccessibleWrap inherit from MsaaAccessible.

Comment 2

[James Teh [:Jamie]]

Attachment: Bug 1694865 part 2: Add missing includes and forward declarations to fix build errors when unified build order is changed in subsequent patches.

Comment 3

[James Teh [:Jamie]]

Attachment: Bug 1694865 part 3: Move id management to MsaaAccessible.

Comment 4

[James Teh [:Jamie]]

Attachment: Bug 1694865 part 4: Move foundational helper functions for retrieving descendant accessibles to MsaaAccessible.

Comment 5

[James Teh [:Jamie]]

Attachment: Bug 1694865 part 5: Move shutdown code into MsaaAccessible.

Comment 6

[James Teh [:Jamie]]

Attachment: Bug 1694865 part 6: Move event firing code to MsaaAccessible.

Comment 7

[James Teh [:Jamie]]

Attachment: Bug 1694865 part 7: Move all IAccessible and IDispatch methods to MsaaAccessible.

Comment 8

[James Teh [:Jamie]]

Attachment: Bug 1694865 part 8: Remove some dead code from AccessibleWrap.

Comment 9

[James Teh [:Jamie]]

Attachment: Bug 1694865 part 9: Make MsaaAccessible::LocalAcc return null if defunct.

Once MsaaAccessible is no longer inherited into AccessibleWrap, we'll null the AccessibleWrap pointer when the AccessibleWrap shuts down.
At that point, MsaaAccessible won't deal with defunct AccessibleWraps.
Prepare for this by removing defunct special cases from most callers.

Comment 10

[James Teh [:Jamie]]

Attachment: Bug 1694865 part 10: Prepare ia2Accessible/Action/Component/Hyperlink/Relation/Value to not rely on AccessibleWrap inheritance.

Rather than static_casting this, there is now a LocalAcc() method which in turn calls MsaaAccessible::LocalAcc().
Since MsaaAccessible::LocalAcc() returns null if defunct, defunct checks have been adjusted accordingly.
For LocalAccessibles other than this, rather than static_casting, GetNativeInterface is now used.
Finally, ia2AccessibleValue::QueryInterface was adjusted to AddRef the queried interface on this, not the AccessibleWrap.

Comment 11

[James Teh [:Jamie]]

Attachment: Bug 1694865 part 18: Prepare ia2AccessibleEditableText/Hypertext/Image/Text to not rely on AccessibleWrap inheritance.

Rather than static_casting this, there is now an ImageAcc/TextAcc() method which in turn calls MsaaAccessible::LocalAcc().
Since MsaaAccessible::LocalAcc() returns null if defunct, defunct checks have been adjusted accordingly.
For LocalAccessibles other than this, rather than static_casting, GetNativeInterface is now used.
ia2AccessibleText was calling AccessibleWrap::ConvertToIA2Attributes, but this is actually declared in ia2Accessible, so we just add the necessary include and change the call.

Comment 12

[James Teh [:Jamie]]

Attachment: Bug 1694865 part 11: Prepare ia2AccessibleTable/TableCell to not rely on AccessibleWrap inheritance.

For now, we can't use MsaaAccessible::LocalAcc() because these are inherited into multiple Wrap classes, but we don't know which at runtime.
However, we can still prepare by creating a Table/CellAcc() method which can be easily replaced once we kill the Wrap inheritance.
For LocalAccessibles other than this, rather than static_casting, GetNativeInterface is now used.

Comment 13

[James Teh [:Jamie]]

Attachment: Bug 1694865 part 12: Move COM IAccessibleApplication implementation to new ia2AccessibleApplication class.

For now, ApplicationAccessibleWrap inherits from the new class.
ApplicationAccessible calls are made via an AppAcc() method which in turn calls MsaaAccessible::LocalAcc().
Since MsaaAccessible::LocalAcc() returns null if defunct, defunct checks have been adjusted accordingly.

Comment 14

[James Teh [:Jamie]]

Attachment: Bug 1694865 part 13: Move DocAccessibleWrap's MSAA overrides to a new MsaaDocAccessible class.

For now, DocAccessibleWrap inherits from MsaaDocAccessible and MsaaDocAccessible inherits from DocAccessible.
DocAccessible calls are made via a DocAcc() method which in turn calls MsaaAccessible::LocalAcc().
Since MsaaAccessible::LocalAcc() returns null if defunct, defunct checks have been adjusted accordingly.

Comment 15

[James Teh [:Jamie]]

Attachment: Bug 1694865 part 14: Move RootAccessibleWrap's MSAA overrides and other COM stuff to a new MsaaRootAccessible class.

For now, RootAccessibleWrap inherits from MsaaRootAccessible and MsaaRootAccessible inherits from RootAccessible.
RootAccessible calls are made via a RootAcc() method which in turn calls MsaaAccessible::LocalAcc().
Since MsaaAccessible::LocalAcc() returns null if defunct, defunct checks have been adjusted accordingly.

Comment 16

[James Teh [:Jamie]]

Attachment: Bug 1694865 part 15: Move XULMenuitemAccessibleWrap's MSAA overrides to a new MsaaXULMenuitemAccessible class.

For now, XULMenuitemAccessibleWrap inherits from MsaaXULMenuitemAccessible and MsaaXULMenuitemAccessible inherits from XULMenuitemAccessible.
Accessible calls are made via MsaaAccessible::LocalAcc().
Since MsaaAccessible::LocalAcc() returns null if defunct, defunct checks have been adjusted accordingly.

Comment 17

[James Teh [:Jamie]]

Attachment: Bug 1694865 part 16: Transitional implementation of IUnknown in MsaaAccessible/MsaaDocAccessible which delegates to AccessibleWrap.

This is necessary to enable COM objects which aggregate AccessibleWrap/DocAccessibleWrap to aggregate MsaaAccessible/MsaaDocAccessible instead.
Ultimately, the IUnknown implementation will be moved out of AccessibleWrap altogether and into MsaaAccessible, but we can't do that yet.
Even though MsaaDocAccessible indirectly inherits from MsaaAccessible, we have to override QueryInterface due to the naming conflict with nsISupports.

Comment 18

[James Teh [:Jamie]]

Attachment: Bug 1694865 part 19: Make EnumVariant, GeckoCustom and sdn*Accessible classes aggregate MsaaAccessible/MsaaDocAccessible instead of AccessibleWrap/DocAccessibleWrap.

These objects need to aggregate the primary COM object (MsaaAccessible).
Once the IUnknown implementation moves out of AccessibleWrap, it won't be possible to aggregate *AccessibleWrap any more.

Comment 19

[James Teh [:Jamie]]

Attachment: Bug 1694865 part 20: Change ServiceProvider to not rely on AccessibleWrap inheritance.

It now aggregates MsaaAccessible and uses GetNativeInterface when returning accessibles to clients.

Comment 20

[James Teh [:Jamie]]

Attachment: Bug 1694865 part 21: Move QueryInterface implementation from AccessibleWrap to MsaaAccessible.

Comment 21

[James Teh [:Jamie]]

Attachment: Bug 1694865 part 17: Add various functions to get an MsaaAccessible/MsaaDocAccessible and use them where appropriate.

In this patch, these new functions just static_cast, but they'll soon be returning a different object.

Comment 22

[James Teh [:Jamie]]

Attachment: Bug 1694865 part 22: Change LazyInstantiator to aggregate MsaaRootAccessible instead of RootAccessibleWrap.

Comment 23

[James Teh [:Jamie]]

Attachment: Bug 1694865 part 23: Support inheritance/overriding of reference counting in IUnknownImpl.

Until now, our a11y COM implementation has relied on AddRef and Release from nsISupports, but this soon won't be possible.
Instead, MsaaAccessible will implement its own reference counting using DECL_IUNKNOWN.

DECL_IUNKNOWN previously declared AddRef and Release as final.
This doesn't work for MsaaAccessible because there is an aggregatable subclass (MsaaRootAccessible) and because some subclasses will inherit additional interfaces (ia2AccessibleApplication, etc.).

When subclasses inherit additional interfaces, they inherit an additional IUnknown, so the compiler doesn't know which AddRef/Release to call.
To support this, IMPL_IUNKNOWN_REFCOUNTING_INHERITED HAS BEEN ADDED to specify which base class implements reference counting.

Comment 24

[James Teh [:Jamie]]

Attachment: Bug 1694865 part 24: Change uiaRawElmProvider to not rely on AccessibleWrap inheritance.

It now uses GetNativeInterface when returning IAccessibles to clients.

Comment 25

[James Teh [:Jamie]]

Attachment: Bug 1694865 part 25: Remove inheritance of Msaa*Accessible/ia2*Accessible into *AccessibleWrap!

1. ia2AccessibleApplication is instantiated for ApplicationAccessible, so it now inherits from MsaaAccessible.
2. ia2AccessibleHypertext is instantiated for HyperTextAccessible, so it now inherits from MsaaAccessible.
3. ia2AccessibleImage is instantiated for ImageAccessible, so it inherits from MsaaAccessible.
4. ia2AccessibleTable is instantiated for TableAccessible, so it inherits from ia2AccessibleHypertext (since most TableAccessible implementations implement HyperTextAccessible).
5. ia2AccessibleTableCell is instantiated for TableCellAccessible, so it inherits from ia2AccessibleHypertext (since most TableCellAccessible implementations implement HyperTextAccessible).
6. All of the above override QueryInterface as appropriate, replacing the QueryInterface implementations from all *AccessibleWrap classes.
7. The ARIAGridAccessibleWrap, HTMLTableAccessibleWrap, ImageAccessibleWrap, XULListboxAccessibleWrap and XULTreeGridAccessibleWrap classes previously served only to host ia2AccessibleImage, ia2AccessibleTable, etc. Since these ia2 classes are now instantiated via MsaaAccessible, these Wrap classes have been removed and replaced with aliases.
8. The QueryInterface handling for ISimpleDOMText has been moved into MsaaAccessible. Since this was the only purpose of TextLeafAccessibleWrap, this too has been removed and replaced with an alias.
9. AccessibleWrap now holds a strong reference to MsaaAccessible and MsaaAccessible holds a weak reference to AccessibleWrap.
10. An MsaaAccessible (or derived class) is instantiated by MsaaAccessible::Create.
11. MsaaAccessible now implements its own COM reference counting using DECL_IUNKNOWN, since it does not need nsISupports (XPCOM).

Comment 26

[James Teh [:Jamie]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1994c31ac5aa8ae9a8063e066c34e385f90a5356

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9d27bb4a4ab4
part 1: Create new skeleton MsaaAccessible class. r=morgan
https://hg.mozilla.org/integration/autoland/rev/f283f3cf03cf
part 2: Add missing includes and forward declarations to fix build errors when unified build order is changed in subsequent patches. r=morgan
https://hg.mozilla.org/integration/autoland/rev/ba238a1e12df
part 3: Move id management to MsaaAccessible. r=morgan
https://hg.mozilla.org/integration/autoland/rev/5dcd83a1c82c
part 4: Move foundational helper functions for retrieving descendant accessibles to MsaaAccessible. r=morgan
https://hg.mozilla.org/integration/autoland/rev/86c1ddcc8e31
part 5: Move shutdown code into MsaaAccessible. r=morgan
https://hg.mozilla.org/integration/autoland/rev/7c345ab7e3be
part 6: Move event firing code to MsaaAccessible. r=morgan

Comment 28

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/9d27bb4a4ab4
https://hg.mozilla.org/mozilla-central/rev/f283f3cf03cf
https://hg.mozilla.org/mozilla-central/rev/ba238a1e12df
https://hg.mozilla.org/mozilla-central/rev/5dcd83a1c82c
https://hg.mozilla.org/mozilla-central/rev/86c1ddcc8e31
https://hg.mozilla.org/mozilla-central/rev/7c345ab7e3be

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7dfb88fc34a1
part 7: Move all IAccessible and IDispatch methods to MsaaAccessible. r=morgan
https://hg.mozilla.org/integration/autoland/rev/40c7e0bcfcfd
part 8: Remove some dead code from AccessibleWrap. r=morgan
https://hg.mozilla.org/integration/autoland/rev/acafc5835608
part 9: Make MsaaAccessible::LocalAcc return null if defunct. r=morgan
https://hg.mozilla.org/integration/autoland/rev/461b46e1a2cb
part 10: Prepare ia2Accessible/Action/Component/Hyperlink/Relation/Value to not rely on AccessibleWrap inheritance. r=morgan
https://hg.mozilla.org/integration/autoland/rev/309f737d3bc3
part 11: Prepare ia2AccessibleEditableText/Hypertext/Image/Text to not rely on AccessibleWrap inheritance. r=morgan
https://hg.mozilla.org/integration/autoland/rev/53ed762843ec
part 12: Prepare ia2AccessibleTable/TableCell to not rely on AccessibleWrap inheritance. r=morgan

Comment 30

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/7dfb88fc34a1
https://hg.mozilla.org/mozilla-central/rev/40c7e0bcfcfd
https://hg.mozilla.org/mozilla-central/rev/acafc5835608
https://hg.mozilla.org/mozilla-central/rev/461b46e1a2cb
https://hg.mozilla.org/mozilla-central/rev/309f737d3bc3
https://hg.mozilla.org/mozilla-central/rev/53ed762843ec

Comment 31

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/62137fbc81cf
part 13: Move COM IAccessibleApplication implementation to new ia2AccessibleApplication class. r=morgan
https://hg.mozilla.org/integration/autoland/rev/61aad68338f7
part 14: Move DocAccessibleWrap's MSAA overrides to a new MsaaDocAccessible class. r=morgan
https://hg.mozilla.org/integration/autoland/rev/70b71a350151
part 15: Move RootAccessibleWrap's MSAA overrides and other COM stuff to a new MsaaRootAccessible class. r=morgan
https://hg.mozilla.org/integration/autoland/rev/702b601cc6fc
part 16: Move XULMenuitemAccessibleWrap's MSAA overrides to a new MsaaXULMenuitemAccessible class. r=morgan
https://hg.mozilla.org/integration/autoland/rev/3ea7b46bb4f8
part 17: Transitional implementation of IUnknown in MsaaAccessible/MsaaDocAccessible which delegates to AccessibleWrap. r=morgan
https://hg.mozilla.org/integration/autoland/rev/1a948c0ec982
part 18: Make EnumVariant, GeckoCustom and sdn*Accessible classes aggregate MsaaAccessible/MsaaDocAccessible instead of AccessibleWrap/DocAccessibleWrap. r=morgan

Comment 32

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out changes 7-12 for accessibility crashes (part 13-18 will be backed in a few minutes together with the merge of the backout of 7-12 to central):

https://hg.mozilla.org/mozilla-central/rev/d9e30da70def144c22f40e7debc5b190a31eb1f1

Crashes:
https://crash-stats.mozilla.org/signature/?build_id=%3E20210429000000&product=Firefox&version=90.0a1&date=%3E%3D2021-04-26T00%3A00%3A00.000Z&date=%3C2021-04-29T23%3A59%3A00.000Z&_sort=-date&signature=mozilla%3A%3Aa11y%3A%3AMsaaAccessible%3A%3Aget_accValue
https://crash-stats.mozilla.org/signature/?build_id=%3E20210429000000&product=Firefox&version=90.0a1&signature=mozilla%3A%3Aa11y%3A%3AMsaaAccessible%3A%3Aget_accDescription&date=%3E%3D2021-04-26T00%3A00%3A00.000Z&date=%3C2021-04-29T23%3A59%3A00.000Z&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&_columns=install_time&_columns=startup_crash&_sort=-date&page=1#summary

Comment 33

[Iulian Moraru]

Backed out changes 13-18 for causing merge conflict.

https://hg.mozilla.org/integration/autoland/rev/2a4d3f13161de6a01d6c29b7c92658d74748ff67

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/de1f048e7f59
part 7: Move all IAccessible and IDispatch methods to MsaaAccessible. r=morgan
https://hg.mozilla.org/integration/autoland/rev/d88639c490d2
part 8: Remove some dead code from AccessibleWrap. r=morgan
https://hg.mozilla.org/integration/autoland/rev/16e294b866a6
part 9: Make MsaaAccessible::LocalAcc return null if defunct. r=morgan
https://hg.mozilla.org/integration/autoland/rev/d215650648ea
part 10: Prepare ia2Accessible/Action/Component/Hyperlink/Relation/Value to not rely on AccessibleWrap inheritance. r=morgan
https://hg.mozilla.org/integration/autoland/rev/eaee7f4386eb
part 11: Prepare ia2AccessibleTable/TableCell to not rely on AccessibleWrap inheritance. r=morgan
https://hg.mozilla.org/integration/autoland/rev/5a74388d847a
part 12: Move COM IAccessibleApplication implementation to new ia2AccessibleApplication class. r=morgan

Comment 35

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/de1f048e7f59
https://hg.mozilla.org/mozilla-central/rev/d88639c490d2
https://hg.mozilla.org/mozilla-central/rev/16e294b866a6
https://hg.mozilla.org/mozilla-central/rev/d215650648ea
https://hg.mozilla.org/mozilla-central/rev/eaee7f4386eb
https://hg.mozilla.org/mozilla-central/rev/5a74388d847a

Comment 36

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eb4b8e688202
part 13: Move DocAccessibleWrap's MSAA overrides to a new MsaaDocAccessible class. r=morgan
https://hg.mozilla.org/integration/autoland/rev/f967d16a75ae
part 14: Move RootAccessibleWrap's MSAA overrides and other COM stuff to a new MsaaRootAccessible class. r=morgan
https://hg.mozilla.org/integration/autoland/rev/bec6aca2ad6a
part 15: Move XULMenuitemAccessibleWrap's MSAA overrides to a new MsaaXULMenuitemAccessible class. r=morgan
https://hg.mozilla.org/integration/autoland/rev/262aaf0142f7
part 16: Transitional implementation of IUnknown in MsaaAccessible/MsaaDocAccessible which delegates to AccessibleWrap. r=morgan
https://hg.mozilla.org/integration/autoland/rev/ae2dd5aeb7bf
part 17: Add various functions to get an MsaaAccessible/MsaaDocAccessible and use them where appropriate. r=morgan
https://hg.mozilla.org/integration/autoland/rev/0d57c4867cea
part 18: Prepare ia2AccessibleEditableText/Hypertext/Image/Text to not rely on AccessibleWrap inheritance. r=morgan

Comment 37

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/eb4b8e688202
https://hg.mozilla.org/mozilla-central/rev/f967d16a75ae
https://hg.mozilla.org/mozilla-central/rev/bec6aca2ad6a
https://hg.mozilla.org/mozilla-central/rev/262aaf0142f7
https://hg.mozilla.org/mozilla-central/rev/ae2dd5aeb7bf
https://hg.mozilla.org/mozilla-central/rev/0d57c4867cea

Comment 38

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a53fb045a8e3
part 19: Make EnumVariant, GeckoCustom and sdn*Accessible classes aggregate MsaaAccessible/MsaaDocAccessible instead of AccessibleWrap/DocAccessibleWrap. r=morgan
https://hg.mozilla.org/integration/autoland/rev/63f10db368c8
part 20: Change ServiceProvider to not rely on AccessibleWrap inheritance. r=morgan
https://hg.mozilla.org/integration/autoland/rev/81587e0240ca
part 21: Move QueryInterface implementation from AccessibleWrap to MsaaAccessible. r=morgan
https://hg.mozilla.org/integration/autoland/rev/40805a1f2c1e
part 22: Change LazyInstantiator to aggregate MsaaRootAccessible instead of RootAccessibleWrap. r=morgan
https://hg.mozilla.org/integration/autoland/rev/893c83f82c16
part 23: Support inheritance/overriding of reference counting in IUnknownImpl. r=morgan
https://hg.mozilla.org/integration/autoland/rev/f478ad97306a
part 24: Change uiaRawElmProvider to not rely on AccessibleWrap inheritance. r=morgan

Comment 39

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/a53fb045a8e3
https://hg.mozilla.org/mozilla-central/rev/63f10db368c8
https://hg.mozilla.org/mozilla-central/rev/81587e0240ca
https://hg.mozilla.org/mozilla-central/rev/40805a1f2c1e
https://hg.mozilla.org/mozilla-central/rev/893c83f82c16
https://hg.mozilla.org/mozilla-central/rev/f478ad97306a

Comment 40

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4173ffb5fcb9
part 25: Remove inheritance of Msaa*Accessible/ia2*Accessible into *AccessibleWrap! r=morgan

Comment 41

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/4173ffb5fcb9