Bug 116112 – AX: Use caching when requesting children object on iOS

• Component: Accessibility
• Status: RESOLVED FIXED

[chris fleizach]

To speed up performance on iOS we should use the attribute caching that was put in

[chris fleizach]

Created attachment 201736 [details]
patch

[David Kilzer (:ddkilzer)]

Comment on attachment 201736 [details]
patch 

View in context: https://bugs.webkit.org/attachment.cgi?id=201736&action=review

r=me, but please consider using the stack-allocated C++ object to start/stop the cache.

> Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:307
> +    [self startCachingAttributes];
>      RefPtr<AccessibilityObject> axObject = m_object->accessibilityHitTest(IntPoint(point));
> +    [self stopCachingAttributes];

Can we use the stack-allocated C++ object pattern to turn the caching on and off (rather than the Objective-C start/stop calls)?

Basically you just declare the object on the stack (usually in its own block to isolate the code that uses the cache).  The constructor would call the -startCachingAttributes method and the destructor would call the -stopCachingAttributes method.

This also makes it easier to change the implementation later without changing all the call sites where the cache is used.  And if the cache is slightly different on iOS and Mac, that could be abstracted into this locking object so that (maybe) more code could be shared.

[Darin Adler]

Comment on attachment 201736 [details]
patch 

View in context: https://bugs.webkit.org/attachment.cgi?id=201736&action=review

> Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:1095
> +    [self startCachingAttributes];
>      
>      // As long as there's a parent wrapper, that's the correct chain to climb.
>      AccessibilityObject* parent = m_object->parentObjectUnignored(); 
>      if (parent)
>          return parent->wrapper();
> +    
> +    [self stopCachingAttributes];

If parent is 0 looks like we’ll call start but not stop. Isn’t that a problem?

[Darin Adler]

Comment on attachment 201736 [details]
patch 

View in context: https://bugs.webkit.org/attachment.cgi?id=201736&action=review

>> Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:1095
>> +    [self startCachingAttributes];
>>      
>>      // As long as there's a parent wrapper, that's the correct chain to climb.
>>      AccessibilityObject* parent = m_object->parentObjectUnignored(); 
>>      if (parent)
>>          return parent->wrapper();
>> +    
>> +    [self stopCachingAttributes];
> 
> If parent is 0 looks like we’ll call start but not stop. Isn’t that a problem?

If parent is non-zero, I mean.

[chris fleizach]

(In reply to comment #4)
> (From update of attachment 201736 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=201736&action=review
> 
> >> Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:1095
> >> +    [self startCachingAttributes];
> >>      
> >>      // As long as there's a parent wrapper, that's the correct chain to climb.
> >>      AccessibilityObject* parent = m_object->parentObjectUnignored(); 
> >>      if (parent)
> >>          return parent->wrapper();
> >> +    
> >> +    [self stopCachingAttributes];
> > 
> > If parent is 0 looks like we’ll call start but not stop. Isn’t that a problem?
> 
> If parent is non-zero, I mean.

Yep good catch. Switching to David's idea of a C++ stack-allocated object will fix this issue. Thanks

[chris fleizach]

http://trac.webkit.org/changeset/150145