Title: [187053] trunk
- Revision
- 187053
- Author
- commit-qu...@webkit.org
- Date
- 2015-07-20 18:08:53 -0700 (Mon, 20 Jul 2015)
Log Message
AX: Selection change as a result of focusing an element may cause Safari to crash
https://bugs.webkit.org/show_bug.cgi?id=147052
<rdar://problem/21778212>
Patch by Nan Wang <n_w...@apple.com> on 2015-07-20
Reviewed by Chris Fleizach.
Source/WebCore:
When focusing an element, it may trigger a deferred layout that invalidates the render
element, which will cause axObjectCache() to be a nullptr, and lead to a crash. Fix that
by using a RefPtr to hold the object and also caching the axObjectCache().
Test: platform/mac/accessibility/focus-crash.html
* accessibility/AccessibilityRenderObject.cpp:
(WebCore::AccessibilityRenderObject::setFocused):
(WebCore::AccessibilityRenderObject::setSelectedRows):
LayoutTests:
* platform/mac/accessibility/focus-crash-expected.txt: Added.
* platform/mac/accessibility/focus-crash.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (187052 => 187053)
--- trunk/LayoutTests/ChangeLog 2015-07-21 00:58:38 UTC (rev 187052)
+++ trunk/LayoutTests/ChangeLog 2015-07-21 01:08:53 UTC (rev 187053)
@@ -1,3 +1,14 @@
+2015-07-20 Nan Wang <n_w...@apple.com>
+
+ AX: Selection change as a result of focusing an element may cause Safari to crash
+ https://bugs.webkit.org/show_bug.cgi?id=147052
+ <rdar://problem/21778212>
+
+ Reviewed by Chris Fleizach.
+
+ * platform/mac/accessibility/focus-crash-expected.txt: Added.
+ * platform/mac/accessibility/focus-crash.html: Added.
+
2015-07-20 Brian J. Burg <b...@cs.washington.edu>
Web Inspector: start using Promises to handle asynchronous steps in protocol tests
Added: trunk/LayoutTests/platform/mac/accessibility/focus-crash-expected.txt (0 => 187053)
--- trunk/LayoutTests/platform/mac/accessibility/focus-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/platform/mac/accessibility/focus-crash-expected.txt 2015-07-21 01:08:53 UTC (rev 187053)
@@ -0,0 +1,10 @@
+This tests that focusing and removing element won't cause crash.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS document.getElementById("toBeRemoved") is null
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/platform/mac/accessibility/focus-crash.html (0 => 187053)
--- trunk/LayoutTests/platform/mac/accessibility/focus-crash.html (rev 0)
+++ trunk/LayoutTests/platform/mac/accessibility/focus-crash.html 2015-07-21 01:08:53 UTC (rev 187053)
@@ -0,0 +1,42 @@
+<!DOCTYPE HTML>
+<html>
+<body>
+<script src=""
+
+<input type="text" id="toBeRemoved" _onfocus_="focusHandler(this)"></input>
+
+
+<script>
+
+var jsTestIsAsync = true;
+
+description("This tests that focusing and removing element won't cause crash.");
+
+if (window.testRunner && window.accessibilityController) {
+
+ // Try to focus on the element.
+ accessibilityController.accessibleElementById("toBeRemoved").takeFocus();
+}
+
+function focusHandler(node)
+{
+ // Make sure we don't crash after removing the element.
+ node.parentNode.removeChild(node);
+ gc();
+ setTimeout("finishTest()", 0);
+}
+
+function finishTest()
+{
+ // Element should be removed while on focus
+ shouldBe("document.getElementById(\"toBeRemoved\")", "null");
+
+ // Make sure accessing the element won't lead to crash
+ var detached = accessibilityController.accessibleElementById("toBeRemoved")
+ finishJSTest();
+}
+</script>
+
+<script src=""
+</body>
+</html>
\ No newline at end of file
Modified: trunk/Source/WebCore/ChangeLog (187052 => 187053)
--- trunk/Source/WebCore/ChangeLog 2015-07-21 00:58:38 UTC (rev 187052)
+++ trunk/Source/WebCore/ChangeLog 2015-07-21 01:08:53 UTC (rev 187053)
@@ -1,3 +1,21 @@
+2015-07-20 Nan Wang <n_w...@apple.com>
+
+ AX: Selection change as a result of focusing an element may cause Safari to crash
+ https://bugs.webkit.org/show_bug.cgi?id=147052
+ <rdar://problem/21778212>
+
+ Reviewed by Chris Fleizach.
+
+ When focusing an element, it may trigger a deferred layout that invalidates the render
+ element, which will cause axObjectCache() to be a nullptr, and lead to a crash. Fix that
+ by using a RefPtr to hold the object and also caching the axObjectCache().
+
+ Test: platform/mac/accessibility/focus-crash.html
+
+ * accessibility/AccessibilityRenderObject.cpp:
+ (WebCore::AccessibilityRenderObject::setFocused):
+ (WebCore::AccessibilityRenderObject::setSelectedRows):
+
2015-07-20 Alex Christensen <achristen...@webkit.org>
Unreviewed build fix after r187049.
Modified: trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp (187052 => 187053)
--- trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp 2015-07-21 00:58:38 UTC (rev 187052)
+++ trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp 2015-07-21 01:08:53 UTC (rev 187053)
@@ -1683,9 +1683,15 @@
if (document->focusedElement() == node)
document->setFocusedElement(nullptr);
- axObjectCache()->setIsSynchronizingSelection(true);
+ // When a node is told to set focus, that can cause it to be deallocated, which means that doing
+ // anything else inside this object will crash. To fix this, we added a RefPtr to protect this object
+ // long enough for duration. We can also locally cache the axObjectCache.
+ RefPtr<AccessibilityObject> protect(this);
+ AXObjectCache* cache = axObjectCache();
+
+ cache->setIsSynchronizingSelection(true);
downcast<Element>(*node).focus();
- axObjectCache()->setIsSynchronizingSelection(false);
+ cache->setIsSynchronizingSelection(false);
}
void AccessibilityRenderObject::setSelectedRows(AccessibilityChildrenVector& selectedRows)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
