Bug 256526

<rdar://problem/109100219>

Created attachment 466293 [details]
Patch

Comment on attachment 466293 [details]
Patch

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

> COMMIT_MESSAGE:7
> +To accomplish not higging the main thread upon primary scrren height requests, the following changes were necessary:

higging -> hitting
scrren -> screen

> Source/WebCore/accessibility/mac/AccessibilityObjectMac.mm:84
> +    return screenRectForPrimaryScreen();

will this be expensive to call on initialization?

Comment on attachment 466293 [details]
Patch

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

> COMMIT_MESSAGE:7
> +To accomplish not higging the main thread upon primary scrren height requests, the following changes were necessary:

typo: higging, hitting

> Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:308
> +    if (object.isScrollView() || object.isWebArea()) {
> +        // For the ScrollView and WebArea objects, cache the TitleAttributeValue and DescriptionAttributeValue eagerly to avoid hitting the main thread in getOrRetrievePropertyValue during the construction of the isolated tree.
> +        setProperty(AXPropertyName::TitleAttributeValue, object.titleAttributeValue().isolatedCopy());
> +        setProperty(AXPropertyName::DescriptionAttributeValue, object.descriptionAttributeValue().isolatedCopy());

I believe one or both of these results in calls to textUnderElement, right? Since the point of this patch to facilitate fast startup, is it possible that these functions will recursively call textUnderElement all the way to the leaves?

Created attachment 466298 [details]
Patch

(In reply to chris fleizach from comment #3)
> Comment on attachment 466293 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=466293&action=review
> 
> > COMMIT_MESSAGE:7
> > +To accomplish not higging the main thread upon primary scrren height requests, the following changes were necessary:
> 
> higging -> hitting
> scrren -> screen

Fixed.
> 
> > Source/WebCore/accessibility/mac/AccessibilityObjectMac.mm:84
> > +    return screenRectForPrimaryScreen();
> 
> will this be expensive to call on initialization?

In this particular line, it is not initialization, this is the live object on demand. But we do call it on construction of the AXGeometryManager object and it is not expensive. the problem was that it has to be called on the main thread, not that it was expensive.

(In reply to Tyler Wilcock from comment #4)
> Comment on attachment 466293 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=466293&action=review
> 
> > COMMIT_MESSAGE:7
> > +To accomplish not higging the main thread upon primary scrren height requests, the following changes were necessary:
> 
> typo: higging, hitting

Fixed.
> 
> > Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:308
> > +    if (object.isScrollView() || object.isWebArea()) {
> > +        // For the ScrollView and WebArea objects, cache the TitleAttributeValue and DescriptionAttributeValue eagerly to avoid hitting the main thread in getOrRetrievePropertyValue during the construction of the isolated tree.
> > +        setProperty(AXPropertyName::TitleAttributeValue, object.titleAttributeValue().isolatedCopy());
> > +        setProperty(AXPropertyName::DescriptionAttributeValue, object.descriptionAttributeValue().isolatedCopy());
> 
> I believe one or both of these results in calls to textUnderElement, right?
> Since the point of this patch to facilitate fast startup, is it possible
> that these functions will recursively call textUnderElement all the way to
> the leaves?

That doesn't happen for ScrollView or WebArea objects.

(In reply to Andres Gonzalez from comment #7)
> (In reply to Tyler Wilcock from comment #4)
> > Comment on attachment 466293 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=466293&action=review
> > 
> > > COMMIT_MESSAGE:7
> > > +To accomplish not higging the main thread upon primary scrren height requests, the following changes were necessary:
> > 
> > typo: higging, hitting
> 
> Fixed.
> > 
> > > Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:308
> > > +    if (object.isScrollView() || object.isWebArea()) {
> > > +        // For the ScrollView and WebArea objects, cache the TitleAttributeValue and DescriptionAttributeValue eagerly to avoid hitting the main thread in getOrRetrievePropertyValue during the construction of the isolated tree.
> > > +        setProperty(AXPropertyName::TitleAttributeValue, object.titleAttributeValue().isolatedCopy());
> > > +        setProperty(AXPropertyName::DescriptionAttributeValue, object.descriptionAttributeValue().isolatedCopy());
> > 
> > I believe one or both of these results in calls to textUnderElement, right?
> > Since the point of this patch to facilitate fast startup, is it possible
> > that these functions will recursively call textUnderElement all the way to
> > the leaves?
> 
> That doesn't happen for ScrollView or WebArea objects.

I.e., textUnderElement is not called as a result of title or description AttributeValue for either of these object types.

Committed 263896@main (5fae8467d69f): <https://commits.webkit.org/263896@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 466298 [details].