Title: [187093] branches/safari-601.1-branch

Diff

Modified: branches/safari-601.1-branch/LayoutTests/ChangeLog (187092 => 187093)


--- branches/safari-601.1-branch/LayoutTests/ChangeLog	2015-07-21 05:27:51 UTC (rev 187092)
+++ branches/safari-601.1-branch/LayoutTests/ChangeLog	2015-07-21 05:27:55 UTC (rev 187093)
@@ -1,5 +1,20 @@
 2015-07-20  Matthew Hanson  <[email protected]>
 
+        Merge r187053. rdar://problem/21778212
+
+    2015-07-20  Nan Wang  <[email protected]>
+
+            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  Matthew Hanson  <[email protected]>
+
         Merge r186955. rdar://problem/14489674
 
     2015-07-16  Mark Lam  <[email protected]>

Added: branches/safari-601.1-branch/LayoutTests/platform/mac/accessibility/focus-crash-expected.txt (0 => 187093)


--- branches/safari-601.1-branch/LayoutTests/platform/mac/accessibility/focus-crash-expected.txt	                        (rev 0)
+++ branches/safari-601.1-branch/LayoutTests/platform/mac/accessibility/focus-crash-expected.txt	2015-07-21 05:27:55 UTC (rev 187093)
@@ -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: branches/safari-601.1-branch/LayoutTests/platform/mac/accessibility/focus-crash.html (0 => 187093)


--- branches/safari-601.1-branch/LayoutTests/platform/mac/accessibility/focus-crash.html	                        (rev 0)
+++ branches/safari-601.1-branch/LayoutTests/platform/mac/accessibility/focus-crash.html	2015-07-21 05:27:55 UTC (rev 187093)
@@ -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: branches/safari-601.1-branch/Source/WebCore/ChangeLog (187092 => 187093)


--- branches/safari-601.1-branch/Source/WebCore/ChangeLog	2015-07-21 05:27:51 UTC (rev 187092)
+++ branches/safari-601.1-branch/Source/WebCore/ChangeLog	2015-07-21 05:27:55 UTC (rev 187093)
@@ -1,5 +1,27 @@
 2015-07-20  Matthew Hanson  <[email protected]>
 
+        Merge r187053. rdar://problem/21778212
+
+    2015-07-20  Nan Wang  <[email protected]>
+
+            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  Matthew Hanson  <[email protected]>
+
         Merge r187044. rdar://problem/21661808
 
     2015-07-20  Jeremy Jones  <[email protected]>

Modified: branches/safari-601.1-branch/Source/WebCore/accessibility/AccessibilityRenderObject.cpp (187092 => 187093)


--- branches/safari-601.1-branch/Source/WebCore/accessibility/AccessibilityRenderObject.cpp	2015-07-21 05:27:51 UTC (rev 187092)
+++ branches/safari-601.1-branch/Source/WebCore/accessibility/AccessibilityRenderObject.cpp	2015-07-21 05:27:55 UTC (rev 187093)
@@ -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
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to