Get rid of RemoteAccessibleWrap on Windows
Categories
(Core :: Disability Access APIs, task)
Tracking
()
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.
Assignee | ||
Comment 1•3 years ago
|
||
Assignee | ||
Comment 2•3 years ago
|
||
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.
Assignee | ||
Comment 3•3 years ago
|
||
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.
Assignee | ||
Comment 4•3 years ago
|
||
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.
Assignee | ||
Comment 5•3 years ago
|
||
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.
Assignee | ||
Comment 6•3 years ago
|
||
Assignee | ||
Comment 7•3 years ago
|
||
Now that ProxyWrappers (RemoteAccessibleWrap and friends) are gone, IsProxy() can never be true, so checking it is pointless.
Comment 8•3 years ago
|
||
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.
Comment 9•3 years ago
|
||
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.
Comment 10•3 years ago
|
||
I am tentatively setting Fission Milestone to MVP because this bug blocks Windows crash bug 1715284, which is a blocker for Fission MVP.
Comment 11•3 years ago
|
||
Comment 12•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/675e63b1d318
https://hg.mozilla.org/mozilla-central/rev/30b193092e1e
https://hg.mozilla.org/mozilla-central/rev/e7acb70d7295
https://hg.mozilla.org/mozilla-central/rev/22c51b4e5a35
https://hg.mozilla.org/mozilla-central/rev/80e252f66347
https://hg.mozilla.org/mozilla-central/rev/54bd28570b2d
Description
•