| Summary: | [WebAccessibilityObjectWrapper remoteAccessibilityParentObject should be cached to avoid hitting the main thread often. | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Andres Gonzalez <andresg_22> | ||||||
| Component: | New Bugs | Assignee: | Andres Gonzalez <andresg_22> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | aboxhall, apinheiro, cfleizach, dmazzoni, ews-watchlist, jcraig, jdiggs, samuel_white, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Andres Gonzalez
2020-03-25 18:57:15 PDT
Created attachment 394568 [details]
Patch
Comment on attachment 394568 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=394568&action=review > Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h:906 > + RetainPtr<RemoteAXObjectRef> m_remoteParentObject; should this be a WeakPtr so it goes away on its own? (In reply to chris fleizach from comment #2) > Comment on attachment 394568 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=394568&action=review > > > Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h:906 > > + RetainPtr<RemoteAXObjectRef> m_remoteParentObject; > > should this be a WeakPtr so it goes away on its own? RemoteAXObjectRef is an id. WeakPtr does not work with id which is the same as void *. I believe the only way to hold a reference to this id is with RetainPtr. I don't know however if there is another way to do the equivalent to @property (weak) id. Comment on attachment 394568 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=394568&action=review > Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:381 > + m_remoteParentObject = object.remoteParentObject(); I think we only want to do this for the top level scroll view... see logic in scrollViewParent Might be good to store it in the attribute hash so that we don't have to burn an ivar memory for every isolated object, when only 1 of them needs it Created attachment 394844 [details]
Patch
(In reply to chris fleizach from comment #4) > Comment on attachment 394568 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=394568&action=review > > > Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:381 > > + m_remoteParentObject = object.remoteParentObject(); > > I think we only want to do this for the top level scroll view... see logic in > > scrollViewParent > > Might be good to store it in the attribute hash so that we don't have to > burn an ivar memory for every isolated object, when only 1 of them needs it Cached it in the variant along with all other properties. Also moved the PlatformWidget cached value to the Variant as well. Cache it only for scroll views, however this property can be requested for all objects, so had to return it for all objects as you see in AXIsolatedObjectMac.mm. Committed r259220: <https://trac.webkit.org/changeset/259220> All reviewed patches have been landed. Closing bug and clearing flags on attachment 394844 [details]. |