Title: [192433] trunk
Revision
192433
Author
jiewen_...@apple.com
Date
2015-11-13 10:36:31 -0800 (Fri, 13 Nov 2015)

Log Message

Element::focus() should acquire the ownership of Frame.
https://bugs.webkit.org/show_bug.cgi?id=150204
<rdar://problem/23136794>

Reviewed by Brent Fulgham.

Source/WebCore:

The FrameSelection::setSelection method sometimes releases the last reference to a frame.
When this happens, the Element::updateFocusAppearance would attempt to use dereferenced memory.
Instead, we should ensure that the Frame lifetime is guaranteed to extend through the duration
of the method call.

Test: editing/selection/focus-iframe-removal-crash.html

* dom/Element.cpp:
(WebCore::Element::updateFocusAppearance):

LayoutTests:

* editing/selection/focus-iframe-removal-crash-expected.txt: Added.
* editing/selection/focus-iframe-removal-crash.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (192432 => 192433)


--- trunk/LayoutTests/ChangeLog	2015-11-13 18:11:42 UTC (rev 192432)
+++ trunk/LayoutTests/ChangeLog	2015-11-13 18:36:31 UTC (rev 192433)
@@ -1,3 +1,14 @@
+2015-11-13  Jiewen Tan  <jiewen_...@apple.com>
+
+        Element::focus() should acquire the ownership of Frame.
+        https://bugs.webkit.org/show_bug.cgi?id=150204
+        <rdar://problem/23136794>
+
+        Reviewed by Brent Fulgham.
+
+        * editing/selection/focus-iframe-removal-crash-expected.txt: Added.
+        * editing/selection/focus-iframe-removal-crash.html: Added.
+
 2015-11-13  Tim Horton  <timothy_hor...@apple.com>
 
         Hardware keyboard spacebar scrolls too far on iOS

Added: trunk/LayoutTests/editing/selection/focus-iframe-removal-crash-expected.txt (0 => 192433)


--- trunk/LayoutTests/editing/selection/focus-iframe-removal-crash-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/selection/focus-iframe-removal-crash-expected.txt	2015-11-13 18:36:31 UTC (rev 192433)
@@ -0,0 +1 @@
+PASS. WebKit didn't crash.

Added: trunk/LayoutTests/editing/selection/focus-iframe-removal-crash.html (0 => 192433)


--- trunk/LayoutTests/editing/selection/focus-iframe-removal-crash.html	                        (rev 0)
+++ trunk/LayoutTests/editing/selection/focus-iframe-removal-crash.html	2015-11-13 18:36:31 UTC (rev 192433)
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+<body>
+<iframe></iframe>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+function run()
+{
+    var iframe = document.getElementsByTagName('iframe')[0];
+    iframe.contentDocument.documentElement.contentEditable = true;
+    iframe.contentDocument.documentElement.addEventListener('focusout', function () {
+        iframe.parentNode.removeChild(iframe);
+    }, false);
+    iframe.contentDocument.documentElement.focus();
+
+    document.write("PASS. WebKit didn't crash.");
+}
+
+document.addEventListener('DOMContentLoaded', run);
+</script>
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (192432 => 192433)


--- trunk/Source/WebCore/ChangeLog	2015-11-13 18:11:42 UTC (rev 192432)
+++ trunk/Source/WebCore/ChangeLog	2015-11-13 18:36:31 UTC (rev 192433)
@@ -1,3 +1,21 @@
+2015-11-13  Jiewen Tan  <jiewen_...@apple.com>
+
+        Element::focus() should acquire the ownership of Frame.
+        https://bugs.webkit.org/show_bug.cgi?id=150204
+        <rdar://problem/23136794>
+
+        Reviewed by Brent Fulgham.
+
+        The FrameSelection::setSelection method sometimes releases the last reference to a frame.
+        When this happens, the Element::updateFocusAppearance would attempt to use dereferenced memory.
+        Instead, we should ensure that the Frame lifetime is guaranteed to extend through the duration
+        of the method call.
+
+        Test: editing/selection/focus-iframe-removal-crash.html
+
+        * dom/Element.cpp:
+        (WebCore::Element::updateFocusAppearance):
+
 2015-11-13  Sergio Villar Senin  <svil...@igalia.com>
 
         [css-grid] Remove unused GridResolvedPosition constructor

Modified: trunk/Source/WebCore/dom/Element.cpp (192432 => 192433)


--- trunk/Source/WebCore/dom/Element.cpp	2015-11-13 18:11:42 UTC (rev 192432)
+++ trunk/Source/WebCore/dom/Element.cpp	2015-11-13 18:36:31 UTC (rev 192433)
@@ -2252,7 +2252,8 @@
 void Element::updateFocusAppearance(SelectionRestorationMode, SelectionRevealMode revealMode)
 {
     if (isRootEditableElement()) {
-        Frame* frame = document().frame();
+        // Keep frame alive in this method, since setSelection() may release the last reference to |frame|.
+        RefPtr<Frame> frame = document().frame();
         if (!frame)
             return;
         
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to