Fixup of #13542: Do not call IA2 install and uninstall code from NVDA itself

[LeonarddeR]

This was reported by @codeofdusk as part of his work for the Py 3.11 upgrade.

Link to issue number:

Fixup of #13542

Summary of the issue:

In #13542, we introduced support for multi thread IA2 initialization. As part of that, the signature of the install- and uninstallIA2Support functions were changed. Somehow, it was an oversight from my end that these functions were also called from NVDAHelper.py. This causes:

1. NVDA to always log a debug warning on exit, saying that it was unable to unregister IA2 support.
2. NVDA to call installIA2Initialize without a thread Id. It is actually a miracle that the segfault we're now seeing in preliminary PY3.11 work didn't hurt us earlyer. That said, I think I've seen cases were NVDA played the startup sound and then silently crashed. I think this should be the case.

As per point 2, given the risk of NVDA calling a code path that results in undefined behavior and possible segfaults, I strongly recommend to merge this into beta.

Description of user facing changes

No crashes during NVDA startup.

Description of development approach

Removed the calls from NVDAHelper.py and no longer export the functions from NVDAHelper. The calls in NVDA core were actually obsolete, as IA2 support is registered by injection_initialize and injection_terminate. Note that the install and uninstall IA2 functions aren't called in de 64 bit loader of NVDAHelper, yet IA2 works as expected in X64 applications. Also, installIA2Support is no longer C friendly anyway. Note that an add-on author should never call these functions, therefore I don't consider this as an API breaking change.

Testing strategy:

• Tested that IA2 still works in X64 applications
• - [ ] Tested that IA2 still works in X86 applications

Known issues with pull request:

None known

Code Review Checklist:

• Documentation:
  • Change log entry
  • User Documentation
  • Developer / Technical Documentation
  • Context sensitive help for GUI changes
• Testing:
  • Unit tests
  • System (end to end) tests
  • Manual testing
• UX of all users considered:
  • Speech
  • Braille
  • Low Vision
  • Different web browsers
  • Localization in other languages / culture than English
• API is compatible with existing add-ons.
• Security precautions taken.

[michaelDCurran]

@LeonarddeR Can you further elaborate on your statement: "The calls in NVDA core were actually obsolete, as IA2 support is registered by injection_initialize and injection_terminate."?
Where exactly does this happen?
Certainly it (may) indirectly happen if NVDA's own GUI causes a focus / foreground event.
Note that it is very important for the AI2 proxy to be registered in NVDA's main thread so that it can handle the client side of the COM proxy.
In the cases where NvDA's GUI has not fired a focus / foreground and therefore it was not registered, IA2 support may work on recent builds of Windows 10 and higher, as Microsoft includes its own copy of IAccessible2. But for Windows 8.1 and older copies of Windows 10, or where our / the app's IAccessible2 is newer than Windows' version, then this may cause problems...
At very least, nvdaHelper.py's call to installIa2Support should be passed NVDA's main thread ID if this is reverted. And perhaps that error on terminate can be suppressed or changed to debugWarning.
And your comment re 64 bit: again the reason it probably worked was because Windows 10 and higher already have it registered.

[LeonarddeR]

Certainly it (may) indirectly happen if NVDA's own GUI causes a focus / foreground event.

This was indeed my assumption.

Note that it is very important for the AI2 proxy to be registered in NVDA's main thread so that it can handle the client side of the COM proxy.

A, I wasn't aware of that. Turns out we need to test that thoroughly.
That said, I think before this pr and after #13542, the COM proxy was never registered to NVDA's main thread. I'm not even sure what would happen if you'd call a function without parameters when the function requires at least one parameter.

What would be the best way to test this? Probably a Windows 8.1 VM or similar, then ensure an IA2 application is started and NVDA's gui doesn't receive events (i.e. don't open the NVDA menu)?

[michaelDCurran]

Yep, that testing sounds sufficient to me. I think you are right to remove the call from nvdaHelper.py as it can't be called from C. In fact, providing it with an argument causes a write error. Though I think that is more to do with space for the large return type in c++. But, if testing shows on 8.1 that IA2 is not functioning, then I think you'd need to call it from injection_initialize, or outprocThreadFunc, within c++. Or write a c-friendly wrapper for it and again call it from nvdaHelper.py.

[LeonarddeR]

I'm not sure whether testing this is worth the investment. I created #15517 to fix this, i.e. the state is now back to what it was before on the public side of NVDAHelper. Better safe than sorry.

[michaelDCurran]

Yep, looks good.