WebKit Bugzilla
New
Browse
Search+
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
256526
AX: Avoid hitting the main thread during the construction of the isolated tree due to AXPrimaryScreenHeight.
https://bugs.webkit.org/show_bug.cgi?id=256526
Summary
AX: Avoid hitting the main thread during the construction of the isolated tre...
Andres Gonzalez
Reported
2023-05-09 09:07:09 PDT
This is one of the factors contributing to irresponsiveness during page load of large documents.
Attachments
Patch
(22.05 KB, patch)
2023-05-09 09:13 PDT
,
Andres Gonzalez
no flags
Details
Formatted Diff
Diff
Patch
(22.05 KB, patch)
2023-05-09 13:06 PDT
,
Andres Gonzalez
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2023-05-09 09:07:24 PDT
<
rdar://problem/109100219
>
Andres Gonzalez
Comment 2
2023-05-09 09:13:45 PDT
Created
attachment 466293
[details]
Patch
chris fleizach
Comment 3
2023-05-09 10:24:29 PDT
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?
Tyler Wilcock
Comment 4
2023-05-09 10:24:39 PDT
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?
Andres Gonzalez
Comment 5
2023-05-09 13:06:02 PDT
Created
attachment 466298
[details]
Patch
Andres Gonzalez
Comment 6
2023-05-09 13:12:26 PDT
(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.
Andres Gonzalez
Comment 7
2023-05-09 13:14:22 PDT
(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.
Andres Gonzalez
Comment 8
2023-05-09 13:17:55 PDT
(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.
EWS
Comment 9
2023-05-10 04:16:19 PDT
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]
.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug