Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move to use Cswin32 IAccessible #10479

Merged
merged 20 commits into from
Dec 21, 2023
Merged

Move to use Cswin32 IAccessible #10479

merged 20 commits into from
Dec 21, 2023

Conversation

lonitra
Copy link
Member

@lonitra lonitra commented Dec 14, 2023

Related: #9795

  • Converts IAccessibleEx and ILegacyIAccessibleProvider to Cswin32 definition in dedicated commits
  • Introduce ComSafeArrayScopeExtensions which provides a way to generically get Safe array scope of corresponding COM pointers given an array of COM interfaces.
  • Move to use Cswin32 IAccessible:
    • Adds a few new internal virtual APIs to avoid excessive allocations by changing between managed and native types e.g. BSTR <-> string, IDispatch <-> AccessibleObject.
      CanXDirectly > Determines whether native type should be grabbed directly since it avoids new allocations
      GetXInternal() > Grabs the native type of interest
      The idea is that our internal objects should not need to move between native and managed types so much. They should use the native types all the way through if they can. Objects that overrides our public facing IAccessible properties/methods (e.g. Parent, GetSelected()`) need to be checked for whether they are subclassed, if they are not then this is another chance where we can avoid allocations by using the optimized path (native types).
  • Add IManagedWrapper to AccessibleObject
  • Fix tests, tests required more mock setup due to newly added internal methods.
Microsoft Reviewers: Open in CodeFlow

@ghost ghost assigned lonitra Dec 14, 2023
@ghost ghost added the draft draft PR label Dec 14, 2023
Copy link
Member

@JeremyKuhne JeremyKuhne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few initial comments, spoke offline.

{
RuntimeTypeHandle type = GetType().TypeHandle;
return type.Equals(typeof(AccessibleObject).TypeHandle)
|| type.GetModuleHandle().Equals(typeof(AccessibleObject).TypeHandle.GetModuleHandle());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One could also check the string name of the module against the known name of the design assembly if that helps.

@ghost ghost added waiting-author-feedback The team requires more information from the author and removed waiting-author-feedback The team requires more information from the author labels Dec 14, 2023
@lonitra lonitra requested a review from JeremyKuhne December 15, 2023 09:27
Copy link
Member

@JeremyKuhne JeremyKuhne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice! Added a few comments.

@ghost ghost added waiting-author-feedback The team requires more information from the author and removed waiting-author-feedback The team requires more information from the author labels Dec 18, 2023
@lonitra lonitra requested a review from JeremyKuhne December 19, 2023 20:55
@lonitra lonitra removed the draft draft PR label Dec 19, 2023
@lonitra lonitra marked this pull request as ready for review December 19, 2023 22:45
@lonitra lonitra requested a review from a team as a code owner December 19, 2023 22:45
Copy link
Member

@JeremyKuhne JeremyKuhne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work, @lonitra!

@lonitra lonitra merged commit cf6eea9 into dotnet:main Dec 21, 2023
9 checks passed
@lonitra lonitra deleted the iaccessible branch December 21, 2023 18:19
@ghost ghost added this to the 9.0 Preview1 milestone Dec 21, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jan 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants