Merge r166457 - Preserve selection end positions in directionOfSelection

<http://webkit.org/b/104813> <rdar://problem/13666417> Reviewed by Brent Fulgham. Merged from Blink (patch by kenrb@chromium.org): https://src.chromium.org/viewvc/blink?revision=150621&view=revision http://crbug.com/164263 VisibleSelection::visibleStart() and VisibleSelection::visibleEnd() can both cause layouts, which has the potential to invalidate any rendertree-based objects. This was causing a problem in FrameSelection::directionOfSelection(), where a reference to a lineBox was being held across a call to visibleEnd(). This patch ensures that the any layout is completed before linebox references are retrieved. Source/WebCore: Test: editing/selection/layout-during-move-selection-crash.html * editing/FrameSelection.cpp: (WebCore::FrameSelection::directionOfSelection): LayoutTests: * editing/selection/layout-during-move-selection-crash-expected.txt: Added. * editing/selection/layout-during-move-selection-crash.html: Added.
WebKit · May 5, 2014 · 3b8d173 · 3b8d173
1 parent 41d51b9
commit 3b8d173
Show file tree

Hide file tree

Showing 5 changed files with 90 additions and 4 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,27 @@
+2014-03-29  David Kilzer  <ddkilzer@apple.com>
+
+        Preserve selection end positions in directionOfSelection
+        <http://webkit.org/b/104813>
+        <rdar://problem/13666417>
+
+        Reviewed by Brent Fulgham.
+
+        Merged from Blink (patch by kenrb@chromium.org):
+        https://src.chromium.org/viewvc/blink?revision=150621&view=revision
+        http://crbug.com/164263
+
+            VisibleSelection::visibleStart() and VisibleSelection::visibleEnd()
+            can both cause layouts, which has the potential to invalidate any
+            rendertree-based objects. This was causing a problem in
+            FrameSelection::directionOfSelection(), where a reference to a
+            lineBox was being held across a call to visibleEnd().
+
+            This patch ensures that the any layout is completed before linebox
+            references are retrieved.
+
+        * editing/selection/layout-during-move-selection-crash-expected.txt: Added.
+        * editing/selection/layout-during-move-selection-crash.html: Added.
+
 2014-03-28  Radu Stavila  <stavila@adobe.com>
 
         In some situations, partial layouts of floating elements produce incorrect results.

diff --git a/LayoutTests/editing/selection/layout-during-move-selection-crash-expected.txt b/LayoutTests/editing/selection/layout-during-move-selection-crash-expected.txt
@@ -0,0 +1,3 @@
+
+button PASS, if no crash or assert in debug
+
diff --git a/LayoutTests/editing/selection/layout-during-move-selection-crash.html b/LayoutTests/editing/selection/layout-during-move-selection-crash.html
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<html>
+<span id=span1></span><button id=button1>button</button>
+<style>
+        .embedStyle { min-height: 13pc; -webkit-transition: 10s; }
+</style>
+<script>
+    function runTest() {
+        div1 = document.createElement("div");
+        document.body.appendChild(div1);
+        embed1 = document.createElementNS("http://www.w3.org/1999/xhtml", "embed");
+        document.body.offsetTop;
+        embed1.setAttribute("class", "embedStyle");
+        span1.appendChild(embed1);
+        embed1.setAttribute("type", "block");
+        document.body.offsetTop;
+        r = document.createRange();
+        r.setStart(button1, 0);
+        r.setEnd(div1, div1.childNodes.length);
+        window.getSelection().addRange(r);
+        o = r.extractContents();
+        document.body.appendChild(o);
+        window.getSelection().modify("move", "Right", "character");
+
+        if (window.testRunner) testRunner.dumpAsText();
+    }
+    document.addEventListener("DOMContentLoaded", runTest, false);
+</script>PASS, if no crash or assert in debug
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,29 @@
+2014-03-29  David Kilzer  <ddkilzer@apple.com>
+
+        Preserve selection end positions in directionOfSelection
+        <http://webkit.org/b/104813>
+        <rdar://problem/13666417>
+
+        Reviewed by Brent Fulgham.
+
+        Merged from Blink (patch by kenrb@chromium.org):
+        https://src.chromium.org/viewvc/blink?revision=150621&view=revision
+        http://crbug.com/164263
+
+            VisibleSelection::visibleStart() and VisibleSelection::visibleEnd()
+            can both cause layouts, which has the potential to invalidate any
+            rendertree-based objects. This was causing a problem in
+            FrameSelection::directionOfSelection(), where a reference to a
+            lineBox was being held across a call to visibleEnd().
+
+            This patch ensures that the any layout is completed before linebox
+            references are retrieved.
+
+        Test: editing/selection/layout-during-move-selection-crash.html
+
+        * editing/FrameSelection.cpp:
+        (WebCore::FrameSelection::directionOfSelection):
+
 2014-03-28  Radu Stavila  <stavila@adobe.com>
 
         In some situations, partial layouts of floating elements produce incorrect results.

diff --git a/Source/WebCore/editing/FrameSelection.cpp b/Source/WebCore/editing/FrameSelection.cpp
@@ -493,10 +493,14 @@ TextDirection FrameSelection::directionOfSelection()
     InlineBox* startBox = 0;
     InlineBox* endBox = 0;
     int unusedOffset;
-    if (m_selection.start().isNotNull())
-        m_selection.visibleStart().getInlineBoxAndOffset(startBox, unusedOffset);
-    if (m_selection.end().isNotNull())
-        m_selection.visibleEnd().getInlineBoxAndOffset(endBox, unusedOffset);
+    // Cache the VisiblePositions because visibleStart() and visibleEnd()
+    // can cause layout, which has the potential to invalidate lineboxes.
+    VisiblePosition startPosition = m_selection.visibleStart();
+    VisiblePosition endPosition = m_selection.visibleEnd();
+    if (startPosition.isNotNull())
+        startPosition.getInlineBoxAndOffset(startBox, unusedOffset);
+    if (endPosition.isNotNull())
+        endPosition.getInlineBoxAndOffset(endBox, unusedOffset);
     if (startBox && endBox && startBox->direction() == endBox->direction())
         return startBox->direction();