ia2utils HyperlinkGetter: Switch to CComPtr #9152
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…o C++ exceptions don't get thrown for COM errors. Newer code uses CComPtr already, so it made sense to switch rather than catching the exceptions.
michaelDCurran
approved these changes
Jan 14, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Link to issue number:
Addresses part of #9133.
Summary of the issue:
When rendering a document for browse mode in Firefox, failure in retrieving an embedded object causes an uncaught exception, thus crashing Firefox. See this Firefox crash report.
Description of how this pull request fixes the issue:
HyperlinkGetter (in nvdaHelper/common/ia2utils) uses the
_com_ptr_tsmart pointer for COM objects._com_ptr_tthrows C++ exceptions for COM errors (other than E_NOINTERFACE) when doing a QueryInterface. Normally, there should not be any COM errors when doing a QI to IAccessible2 on an IAccessibleHyperlink for an embedded object in Firefox. However, it has happened (it's the cause of the above crash) and it is theoretically possible; e.g. if the object died after being retrieved but before QI, since a11y calls do not block the content main thread in Firefox.We had the choice to switch to CComPtr (which guarantees not to throw exceptions) or catch the exceptions. Since we're moving to CComPtr in newer code anyway, I went with the former.
Testing performed:
Tested rendering World War I in Firefox and compared the load times to check they were in the same ballpark. I can't test for the crash, since I can't reproduce it and it is very rare.
Known issues with pull request:
None known.
Change log entry:
Bug fixes:
- Fixed a rare browse mode crash in Firefox.