Closed Bug 1715230 Opened 3 years ago Closed 3 years ago

Get rid of RemoteAccessibleWrap on Windows

Categories

(Core :: Disability Access APIs, task)

Desktop
Windows
task

Tracking

()

RESOLVED FIXED
92 Branch
Fission Milestone MVP
Tracking Status
firefox92 --- fixed

People

(Reporter: Jamie, Assigned: Jamie)

References

(Blocks 1 open bug)

Details

Attachments

(6 files)

RemoteAccessibleWrap is a horrible hack which wraps a RemoteAccessible in a stub AccessibleWrap. This was always confusing, but it's even worse now that we have a unified Accessible base class, since RemoteAccessibleWraps return true for IsLocal(), true for IsProxy() but false for IsRemote(). This hack was necessary when our MSAA and IA2 implementation was tied to AccessibleWrap. Now that this has been separated into MsaaAccessible and friends (bug 1694865) and that MsaaAccessible is growing support for RemoteAccessible (bug 1695116), this is no longer necessary. Beyond that, after the creation of MsaaAccessible, this actually wastes quite a bit of memory, since every RemoteAccessible gets both a RemoteAccessibleWrap and an MsaaAccessible, plus the bidirectional pointers between those.

It still can't handle being called on a RemoteAccessible because Bounds isn't unified yet.
However, this allows it to be called on a local OuterDoc (a XUL browser element) and return a remote document child.
Previously, this was handled by the RemoteAccessibleWrap returned from OuterDocAccessible::LocalChildAt, but that will go away in a subsequent patch.

This was previously used to wrap the COM proxy for the document child of an OOP iframe in a content process.
It was returned by OuterDocAccessible::LocalChildAt*.
Instead, BrowserBridgeChild now directly holds the COM proxy and MsaaAccessible traversal methods have specific code paths to return that as appropriate.

Since we no longer need the Windows OuterDocAccessible traversal overrides for OOP iframes nor remote top level documents, they have been removed entirely.

In a subsequent patch, we want to call it with a RemoteAccessible instead of a RemoteAccessibleWrap.
As an instance method of AccessibleWrap, this wouldn't be possible because AccessibleWrap is a subclass of LocalAccessible.
Strictly speaking, there's no good reason for this to be part of AccessibleWrap at all any more, but I'm not dealing with that here.

Previously, when the cache was disabled, we had a RemoteAccessibleWrap for every RemoteAccessible.
This is no longer necessary and now only serves as an extra level of indirection and memory waste.
We still keep the stub MsaaAccessible to hold the id sent up from content.

Now that ProxyWrappers (RemoteAccessibleWrap and friends) are gone, IsProxy() can never be true, so checking it is pointless.

Blocks: 1715284

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:Jamie, could you have a look please?
For more information, please visit auto_nag documentation.

Flags: needinfo?(mreschenberg)
Flags: needinfo?(jteh)

Jamie is on PTO bot friend :)
I'm not sure if there's more work to do on these, so I don't want to land them outright.

Flags: needinfo?(mreschenberg)
Flags: needinfo?(jteh)

I am tentatively setting Fission Milestone to MVP because this bug blocks Windows crash bug 1715284, which is a blocker for Fission MVP.

Fission Milestone: --- → MVP
Pushed by jteh@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/675e63b1d318 part 1: Make IAccessible::accHitTest use the unified Accessible hierarchy. r=morgan https://hg.mozilla.org/integration/autoland/rev/30b193092e1e part 2: Stop using RemoteIframeDocRemoteAccessibleWrap. r=morgan https://hg.mozilla.org/integration/autoland/rev/e7acb70d7295 part 3: Change AccessibleWrap::DispatchTextChangeToHandler to a static method accepting an Accessible*. r=morgan https://hg.mozilla.org/integration/autoland/rev/22c51b4e5a35 part 4: Stop using RemoteAccessibleWrap! r=morgan https://hg.mozilla.org/integration/autoland/rev/80e252f66347 part 5: Remove Windows ProxyWrappers! r=morgan https://hg.mozilla.org/integration/autoland/rev/54bd28570b2d part 6: Remove IsProxy() checks throughout the Windows a11y code. r=morgan
Regressions: 1725449
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: