Bug 27956 – AX: roleValue should be cached for performance

• Component: Accessibility
• Status: RESOLVED FIXED

[chris fleizach]

a lot of calls to roleValue can be made in the course of an AX object's lifetime. i think it would be beneficial to cache that value

[chris fleizach]

my initial testing shows a 5.1% speedup by caching the role value

[chris fleizach]

testing was done by navigating a web page and totaling the time spent in copyAttributeValue. Of course roleValue is called in other places, so this speedup will be felt in other places as well

[chris fleizach]

Created attachment 34009 [details]
patch

[Mark Rowe (bdash)]

Comment on attachment 34009 [details]
patch

> Index: WebCore/accessibility/AccessibilityImageMapLink.cpp
> ===================================================================
> --- WebCore/accessibility/AccessibilityImageMapLink.cpp	(revision 46732)
> +++ WebCore/accessibility/AccessibilityImageMapLink.cpp	(working copy)
> @@ -43,9 +43,10 @@ namespace WebCore {
>  using namespace HTMLNames;
>  
>  AccessibilityImageMapLink::AccessibilityImageMapLink()
> -    : m_areaElement(0), 
> -      m_mapElement(0)
> +    : m_areaElement(0)
> +    , m_mapElement(0)
>  {
> +    m_role = WebCoreLinkRole;
>  }

Initialization of this form should happen in the initialization list rather than via assignment in the constructor body.

[chris fleizach]

m_role is a protected variable in AccessibilityObject. When i tried to initialize it in the constructor list, it wouldn't compile

[chris fleizach]

specifically
/Volumes/data/WebKit/WebCore/accessibility/AccessibilityImageMapLink.cpp:48: error: class ‘WebCore::AccessibilityImageMapLink’ does not have any field named ‘m_role’

[chris fleizach]

Comment on attachment 34009 [details]
patch

obsoleting this patch

[chris fleizach]

Created attachment 34018 [details]
patch

[Eric Seidel (no email)]

Comment on attachment 34018 [details]
patch

setRoleValue would normally take an argument.  Maybe initializeRoleValues() or determineRoleValues()?   Do determineRoleValue() and determineAriaRole() need to be separate, or can they just be both part of a determinRoleValue()?  Is this something we could just do on first access to roleValue()?  or do we need to call this explicitly?

[chris fleizach]

i think we can re-word some of these methods

[chris fleizach]

unfortunately, we have to determine the role at initialization because too many methods rely on roleValue() to be const (so we can't set the role in roleValue() the first time someone asks for it)

[chris fleizach]

Created attachment 34073 [details]
patch

[chris fleizach]

changed set*Role to updateAccessibilityRole()

[Eric Seidel (no email)]

Comment on attachment 34073 [details]
patch

Looks OK.

[chris fleizach]

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