Changeset 217019 in webkit

Timestamp:

May 17, 2017 5:29:07 PM (7 years ago)

Author:

Alan Bujtas

Message:

Tighten TextIterator::handleTextNode run-renderer mapping logic.
​https://bugs.webkit.org/show_bug.cgi?id=172174

Reviewed by Antti Koivisto.

Source/WebCore:

This patch ensure that when runs and renderers are getting out of sync
we don't run into problems like webkit.org/b/172113 (where we end up
using incorrect content start/end positions).

• editing/TextIterator.cpp:

(WebCore::TextIterator::handleTextNode):

LayoutTests:

• fast/shadow-dom/slot-crash-expected.txt:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/editing/TextIterator.cpp (modified) (2 diffs)
• Source/WebCore/rendering/SimpleLineLayoutFlowContents.h (modified) (2 diffs)
• Source/WebCore/rendering/SimpleLineLayoutFunctions.cpp (modified) (2 diffs)
• Source/WebCore/rendering/SimpleLineLayoutFunctions.h (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2017-05-17  Zalan Bujtas  <zalan@apple.com>
+
+        Tighten TextIterator::handleTextNode run-renderer mapping logic.
+        https://bugs.webkit.org/show_bug.cgi?id=172174
+
+        Reviewed by Antti Koivisto.
+
+        * fast/shadow-dom/slot-crash-expected.txt:
+
 2017-05-17  John Wilander  <wilander@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-05-17  Zalan Bujtas  <zalan@apple.com>
+
+        Tighten TextIterator::handleTextNode run-renderer mapping logic.
+        https://bugs.webkit.org/show_bug.cgi?id=172174
+
+        Reviewed by Antti Koivisto.
+
+        This patch ensure that when runs and renderers are getting out of sync
+        we don't run into problems like webkit.org/b/172113 (where we end up
+        using incorrect content start/end positions).
+
+        * editing/TextIterator.cpp:
+        (WebCore::TextIterator::handleTextNode):
+
 2017-05-17  John Wilander  <wilander@apple.com>
 

trunk/Source/WebCore/editing/TextIterator.cpp

@@ -649 +649 @@
         auto it = range.begin();
         auto end = range.end();
-        while (it != end && (*it).end() <= (static_cast<unsigned>(m_offset) + m_accumulatedSimpleTextLengthInFlow))
+        auto startPosition = static_cast<unsigned>(m_offset) + m_accumulatedSimpleTextLengthInFlow;
+        while (it != end && (*it).end() <= startPosition)
             ++it;
         if (m_nextRunNeedsWhitespace && rendererText[m_offset - 1] == '\n') {
@@ -665 +666 @@
             return false;
         }
-        const auto run = *it;
+        // If the position we are looking for is to the left of the renderer's first run, it could mean that
+        // the runs and the renderers are out of sync (e.g. we skipped a renderer in between).
+        // Better bail out at this point.
+        auto run = *it;
+        if (run.start() > startPosition) {
+            ASSERT(m_flowRunResolverCache);
+            if (&(rendererForPosition(m_flowRunResolverCache->flowContents(), startPosition)) != &renderer) {
+                ASSERT_NOT_REACHED();
+                return true;
+            }
+        }
         ASSERT(run.end() - run.start() <= rendererText.length());
         // contentStart skips leading whitespace.

trunk/Source/WebCore/rendering/SimpleLineLayoutFlowContents.h

@@ -51 +51 @@
     };
     const Segment& segmentForRun(unsigned start, unsigned end) const;
+    const Segment& segmentForPosition(unsigned) const;
 
     typedef Vector<Segment, 8>::const_iterator Iterator;
@@ -71 +72 @@
 }
 
+inline const FlowContents::Segment& FlowContents::segmentForPosition(unsigned position) const
+{
+    auto it = std::lower_bound(m_segments.begin(), m_segments.end(), position, [](const Segment& segment, unsigned position) {
+        return segment.end <= position;
+    });
+    ASSERT(it != m_segments.end());
+    return m_segments[it - m_segments.begin()];
+}
+
 }
 }

trunk/Source/WebCore/rendering/SimpleLineLayoutFunctions.cpp

@@ -41 +41 @@
 #include "RenderView.h"
 #include "Settings.h"
+#include "SimpleLineLayoutFlowContents.h"
 #include "SimpleLineLayoutResolver.h"
 #include "Text.h"
@@ -255 +256 @@
 }
 
+const RenderObject& rendererForPosition(const FlowContents& flowContents, unsigned position)
+{
+    return flowContents.segmentForPosition(position).renderer;
+}
+
 #if ENABLE(TREE_DEBUGGING)
 static void printPrefix(int& printedCharacters, int depth)

trunk/Source/WebCore/rendering/SimpleLineLayoutFunctions.h

@@ -40 +40 @@
 
 namespace SimpleLineLayout {
+class FlowContents;
 
 LayoutUnit computeFlowHeight(const RenderBlockFlow&, const Layout&);
@@ -64 +65 @@
 LayoutUnit lineHeightFromFlow(const RenderBlockFlow&);
 LayoutUnit baselineFromFlow(const RenderBlockFlow&);
+
+const RenderObject& rendererForPosition(const FlowContents&, unsigned);
 
 #if ENABLE(TREE_DEBUGGING)