Title: [290130] trunk/Source/WebCore
- Revision
- 290130
- Author
- [email protected]
- Date
- 2022-02-18 08:47:36 -0800 (Fri, 18 Feb 2022)
Log Message
AX: AccessibilityScrollView should be resilient to destruction of underlying scroll view
https://bugs.webkit.org/show_bug.cgi?id=236750
Reviewed by Andres Gonzalez.
Sometimes we can get into a state where we have a valid AccessibilityScrollView,
backed by an iframe, with a destroyed WeakPtr<ScrollView> m_scrollView. When this
happens, all of the content inside the iframe becomes inaccessible. We need to handle
the case where an iframe changes the frame and document it holds (e.g. respect the current frame in
HTMLFrameOwner::contentFrame, which can change).
This patch fixes this by caching a WeakPtr<HTMLFrameOwnerElement> when an AccessibilityScrollView
is created. A new method currentScrollView() has been added to prefer the original m_scrollView, and
fallback to the scrollview represented by the frame owner element if m_scrollView has been destroyed.
Thanks to Andres Gonzalez for helping to find and fix this issue.
* accessibility/AccessibilityScrollView.cpp:
(WebCore::AccessibilityScrollView::AccessibilityScrollView):
Cache the frame owner of the provided ScrollView* in new
m_frameOwnerElement field.
(WebCore::AccessibilityScrollView::detachRemoteParts):
Detach new m_frameOwnerElement field.
(WebCore::AccessibilityScrollView::currentScrollView const):
Added.
(WebCore::AccessibilityScrollView::isAttachment const):
(WebCore::AccessibilityScrollView::platformWidget const):
(WebCore::AccessibilityScrollView::updateScrollbars):
(WebCore::AccessibilityScrollView::computeAccessibilityIsIgnored const):
(WebCore::AccessibilityScrollView::addChildren):
(WebCore::AccessibilityScrollView::webAreaObject const):
(WebCore::AccessibilityScrollView::elementRect const):
(WebCore::AccessibilityScrollView::document const):
(WebCore::AccessibilityScrollView::documentFrameView const):
(WebCore::AccessibilityScrollView::parentObject const):
(WebCore::AccessibilityScrollView::scrollTo const):
Use new currentScrollView() method instead of directly using m_scrollView.
(WebCore::AccessibilityScrollView::widgetForAttachmentView const):
Inlined in header.
(WebCore::AccessibilityScrollView::parentObjectIfExists const):
Inlined in header as call to parentObject() since these methods were duplicated.
(WebCore::AccessibilityScrollView::getScrollableAreaIfScrollable const):
Inlined in header.
* accessibility/AccessibilityScrollView.h:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (290129 => 290130)
--- trunk/Source/WebCore/ChangeLog 2022-02-18 16:43:58 UTC (rev 290129)
+++ trunk/Source/WebCore/ChangeLog 2022-02-18 16:47:36 UTC (rev 290130)
@@ -1,3 +1,52 @@
+2022-02-18 Tyler Wilcock <[email protected]>
+
+ AX: AccessibilityScrollView should be resilient to destruction of underlying scroll view
+ https://bugs.webkit.org/show_bug.cgi?id=236750
+
+ Reviewed by Andres Gonzalez.
+
+ Sometimes we can get into a state where we have a valid AccessibilityScrollView,
+ backed by an iframe, with a destroyed WeakPtr<ScrollView> m_scrollView. When this
+ happens, all of the content inside the iframe becomes inaccessible. We need to handle
+ the case where an iframe changes the frame and document it holds (e.g. respect the current frame in
+ HTMLFrameOwner::contentFrame, which can change).
+
+ This patch fixes this by caching a WeakPtr<HTMLFrameOwnerElement> when an AccessibilityScrollView
+ is created. A new method currentScrollView() has been added to prefer the original m_scrollView, and
+ fallback to the scrollview represented by the frame owner element if m_scrollView has been destroyed.
+
+ Thanks to Andres Gonzalez for helping to find and fix this issue.
+
+ * accessibility/AccessibilityScrollView.cpp:
+ (WebCore::AccessibilityScrollView::AccessibilityScrollView):
+ Cache the frame owner of the provided ScrollView* in new
+ m_frameOwnerElement field.
+ (WebCore::AccessibilityScrollView::detachRemoteParts):
+ Detach new m_frameOwnerElement field.
+ (WebCore::AccessibilityScrollView::currentScrollView const):
+ Added.
+
+ (WebCore::AccessibilityScrollView::isAttachment const):
+ (WebCore::AccessibilityScrollView::platformWidget const):
+ (WebCore::AccessibilityScrollView::updateScrollbars):
+ (WebCore::AccessibilityScrollView::computeAccessibilityIsIgnored const):
+ (WebCore::AccessibilityScrollView::addChildren):
+ (WebCore::AccessibilityScrollView::webAreaObject const):
+ (WebCore::AccessibilityScrollView::elementRect const):
+ (WebCore::AccessibilityScrollView::document const):
+ (WebCore::AccessibilityScrollView::documentFrameView const):
+ (WebCore::AccessibilityScrollView::parentObject const):
+ (WebCore::AccessibilityScrollView::scrollTo const):
+ Use new currentScrollView() method instead of directly using m_scrollView.
+
+ (WebCore::AccessibilityScrollView::widgetForAttachmentView const):
+ Inlined in header.
+ (WebCore::AccessibilityScrollView::parentObjectIfExists const):
+ Inlined in header as call to parentObject() since these methods were duplicated.
+ (WebCore::AccessibilityScrollView::getScrollableAreaIfScrollable const):
+ Inlined in header.
+ * accessibility/AccessibilityScrollView.h:
+
2022-02-18 Mark Lam <[email protected]>
Split IsoSubspace into a GCClient allocator used by VM and a backend managed by Heap.
Modified: trunk/Source/WebCore/accessibility/AccessibilityScrollView.cpp (290129 => 290130)
--- trunk/Source/WebCore/accessibility/AccessibilityScrollView.cpp 2022-02-18 16:43:58 UTC (rev 290129)
+++ trunk/Source/WebCore/accessibility/AccessibilityScrollView.cpp 2022-02-18 16:47:36 UTC (rev 290130)
@@ -32,7 +32,6 @@
#include "FrameView.h"
#include "HTMLFrameOwnerElement.h"
#include "RenderElement.h"
-#include "ScrollView.h"
#include "Widget.h"
namespace WebCore {
@@ -41,6 +40,8 @@
: m_scrollView(view)
, m_childrenDirty(false)
{
+ if (is<FrameView>(view))
+ m_frameOwnerElement = downcast<FrameView>(*view).frame().ownerElement();
}
AccessibilityScrollView::~AccessibilityScrollView()
@@ -52,6 +53,7 @@
{
AccessibilityObject::detachRemoteParts(detachmentType);
m_scrollView = nullptr;
+ m_frameOwnerElement = nullptr;
}
Ref<AccessibilityScrollView> AccessibilityScrollView::create(ScrollView* view)
@@ -58,7 +60,12 @@
{
return adoptRef(*new AccessibilityScrollView(view));
}
-
+
+ScrollView* AccessibilityScrollView::currentScrollView() const
+{
+ return m_scrollView ? m_scrollView.get() : documentFrameView();
+}
+
AccessibilityObject* AccessibilityScrollView::scrollBar(AccessibilityOrientation orientation)
{
updateScrollbars();
@@ -80,19 +87,18 @@
// In WebKit2, the ScrollView object will return the AX information (because there are no platform widgets).
bool AccessibilityScrollView::isAttachment() const
{
- return m_scrollView && m_scrollView->platformWidget();
+ if (auto* scrollView = currentScrollView())
+ return scrollView->platformWidget();
+ return false;
}
PlatformWidget AccessibilityScrollView::platformWidget() const
{
- return m_scrollView ? m_scrollView->platformWidget() : nullptr;
+ if (auto* scrollView = currentScrollView())
+ return scrollView->platformWidget();
+ return nullptr;
}
-Widget* AccessibilityScrollView::widgetForAttachmentView() const
-{
- return m_scrollView.get();
-}
-
bool AccessibilityScrollView::canSetFocusAttribute() const
{
AccessibilityObject* webArea = webAreaObject();
@@ -125,19 +131,20 @@
void AccessibilityScrollView::updateScrollbars()
{
- if (!m_scrollView)
+ auto* scrollView = currentScrollView();
+ if (!scrollView)
return;
- if (m_scrollView->horizontalScrollbar() && !m_horizontalScrollbar)
- m_horizontalScrollbar = addChildScrollbar(m_scrollView->horizontalScrollbar());
- else if (!m_scrollView->horizontalScrollbar() && m_horizontalScrollbar) {
+ if (scrollView->horizontalScrollbar() && !m_horizontalScrollbar)
+ m_horizontalScrollbar = addChildScrollbar(scrollView->horizontalScrollbar());
+ else if (!scrollView->horizontalScrollbar() && m_horizontalScrollbar) {
removeChildScrollbar(m_horizontalScrollbar.get());
m_horizontalScrollbar = nullptr;
}
- if (m_scrollView->verticalScrollbar() && !m_verticalScrollbar)
- m_verticalScrollbar = addChildScrollbar(m_scrollView->verticalScrollbar());
- else if (!m_scrollView->verticalScrollbar() && m_verticalScrollbar) {
+ if (scrollView->verticalScrollbar() && !m_verticalScrollbar)
+ m_verticalScrollbar = addChildScrollbar(scrollView->verticalScrollbar());
+ else if (!scrollView->verticalScrollbar() && m_verticalScrollbar) {
removeChildScrollbar(m_verticalScrollbar.get());
m_verticalScrollbar = nullptr;
}
@@ -179,7 +186,7 @@
AccessibilityObject* webArea = webAreaObject();
if (!webArea)
return true;
-
+
return webArea->accessibilityIsIgnored();
}
@@ -187,23 +194,20 @@
{
ASSERT(!m_childrenInitialized);
m_childrenInitialized = true;
-
+
addChild(webAreaObject());
- updateScrollbars();
+ updateScrollbars();
}
AccessibilityObject* AccessibilityScrollView::webAreaObject() const
{
- if (!is<FrameView>(m_scrollView))
- return nullptr;
-
- Document* document = downcast<FrameView>(*m_scrollView).frame().document();
+ auto* document = this->document();
if (!document || !document->hasLivingRenderTree())
return nullptr;
- if (AXObjectCache* cache = axObjectCache())
+ if (auto* cache = axObjectCache())
return cache->getOrCreate(document);
-
+
return nullptr;
}
@@ -223,11 +227,13 @@
LayoutRect AccessibilityScrollView::elementRect() const
{
- if (!m_scrollView)
+ auto* scrollView = currentScrollView();
+ if (!scrollView)
return LayoutRect();
- LayoutRect rect = m_scrollView->frameRect();
- float topContentInset = m_scrollView->topContentInset();
+ LayoutRect rect = scrollView->frameRect();
+ float topContentInset = scrollView->topContentInset();
+
// Top content inset pushes the frame down and shrinks it.
rect.move(0, topContentInset);
rect.contract(0, topContentInset);
@@ -234,55 +240,44 @@
return rect;
}
-FrameView* AccessibilityScrollView::documentFrameView() const
+Document* AccessibilityScrollView::document() const
{
- if (!is<FrameView>(m_scrollView))
- return nullptr;
+ if (is<FrameView>(m_scrollView))
+ return downcast<FrameView>(*m_scrollView).frame().document();
+ return AccessibilityObject::document();
+}
- return downcast<FrameView>(m_scrollView.get());
-}
-
-AccessibilityObject* AccessibilityScrollView::parentObject() const
+FrameView* AccessibilityScrollView::documentFrameView() const
{
- if (!is<FrameView>(m_scrollView))
- return nullptr;
+ if (is<FrameView>(m_scrollView))
+ return downcast<FrameView>(m_scrollView.get());
- AXObjectCache* cache = axObjectCache();
- if (!cache)
- return nullptr;
+ if (m_frameOwnerElement && m_frameOwnerElement->contentDocument())
+ return m_frameOwnerElement->contentDocument()->view();
- HTMLFrameOwnerElement* owner = downcast<FrameView>(*m_scrollView).frame().ownerElement();
- if (owner && owner->renderer())
- return cache->getOrCreate(owner);
-
return nullptr;
}
-
-AccessibilityObject* AccessibilityScrollView::parentObjectIfExists() const
+
+AccessibilityObject* AccessibilityScrollView::parentObject() const
{
- if (!is<FrameView>(m_scrollView))
- return nullptr;
-
AXObjectCache* cache = axObjectCache();
if (!cache)
return nullptr;
- HTMLFrameOwnerElement* owner = downcast<FrameView>(*m_scrollView).frame().ownerElement();
+ HTMLFrameOwnerElement* owner = m_frameOwnerElement.get();
+ if (is<FrameView>(m_scrollView))
+ owner = downcast<FrameView>(*m_scrollView).frame().ownerElement();
+
if (owner && owner->renderer())
- return cache->get(owner);
+ return cache->getOrCreate(owner);
return nullptr;
}
-ScrollableArea* AccessibilityScrollView::getScrollableAreaIfScrollable() const
-{
- return m_scrollView.get();
-}
-
void AccessibilityScrollView::scrollTo(const IntPoint& point) const
{
- if (m_scrollView)
- m_scrollView->setScrollPosition(point);
+ if (auto* scrollView = currentScrollView())
+ scrollView->setScrollPosition(point);
}
} // namespace WebCore
Modified: trunk/Source/WebCore/accessibility/AccessibilityScrollView.h (290129 => 290130)
--- trunk/Source/WebCore/accessibility/AccessibilityScrollView.h 2022-02-18 16:43:58 UTC (rev 290129)
+++ trunk/Source/WebCore/accessibility/AccessibilityScrollView.h 2022-02-18 16:47:36 UTC (rev 290130)
@@ -26,6 +26,7 @@
#pragma once
#include "AccessibilityObject.h"
+#include "ScrollView.h"
namespace WebCore {
@@ -37,7 +38,7 @@
public:
static Ref<AccessibilityScrollView> create(ScrollView*);
AccessibilityRole roleValue() const override { return AccessibilityRole::ScrollArea; }
- ScrollView* scrollView() const override { return m_scrollView.get(); }
+ ScrollView* scrollView() const override { return currentScrollView(); }
virtual ~AccessibilityScrollView();
@@ -47,7 +48,8 @@
explicit AccessibilityScrollView(ScrollView*);
void detachRemoteParts(AccessibilityDetachmentType) override;
- ScrollableArea* getScrollableAreaIfScrollable() const override;
+ ScrollView* currentScrollView() const;
+ ScrollableArea* getScrollableAreaIfScrollable() const override { return currentScrollView(); }
void scrollTo(const IntPoint&) const override;
bool computeAccessibilityIsIgnored() const override;
bool isAccessibilityScrollViewInstance() const override { return true; }
@@ -55,7 +57,7 @@
bool isAttachment() const override;
PlatformWidget platformWidget() const override;
- Widget* widgetForAttachmentView() const override;
+ Widget* widgetForAttachmentView() const override { return currentScrollView(); }
AccessibilityObject* scrollBar(AccessibilityOrientation) override;
void addChildren() override;
@@ -67,17 +69,19 @@
void setFocused(bool) override;
bool canSetFocusAttribute() const override;
bool isFocused() const override;
-
+
+ Document* document() const override;
FrameView* documentFrameView() const override;
LayoutRect elementRect() const override;
AccessibilityObject* parentObject() const override;
- AccessibilityObject* parentObjectIfExists() const override;
+ AccessibilityObject* parentObjectIfExists() const override { return parentObject(); }
AccessibilityObject* firstChild() const override { return webAreaObject(); }
AccessibilityScrollbar* addChildScrollbar(Scrollbar*);
void removeChildScrollbar(AccessibilityObject*);
-
+
WeakPtr<ScrollView> m_scrollView;
+ WeakPtr<HTMLFrameOwnerElement> m_frameOwnerElement;
RefPtr<AccessibilityObject> m_horizontalScrollbar;
RefPtr<AccessibilityObject> m_verticalScrollbar;
bool m_childrenDirty;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
