Diff
Modified: trunk/LayoutTests/ChangeLog (216418 => 216419)
--- trunk/LayoutTests/ChangeLog 2017-05-08 15:26:38 UTC (rev 216418)
+++ trunk/LayoutTests/ChangeLog 2017-05-08 15:33:47 UTC (rev 216419)
@@ -1,3 +1,13 @@
+2017-05-06 Zalan Bujtas <za...@apple.com>
+
+ Ensure clean tree before AX cache update.
+ https://bugs.webkit.org/show_bug.cgi?id=171546
+ <rdar://problem/31934942>
+
+ Reviewed by Chris Fleizach.
+
+ * accessibility/crash-when-render-tree-is-not-clean.html: Added.
+
2017-05-08 Myles C. Maxfield <mmaxfi...@apple.com>
Unprefix unicode-bidi CSS values
Added: trunk/LayoutTests/accessibility/crash-when-render-tree-is-not-clean-expected.txt (0 => 216419)
--- trunk/LayoutTests/accessibility/crash-when-render-tree-is-not-clean-expected.txt (rev 0)
+++ trunk/LayoutTests/accessibility/crash-when-render-tree-is-not-clean-expected.txt 2017-05-08 15:33:47 UTC (rev 216419)
@@ -0,0 +1,2 @@
+Pass if no crash or assert.
+
Added: trunk/LayoutTests/accessibility/crash-when-render-tree-is-not-clean.html (0 => 216419)
--- trunk/LayoutTests/accessibility/crash-when-render-tree-is-not-clean.html (rev 0)
+++ trunk/LayoutTests/accessibility/crash-when-render-tree-is-not-clean.html 2017-05-08 15:33:47 UTC (rev 216419)
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>This tests that we properly update the tree before updating accessibility cache.</title>
+<script>
+if (window.accessibilityController)
+ accessibilityController.accessibleElementById("outer");
+if (window.testRunner)
+ testRunner.dumpAsText();
+</script>
+</head>
+<body>
+Pass if no crash or assert.
+<div id=outer><div id=inner>foobar</div></div>
+<script>
+ inner.style.display = "none";
+ outer.setAttribute("aria-labeledby", "inner");
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (216418 => 216419)
--- trunk/Source/WebCore/ChangeLog 2017-05-08 15:26:38 UTC (rev 216418)
+++ trunk/Source/WebCore/ChangeLog 2017-05-08 15:33:47 UTC (rev 216419)
@@ -1,3 +1,43 @@
+2017-05-06 Zalan Bujtas <za...@apple.com>
+
+ Ensure clean tree before AX cache update.
+ https://bugs.webkit.org/show_bug.cgi?id=171546
+ <rdar://problem/31934942>
+
+ While updating an accessibility object state, we might
+ perform unintentional style updates. This style update could
+ end up destroying renderes that are still referenced by function calls
+ on the callstack.
+ To avoid that, AXObjectCache should operate on a clean tree only.
+
+ Reviewed by Chris Fleizach.
+
+ Test: accessibility/crash-when-render-tree-is-not-clean.html
+
+ * accessibility/AXObjectCache.cpp:
+ (WebCore::AXObjectCache::checkedStateChanged):
+ (WebCore::AXObjectCache::selectedChildrenChanged):
+ (WebCore::AXObjectCache::handleAriaExpandedChange):
+ (WebCore::AXObjectCache::handleActiveDescendantChanged):
+ (WebCore::AXObjectCache::handleAriaRoleChanged):
+ (WebCore::AXObjectCache::handleAttributeChanged):
+ (WebCore::AXObjectCache::handleAriaModalChange):
+ (WebCore::AXObjectCache::labelChanged):
+ * accessibility/AXObjectCache.h:
+ (WebCore::AXObjectCache::checkedStateChanged):
+ (WebCore::AXObjectCache::handleActiveDescendantChanged):
+ (WebCore::AXObjectCache::handleAriaExpandedChange):
+ (WebCore::AXObjectCache::handleAriaRoleChanged):
+ (WebCore::AXObjectCache::handleAriaModalChange):
+ (WebCore::AXObjectCache::handleAttributeChanged):
+ (WebCore::AXObjectCache::selectedChildrenChanged):
+ * accessibility/AccessibilityRenderObject.cpp:
+ (WebCore::AccessibilityRenderObject::handleAriaExpandedChanged):
+ * dom/Element.cpp:
+ (WebCore::Element::attributeChanged):
+ * html/HTMLInputElement.cpp:
+ (WebCore::HTMLInputElement::setChecked):
+
2017-05-08 Myles C. Maxfield <mmaxfi...@apple.com>
Unprefix unicode-bidi CSS values
Modified: trunk/Source/WebCore/accessibility/AXObjectCache.cpp (216418 => 216419)
--- trunk/Source/WebCore/accessibility/AXObjectCache.cpp 2017-05-08 15:26:38 UTC (rev 216418)
+++ trunk/Source/WebCore/accessibility/AXObjectCache.cpp 2017-05-08 15:33:47 UTC (rev 216419)
@@ -1002,9 +1002,10 @@
postPlatformNotification(object, notification);
}
-void AXObjectCache::checkedStateChanged(Node* node)
+void AXObjectCache::checkedStateChanged(Element& element)
{
- postNotification(node, AXObjectCache::AXCheckedStateChanged);
+ element.document().updateStyleIfNeeded();
+ postNotification(&element, AXObjectCache::AXCheckedStateChanged);
}
void AXObjectCache::handleMenuItemSelected(Node* node)
@@ -1027,13 +1028,14 @@
platformHandleFocusedUIElementChanged(oldNode, newNode);
}
-void AXObjectCache::selectedChildrenChanged(Node* node)
+void AXObjectCache::selectedChildrenChanged(Element& element)
{
- handleMenuItemSelected(node);
+ element.document().updateStyleIfNeeded();
+ handleMenuItemSelected(&element);
// postTarget is TargetObservableParent so that you can pass in any child of an element and it will go up the parent tree
// to find the container which should send out the notification.
- postNotification(node, AXSelectedChildrenChanged, TargetObservableParent);
+ postNotification(&element, AXSelectedChildrenChanged, TargetObservableParent);
}
void AXObjectCache::selectedChildrenChanged(RenderObject* renderer)
@@ -1402,35 +1404,39 @@
}
}
-void AXObjectCache::handleAriaExpandedChange(Node* node)
+void AXObjectCache::handleAriaExpandedChange(Element& element)
{
- if (AccessibilityObject* obj = getOrCreate(node))
+ ASSERT_WITH_SECURITY_IMPLICATION(!element.needsStyleRecalc());
+ if (AccessibilityObject* obj = getOrCreate(&element))
obj->handleAriaExpandedChanged();
}
-void AXObjectCache::handleActiveDescendantChanged(Node* node)
+void AXObjectCache::handleActiveDescendantChanged(Element& element)
{
- if (AccessibilityObject* obj = getOrCreate(node))
+ ASSERT_WITH_SECURITY_IMPLICATION(!element.needsStyleRecalc());
+ if (AccessibilityObject* obj = getOrCreate(&element))
obj->handleActiveDescendantChanged();
}
-void AXObjectCache::handleAriaRoleChanged(Node* node)
+void AXObjectCache::handleAriaRoleChanged(Element& element)
{
+ ASSERT_WITH_SECURITY_IMPLICATION(!element.needsStyleRecalc());
stopCachingComputedObjectAttributes();
- if (AccessibilityObject* obj = getOrCreate(node)) {
+ if (AccessibilityObject* obj = getOrCreate(&element)) {
obj->updateAccessibilityRole();
obj->notifyIfIgnoredValueChanged();
}
}
-void AXObjectCache::handleAttributeChanged(const QualifiedName& attrName, Element* element)
+void AXObjectCache::handleAttributeChanged(const QualifiedName& attrName, Element& element)
{
+ element.document().updateStyleIfNeeded();
if (attrName == roleAttr)
handleAriaRoleChanged(element);
else if (attrName == altAttr || attrName == titleAttr)
- textChanged(element);
- else if (attrName == forAttr && is<HTMLLabelElement>(*element))
+ textChanged(&element);
+ else if (attrName == forAttr && is<HTMLLabelElement>(element))
labelChanged(element);
if (!attrName.localName().string().startsWith("aria-"))
@@ -1439,11 +1445,11 @@
if (attrName == aria_activedescendantAttr)
handleActiveDescendantChanged(element);
else if (attrName == aria_busyAttr)
- postNotification(element, AXObjectCache::AXElementBusyChanged);
+ postNotification(&element, AXObjectCache::AXElementBusyChanged);
else if (attrName == aria_valuenowAttr || attrName == aria_valuetextAttr)
- postNotification(element, AXObjectCache::AXValueChanged);
+ postNotification(&element, AXObjectCache::AXValueChanged);
else if (attrName == aria_labelAttr || attrName == aria_labeledbyAttr || attrName == aria_labelledbyAttr)
- textChanged(element);
+ textChanged(&element);
else if (attrName == aria_checkedAttr)
checkedStateChanged(element);
else if (attrName == aria_selectedAttr)
@@ -1451,34 +1457,32 @@
else if (attrName == aria_expandedAttr)
handleAriaExpandedChange(element);
else if (attrName == aria_hiddenAttr)
- childrenChanged(element->parentNode(), element);
+ childrenChanged(element.parentNode(), &element);
else if (attrName == aria_invalidAttr)
- postNotification(element, AXObjectCache::AXInvalidStatusChanged);
+ postNotification(&element, AXObjectCache::AXInvalidStatusChanged);
else if (attrName == aria_modalAttr)
handleAriaModalChange(element);
else if (attrName == aria_currentAttr)
- postNotification(element, AXObjectCache::AXCurrentChanged);
+ postNotification(&element, AXObjectCache::AXCurrentChanged);
else
- postNotification(element, AXObjectCache::AXAriaAttributeChanged);
+ postNotification(&element, AXObjectCache::AXAriaAttributeChanged);
}
-void AXObjectCache::handleAriaModalChange(Node* node)
+void AXObjectCache::handleAriaModalChange(Element& element)
{
- if (!is<Element>(node))
+ ASSERT_WITH_SECURITY_IMPLICATION(!element.needsStyleRecalc());
+ if (!nodeHasRole(&element, "dialog") && !nodeHasRole(&element, "alertdialog"))
return;
- if (!nodeHasRole(node, "dialog") && !nodeHasRole(node, "alertdialog"))
- return;
-
stopCachingComputedObjectAttributes();
- if (equalLettersIgnoringASCIICase(downcast<Element>(*node).attributeWithoutSynchronization(aria_modalAttr), "true")) {
+ if (equalLettersIgnoringASCIICase(element.attributeWithoutSynchronization(aria_modalAttr), "true")) {
// Add the newly modified node to the modal nodes set, and set it to be the current valid aria modal node.
// We will recompute the current valid aria modal node in ariaModalNode() when this node is not visible.
- m_ariaModalNodesSet.add(node);
- m_currentAriaModalNode = node;
+ m_ariaModalNodesSet.add(&element);
+ m_currentAriaModalNode = &element;
} else {
// Remove the node from the modal nodes set. There might be other visible modal nodes, so we recompute here.
- m_ariaModalNodesSet.remove(node);
+ m_ariaModalNodesSet.remove(&element);
updateCurrentAriaModalNode();
}
if (m_currentAriaModalNode)
@@ -1487,10 +1491,11 @@
startCachingComputedObjectAttributesUntilTreeMutates();
}
-void AXObjectCache::labelChanged(Element* element)
+void AXObjectCache::labelChanged(Element& element)
{
- ASSERT(is<HTMLLabelElement>(*element));
- HTMLElement* correspondingControl = downcast<HTMLLabelElement>(*element).control();
+ ASSERT_WITH_SECURITY_IMPLICATION(!element.needsStyleRecalc());
+ ASSERT(is<HTMLLabelElement>(element));
+ HTMLElement* correspondingControl = downcast<HTMLLabelElement>(element).control();
textChanged(correspondingControl);
}
Modified: trunk/Source/WebCore/accessibility/AXObjectCache.h (216418 => 216419)
--- trunk/Source/WebCore/accessibility/AXObjectCache.h 2017-05-08 15:26:38 UTC (rev 216418)
+++ trunk/Source/WebCore/accessibility/AXObjectCache.h 2017-05-08 15:33:47 UTC (rev 216419)
@@ -164,8 +164,8 @@
void childrenChanged(Node*, Node* newChild = nullptr);
void childrenChanged(RenderObject*, RenderObject* newChild = nullptr);
void childrenChanged(AccessibilityObject*);
- void checkedStateChanged(Node*);
- void selectedChildrenChanged(Node*);
+ void checkedStateChanged(Element&);
+ void selectedChildrenChanged(Element&);
void selectedChildrenChanged(RenderObject*);
// Called by a node when text or a text equivalent (e.g. alt) attribute is changed.
void textChanged(Node*);
@@ -173,17 +173,13 @@
// Called when a node has just been attached, so we can make sure we have the right subclass of AccessibilityObject.
void updateCacheAfterNodeIsAttached(Node*);
- void handleActiveDescendantChanged(Node*);
- void handleAriaRoleChanged(Node*);
void handleFocusedUIElementChanged(Node* oldFocusedNode, Node* newFocusedNode);
void handleScrolledToAnchor(const Node* anchorNode);
- void handleAriaExpandedChange(Node*);
void handleScrollbarUpdate(ScrollView*);
-
- void handleAriaModalChange(Node*);
+
Node* ariaModalNode();
- void handleAttributeChanged(const QualifiedName& attrName, Element*);
+ void handleAttributeChanged(const QualifiedName& attrName, Element&);
void recomputeIsIgnored(RenderObject* renderer);
#if HAVE(ACCESSIBILITY)
@@ -346,7 +342,6 @@
void frameLoadingEventPlatformNotification(AccessibilityObject*, AXLoadingEvent);
void textChanged(AccessibilityObject*);
- void labelChanged(Element*);
// This is a weak reference cache for knowing if Nodes used by TextMarkers are valid.
void setNodeInUse(Node* n) { m_textMarkerNodes.add(n); }
@@ -403,6 +398,12 @@
void updateCurrentAriaModalNode();
bool isNodeVisible(Node*) const;
+ void handleActiveDescendantChanged(Element&);
+ void handleAriaRoleChanged(Element&);
+ void handleAriaExpandedChange(Element&);
+ void handleAriaModalChange(Element&);
+ void labelChanged(Element&);
+
Document& m_document;
HashMap<AXID, RefPtr<AccessibilityObject>> m_objects;
HashMap<RenderObject*, AXID> m_renderObjectMapping;
@@ -474,7 +475,7 @@
inline const Element* AXObjectCache::rootAXEditableElement(const Node*) { return nullptr; }
inline Node* AXObjectCache::ariaModalNode() { return nullptr; }
inline void AXObjectCache::attachWrapper(AccessibilityObject*) { }
-inline void AXObjectCache::checkedStateChanged(Node*) { }
+inline void AXObjectCache::checkedStateChanged(Element&) { }
inline void AXObjectCache::childrenChanged(RenderObject*, RenderObject*) { }
inline void AXObjectCache::childrenChanged(Node*, Node*) { }
inline void AXObjectCache::childrenChanged(AccessibilityObject*) { }
@@ -485,13 +486,13 @@
inline void AXObjectCache::detachWrapper(AccessibilityObject*, AccessibilityDetachmentType) { }
inline void AXObjectCache::frameLoadingEventNotification(Frame*, AXLoadingEvent) { }
inline void AXObjectCache::frameLoadingEventPlatformNotification(AccessibilityObject*, AXLoadingEvent) { }
-inline void AXObjectCache::handleActiveDescendantChanged(Node*) { }
-inline void AXObjectCache::handleAriaExpandedChange(Node*) { }
-inline void AXObjectCache::handleAriaRoleChanged(Node*) { }
-inline void AXObjectCache::handleAriaModalChange(Node*) { }
+inline void AXObjectCache::handleActiveDescendantChanged(Element&) { }
+inline void AXObjectCache::handleAriaExpandedChange(Element&) { }
+inline void AXObjectCache::handleAriaRoleChanged(Element&) { }
+inline void AXObjectCache::handleAriaModalChange(Element&) { }
inline void AXObjectCache::handleFocusedUIElementChanged(Node*, Node*) { }
inline void AXObjectCache::handleScrollbarUpdate(ScrollView*) { }
-inline void AXObjectCache::handleAttributeChanged(const QualifiedName&, Element*) { }
+inline void AXObjectCache::handleAttributeChanged(const QualifiedName&, Element&) { }
inline void AXObjectCache::recomputeIsIgnored(RenderObject*) { }
inline void AXObjectCache::recomputeDeferredIsIgnored(RenderBlock&) { }
inline void AXObjectCache::deferTextChanged(RenderText&) { }
@@ -512,7 +513,7 @@
inline void AXObjectCache::remove(Node*) { }
inline void AXObjectCache::remove(Widget*) { }
inline void AXObjectCache::selectedChildrenChanged(RenderObject*) { }
-inline void AXObjectCache::selectedChildrenChanged(Node*) { }
+inline void AXObjectCache::selectedChildrenChanged(Element&) { }
inline void AXObjectCache::setIsSynchronizingSelection(bool) { }
inline void AXObjectCache::setTextSelectionIntent(const AXTextStateChangeIntent&) { }
inline RefPtr<Range> AXObjectCache::rangeForUnorderedCharacterOffsets(const CharacterOffset&, const CharacterOffset&) { return nullptr; }
Modified: trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp (216418 => 216419)
--- trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp 2017-05-08 15:26:38 UTC (rev 216418)
+++ trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp 2017-05-08 15:33:47 UTC (rev 216419)
@@ -2442,7 +2442,7 @@
{
// This object might be deleted under the call to the parentObject() method.
auto protectedThis = makeRef(*this);
-
+
// Find if a parent of this object should handle aria-expanded changes.
AccessibilityObject* containerParent = this->parentObject();
while (containerParent) {
Modified: trunk/Source/WebCore/dom/Element.cpp (216418 => 216419)
--- trunk/Source/WebCore/dom/Element.cpp 2017-05-08 15:26:38 UTC (rev 216418)
+++ trunk/Source/WebCore/dom/Element.cpp 2017-05-08 15:33:47 UTC (rev 216419)
@@ -1343,7 +1343,7 @@
invalidateNodeListAndCollectionCachesInAncestors(&name, this);
if (AXObjectCache* cache = document().existingAXObjectCache())
- cache->handleAttributeChanged(name, this);
+ cache->handleAttributeChanged(name, *this);
}
template <typename CharacterType>
Modified: trunk/Source/WebCore/html/HTMLInputElement.cpp (216418 => 216419)
--- trunk/Source/WebCore/html/HTMLInputElement.cpp 2017-05-08 15:26:38 UTC (rev 216418)
+++ trunk/Source/WebCore/html/HTMLInputElement.cpp 2017-05-08 15:33:47 UTC (rev 216419)
@@ -915,7 +915,7 @@
// because of the way the code is structured.
if (renderer()) {
if (AXObjectCache* cache = renderer()->document().existingAXObjectCache())
- cache->checkedStateChanged(this);
+ cache->checkedStateChanged(*this);
}
// Only send a change event for items in the document (avoid firing during
