- Revision
- 258356
- Author
- [email protected]
- Date
- 2020-03-12 14:12:13 -0700 (Thu, 12 Mar 2020)
Log Message
Cache the ScrollView platformWidget to avoid [WebAccessibilityObjectWrapper scrollViewParent] hitting the main thread too often.
https://bugs.webkit.org/show_bug.cgi?id=209010
Reviewed by Chris Fleizach.
- [WebAccessibilityObjectWrapper scrollViewParent] is called very often
and blocks the AXThread to retrieve a value from the main thread. This
change caches the PlatformWidget for the corresponding ScrollView (an
NSView) to avoid hitting the main thread that often.
- In Addition, made the ScrollView member of AccessibilityScrollView a
WeakPtr instead of a naked pointer.
- Removed an unused lock from AXIsolatedObject and the const qualifier
from the return value of stringAttributeValue which is unnecessary.
* accessibility/AccessibilityObject.h:
* accessibility/AccessibilityObjectInterface.h:
* accessibility/AccessibilityScrollView.cpp:
(WebCore::AccessibilityScrollView::AccessibilityScrollView):
(WebCore::AccessibilityScrollView::platformWidget const):
(WebCore::AccessibilityScrollView::widgetForAttachmentView const):
(WebCore::AccessibilityScrollView::webAreaObject const):
(WebCore::AccessibilityScrollView::documentFrameView const):
(WebCore::AccessibilityScrollView::parentObject const):
(WebCore::AccessibilityScrollView::parentObjectIfExists const):
(WebCore::AccessibilityScrollView::getScrollableAreaIfScrollable const):
* accessibility/AccessibilityScrollView.h:
* accessibility/isolatedtree/AXIsolatedObject.cpp:
(WebCore::AXIsolatedObject::initializeAttributeData):
(WebCore::AXIsolatedObject::stringAttributeValue const):
(WebCore::AXIsolatedObject::platformWidget const):
* accessibility/isolatedtree/AXIsolatedObject.h:
* accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
(-[WebAccessibilityObjectWrapper scrollViewParent]):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (258355 => 258356)
--- trunk/Source/WebCore/ChangeLog 2020-03-12 21:06:33 UTC (rev 258355)
+++ trunk/Source/WebCore/ChangeLog 2020-03-12 21:12:13 UTC (rev 258356)
@@ -1,5 +1,41 @@
2020-03-12 Andres Gonzalez <[email protected]>
+ Cache the ScrollView platformWidget to avoid [WebAccessibilityObjectWrapper scrollViewParent] hitting the main thread too often.
+ https://bugs.webkit.org/show_bug.cgi?id=209010
+
+ Reviewed by Chris Fleizach.
+
+ - [WebAccessibilityObjectWrapper scrollViewParent] is called very often
+ and blocks the AXThread to retrieve a value from the main thread. This
+ change caches the PlatformWidget for the corresponding ScrollView (an
+ NSView) to avoid hitting the main thread that often.
+ - In Addition, made the ScrollView member of AccessibilityScrollView a
+ WeakPtr instead of a naked pointer.
+ - Removed an unused lock from AXIsolatedObject and the const qualifier
+ from the return value of stringAttributeValue which is unnecessary.
+
+ * accessibility/AccessibilityObject.h:
+ * accessibility/AccessibilityObjectInterface.h:
+ * accessibility/AccessibilityScrollView.cpp:
+ (WebCore::AccessibilityScrollView::AccessibilityScrollView):
+ (WebCore::AccessibilityScrollView::platformWidget const):
+ (WebCore::AccessibilityScrollView::widgetForAttachmentView const):
+ (WebCore::AccessibilityScrollView::webAreaObject const):
+ (WebCore::AccessibilityScrollView::documentFrameView const):
+ (WebCore::AccessibilityScrollView::parentObject const):
+ (WebCore::AccessibilityScrollView::parentObjectIfExists const):
+ (WebCore::AccessibilityScrollView::getScrollableAreaIfScrollable const):
+ * accessibility/AccessibilityScrollView.h:
+ * accessibility/isolatedtree/AXIsolatedObject.cpp:
+ (WebCore::AXIsolatedObject::initializeAttributeData):
+ (WebCore::AXIsolatedObject::stringAttributeValue const):
+ (WebCore::AXIsolatedObject::platformWidget const):
+ * accessibility/isolatedtree/AXIsolatedObject.h:
+ * accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
+ (-[WebAccessibilityObjectWrapper scrollViewParent]):
+
+2020-03-12 Andres Gonzalez <[email protected]>
+
Attributes SelectionTextMarkerRange and Start/EndTextMarker need to run on the main thread.
https://bugs.webkit.org/show_bug.cgi?id=208996
Modified: trunk/Source/WebCore/accessibility/AccessibilityObject.h (258355 => 258356)
--- trunk/Source/WebCore/accessibility/AccessibilityObject.h 2020-03-12 21:06:33 UTC (rev 258355)
+++ trunk/Source/WebCore/accessibility/AccessibilityObject.h 2020-03-12 21:12:13 UTC (rev 258356)
@@ -442,6 +442,7 @@
String accessKey() const override { return nullAtom(); }
String actionVerb() const override;
Widget* widget() const override { return nullptr; }
+ PlatformWidget platformWidget() const override { return nullptr; }
Widget* widgetForAttachmentView() const override { return nullptr; }
Page* page() const override;
Document* document() const override;
Modified: trunk/Source/WebCore/accessibility/AccessibilityObjectInterface.h (258355 => 258356)
--- trunk/Source/WebCore/accessibility/AccessibilityObjectInterface.h 2020-03-12 21:06:33 UTC (rev 258355)
+++ trunk/Source/WebCore/accessibility/AccessibilityObjectInterface.h 2020-03-12 21:12:13 UTC (rev 258356)
@@ -868,6 +868,7 @@
virtual String accessKey() const = 0;
virtual String actionVerb() const = 0;
virtual Widget* widget() const = 0;
+ virtual PlatformWidget platformWidget() const = 0;
virtual Widget* widgetForAttachmentView() const = 0;
virtual Page* page() const = 0;
virtual Document* document() const = 0;
Modified: trunk/Source/WebCore/accessibility/AccessibilityScrollView.cpp (258355 => 258356)
--- trunk/Source/WebCore/accessibility/AccessibilityScrollView.cpp 2020-03-12 21:06:33 UTC (rev 258355)
+++ trunk/Source/WebCore/accessibility/AccessibilityScrollView.cpp 2020-03-12 21:12:13 UTC (rev 258356)
@@ -38,7 +38,7 @@
namespace WebCore {
AccessibilityScrollView::AccessibilityScrollView(ScrollView* view)
- : m_scrollView(view)
+ : m_scrollView(makeWeakPtr(view))
, m_childrenDirty(false)
{
}
@@ -83,9 +83,14 @@
return m_scrollView && m_scrollView->platformWidget();
}
+PlatformWidget AccessibilityScrollView::platformWidget() const
+{
+ return m_scrollView ? m_scrollView->platformWidget() : nullptr;
+}
+
Widget* AccessibilityScrollView::widgetForAttachmentView() const
{
- return m_scrollView;
+ return m_scrollView.get();
}
bool AccessibilityScrollView::canSetFocusAttribute() const
@@ -186,9 +191,9 @@
AccessibilityObject* AccessibilityScrollView::webAreaObject() const
{
- if (!is<FrameView>(m_scrollView))
+ if (!is<FrameView>(m_scrollView.get()))
return nullptr;
-
+
Document* document = downcast<FrameView>(*m_scrollView).frame().document();
if (!document || !document->hasLivingRenderTree())
return nullptr;
@@ -228,15 +233,15 @@
FrameView* AccessibilityScrollView::documentFrameView() const
{
- if (!is<FrameView>(m_scrollView))
+ if (!is<FrameView>(m_scrollView.get()))
return nullptr;
-
- return downcast<FrameView>(m_scrollView);
+
+ return downcast<FrameView>(m_scrollView.get());
}
AccessibilityObject* AccessibilityScrollView::parentObject() const
{
- if (!is<FrameView>(m_scrollView))
+ if (!is<FrameView>(m_scrollView.get()))
return nullptr;
AXObjectCache* cache = axObjectCache();
@@ -252,23 +257,23 @@
AccessibilityObject* AccessibilityScrollView::parentObjectIfExists() const
{
- if (!is<FrameView>(m_scrollView))
+ if (!is<FrameView>(m_scrollView.get()))
return nullptr;
-
+
AXObjectCache* cache = axObjectCache();
if (!cache)
return nullptr;
- HTMLFrameOwnerElement* owner = downcast<FrameView>(m_scrollView)->frame().ownerElement();
+ HTMLFrameOwnerElement* owner = downcast<FrameView>(*m_scrollView).frame().ownerElement();
if (owner && owner->renderer())
return cache->get(owner);
-
+
return nullptr;
}
ScrollableArea* AccessibilityScrollView::getScrollableAreaIfScrollable() const
{
- return m_scrollView;
+ return m_scrollView.get();
}
void AccessibilityScrollView::scrollTo(const IntPoint& point) const
Modified: trunk/Source/WebCore/accessibility/AccessibilityScrollView.h (258355 => 258356)
--- trunk/Source/WebCore/accessibility/AccessibilityScrollView.h 2020-03-12 21:06:33 UTC (rev 258355)
+++ trunk/Source/WebCore/accessibility/AccessibilityScrollView.h 2020-03-12 21:12:13 UTC (rev 258356)
@@ -37,7 +37,7 @@
public:
static Ref<AccessibilityScrollView> create(ScrollView*);
AccessibilityRole roleValue() const override { return AccessibilityRole::ScrollArea; }
- ScrollView* scrollView() const override { return m_scrollView; }
+ ScrollView* scrollView() const override { return m_scrollView.get(); }
virtual ~AccessibilityScrollView();
@@ -54,6 +54,7 @@
bool isEnabled() const override { return true; }
bool isAttachment() const override;
+ PlatformWidget platformWidget() const override;
Widget* widgetForAttachmentView() const override;
AccessibilityObject* scrollBar(AccessibilityOrientation) override;
@@ -76,7 +77,7 @@
AccessibilityScrollbar* addChildScrollbar(Scrollbar*);
void removeChildScrollbar(AccessibilityObject*);
- ScrollView* m_scrollView;
+ WeakPtr<ScrollView> m_scrollView;
RefPtr<AccessibilityObject> m_horizontalScrollbar;
RefPtr<AccessibilityObject> m_verticalScrollbar;
bool m_childrenDirty;
Modified: trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp (258355 => 258356)
--- trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp 2020-03-12 21:06:33 UTC (rev 258355)
+++ trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp 2020-03-12 21:12:13 UTC (rev 258356)
@@ -375,7 +375,10 @@
setMathscripts(AXPropertyName::MathPrescripts, object);
setMathscripts(AXPropertyName::MathPostscripts, object);
}
-
+
+ if (object.isScrollView())
+ m_platformWidget = object.platformWidget();
+
if (isRoot) {
setObjectProperty(AXPropertyName::WebArea, object.webAreaObject());
setProperty(AXPropertyName::PreventKeyboardDOMEventDispatch, object.preventKeyboardDOMEventDispatch());
@@ -828,7 +831,7 @@
);
}
-const String AXIsolatedObject::stringAttributeValue(AXPropertyName propertyName) const
+String AXIsolatedObject::stringAttributeValue(AXPropertyName propertyName) const
{
auto value = m_attributeMap.get(propertyName);
return WTF::switchOn(value,
@@ -1560,6 +1563,15 @@
return nullptr;
}
+PlatformWidget AXIsolatedObject::platformWidget() const
+{
+#if PLATFORM(COCOA)
+ return m_platformWidget.get();
+#else
+ return m_platformWidget;
+#endif
+}
+
Widget* AXIsolatedObject::widgetForAttachmentView() const
{
ASSERT_NOT_REACHED();
Modified: trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h (258355 => 258356)
--- trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h 2020-03-12 21:06:33 UTC (rev 258355)
+++ trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h 2020-03-12 21:12:13 UTC (rev 258356)
@@ -344,8 +344,9 @@
void setObjectProperty(AXPropertyName, AXCoreObject*);
void setObjectVectorProperty(AXPropertyName, const AccessibilityChildrenVector&);
+ // FIXME: consolidate all AttributeValue retrieval in a single template method.
bool boolAttributeValue(AXPropertyName) const;
- const String stringAttributeValue(AXPropertyName) const;
+ String stringAttributeValue(AXPropertyName) const;
int intAttributeValue(AXPropertyName) const;
unsigned unsignedAttributeValue(AXPropertyName) const;
double doubleAttributeValue(AXPropertyName) const;
@@ -821,6 +822,7 @@
bool supportsPath() const override { return boolAttributeValue(AXPropertyName::SupportsPath); }
TextIteratorBehavior textIteratorBehaviorForTextRange() const override;
Widget* widget() const override;
+ PlatformWidget platformWidget() const override;
Widget* widgetForAttachmentView() const override;
Page* page() const override;
Document* document() const override;
@@ -896,7 +898,12 @@
Vector<RefPtr<AXCoreObject>> m_children;
HashMap<AXPropertyName, AttributeValueVariant, WTF::IntHash<AXPropertyName>, WTF::StrongEnumHashTraits<AXPropertyName>> m_attributeMap;
- Lock m_attributeMapLock;
+
+#if PLATFORM(COCOA)
+ RetainPtr<NSView> m_platformWidget;
+#else
+ PlatformWidget m_platformWidget;
+#endif
};
} // namespace WebCore
Modified: trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm (258355 => 258356)
--- trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm 2020-03-12 21:06:33 UTC (rev 258355)
+++ trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm 2020-03-12 21:12:13 UTC (rev 258356)
@@ -2269,25 +2269,19 @@
- (id)scrollViewParent
{
- return Accessibility::retrieveAutoreleasedValueFromMainThread<id>([protectedSelf = RetainPtr<WebAccessibilityObjectWrapper>(self)] () -> RetainPtr<id> {
- auto* backingObject = protectedSelf.get().axBackingObject;
- if (!backingObject || !backingObject->isScrollView())
- return nil;
+ auto* backingObject = self.axBackingObject;
+ if (!backingObject || !backingObject->isScrollView())
+ return nil;
- // If this scroll view provides it's parent object (because it's a sub-frame), then
- // we should not find the remoteAccessibilityParent.
- if (backingObject->parentObject())
- return nil;
+ // If this scroll view provides it's parent object (because it's a sub-frame), then
+ // we should not find the remoteAccessibilityParent.
+ if (backingObject->parentObject())
+ return nil;
- auto* scroll = backingObject->scrollView();
- if (!scroll)
- return nil;
+ if (auto platformWidget = backingObject->platformWidget())
+ return NSAccessibilityUnignoredAncestor(platformWidget);
- if (scroll->platformWidget())
- return NSAccessibilityUnignoredAncestor(scroll->platformWidget());
-
- return [protectedSelf remoteAccessibilityParentObject];
- });
+ return [self remoteAccessibilityParentObject];
}
- (id)windowElement:(NSString*)attributeName
