Bug 209576 - [WebAccessibilityObjectWrapper remoteAccessibilityParentObject should be cached to avoid hitting the main thread often.
Summary: [WebAccessibilityObjectWrapper remoteAccessibilityParentObject should be cach...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andres Gonzalez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-03-25 18:57 PDT by Andres Gonzalez
Modified: 2020-03-30 13:08 PDT (History)
9 users (show)

See Also:


Attachments
Patch (7.57 KB, patch)
2020-03-25 19:09 PDT, Andres Gonzalez
no flags Details | Formatted Diff | Diff
Patch (16.36 KB, patch)
2020-03-28 15:28 PDT, Andres Gonzalez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andres Gonzalez 2020-03-25 18:57:15 PDT
[WebAccessibilityObjectWrapper remoteAccessibilityParentObject should be cached to avoid hitting the main thread often.
Comment 1 Andres Gonzalez 2020-03-25 19:09:21 PDT
Created attachment 394568 [details]
Patch
Comment 2 chris fleizach 2020-03-25 19:58:53 PDT
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?
Comment 3 Andres Gonzalez 2020-03-26 20:00:38 PDT
(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 4 chris fleizach 2020-03-27 00:56:50 PDT
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
Comment 5 Andres Gonzalez 2020-03-28 15:28:40 PDT
Created attachment 394844 [details]
Patch
Comment 6 Andres Gonzalez 2020-03-28 15:33:02 PDT
(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.
Comment 7 EWS 2020-03-30 13:07:17 PDT
Committed r259220: <https://trac.webkit.org/changeset/259220>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 394844 [details].
Comment 8 Radar WebKit Bug Importer 2020-03-30 13:08:15 PDT
<rdar://problem/61069499>