Changeset 181685 in webkit

Timestamp:

Mar 17, 2015, 11:09:56 PM (11 years ago)

Author:

commit-queue@webkit.org

Message:

Unreviewed, rolling out r181667, r181682, and r181683.
​https://bugs.webkit.org/show_bug.cgi?id=142812

Broke multiple tests with ASan, plus dependent commits
(Requested by ap on #webkit).

Reverted changesets:

"Simple line layout: Split fragments on renderer boundary on
the fly."
​https://bugs.webkit.org/show_bug.cgi?id=142579
​http://trac.webkit.org/changeset/181667

"Simple line layout: Change FlowContents::segmentForPosition()
to segmentForRun()."
​https://bugs.webkit.org/show_bug.cgi?id=142785
​http://trac.webkit.org/changeset/181682

"Simple line layout: Use Vector<>::const_iterator instead of
custom FlowContents::Iterator."
​https://bugs.webkit.org/show_bug.cgi?id=142809
​http://trac.webkit.org/changeset/181683

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/text/simple-line-with-multiple-renderers-expected.html (deleted)
• LayoutTests/fast/text/simple-line-with-multiple-renderers.html (deleted)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/SimpleLineLayout.cpp (modified) (11 diffs)
• Source/WebCore/rendering/SimpleLineLayoutFlowContents.cpp (modified) (1 diff)
• Source/WebCore/rendering/SimpleLineLayoutFlowContents.h (modified) (1 diff)
• Source/WebCore/rendering/SimpleLineLayoutFunctions.cpp (modified) (1 diff)
• Source/WebCore/rendering/SimpleLineLayoutResolver.cpp (modified) (1 diff)
• Source/WebCore/rendering/SimpleLineLayoutTextFragmentIterator.cpp (modified) (3 diffs)
• Source/WebCore/rendering/SimpleLineLayoutTextFragmentIterator.h (modified) (4 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2015-03-17  Commit Queue  <commit-queue@webkit.org>
+
+        Unreviewed, rolling out r181667, r181682, and r181683.
+        https://bugs.webkit.org/show_bug.cgi?id=142812
+
+        Broke multiple tests with ASan, plus dependent commits
+        (Requested by ap on #webkit).
+
+        Reverted changesets:
+
+        "Simple line layout: Split fragments on renderer boundary on
+        the fly."
+        https://bugs.webkit.org/show_bug.cgi?id=142579
+        http://trac.webkit.org/changeset/181667
+
+        "Simple line layout: Change FlowContents::segmentForPosition()
+        to segmentForRun()."
+        https://bugs.webkit.org/show_bug.cgi?id=142785
+        http://trac.webkit.org/changeset/181682
+
+        "Simple line layout: Use Vector<>::const_iterator instead of
+        custom FlowContents::Iterator."
+        https://bugs.webkit.org/show_bug.cgi?id=142809
+        http://trac.webkit.org/changeset/181683
+
 2015-03-17  Alexey Proskuryakov  <ap@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2015-03-17  Commit Queue  <commit-queue@webkit.org>
+
+        Unreviewed, rolling out r181667, r181682, and r181683.
+        https://bugs.webkit.org/show_bug.cgi?id=142812
+
+        Broke multiple tests with ASan, plus dependent commits
+        (Requested by ap on #webkit).
+
+        Reverted changesets:
+
+        "Simple line layout: Split fragments on renderer boundary on
+        the fly."
+        https://bugs.webkit.org/show_bug.cgi?id=142579
+        http://trac.webkit.org/changeset/181667
+
+        "Simple line layout: Change FlowContents::segmentForPosition()
+        to segmentForRun()."
+        https://bugs.webkit.org/show_bug.cgi?id=142785
+        http://trac.webkit.org/changeset/181682
+
+        "Simple line layout: Use Vector<>::const_iterator instead of
+        custom FlowContents::Iterator."
+        https://bugs.webkit.org/show_bug.cgi?id=142809
+        http://trac.webkit.org/changeset/181683
+
 2015-03-17  Zalan Bujtas  <zalan@apple.com>
 

trunk/Source/WebCore/rendering/SimpleLineLayout.cpp

@@ -232 +232 @@
 }
 
-static void revertRuns(Layout::RunVector& runs, unsigned length, float width)
-{
-    ASSERT(runs.size());
-    while (length) {
-        Run& lastRun = runs.last();
-        unsigned lastRunLength = lastRun.end - lastRun.start;
-        if (lastRunLength > length) {
-            lastRun.logicalRight -= width;
-            lastRun.end -= length;
-            break;
-        }
-        runs.removeLast();
-        length -= lastRunLength;
-        width -= (lastRun.logicalRight - lastRun.logicalLeft);
-    }
-}
-
 class LineState {
 public:
     void setAvailableWidth(float width) { m_availableWidth = width; }
-    void setCollapedWhitespaceWidth(float width) { m_collapsedWhitespaceWidth = width; }
     void setLogicalLeftOffset(float offset) { m_logicalLeftOffset = offset; }
     void setOverflowedFragment(const TextFragmentIterator::TextFragment& fragment) { m_overflowedFragment = fragment; }
@@ -260 +242 @@
     const TextFragmentIterator::TextFragment& overflowedFragment() const { return m_overflowedFragment; }
     bool hasTrailingWhitespace() const { return m_trailingWhitespaceLength; }
-    TextFragmentIterator::TextFragment lastFragment() const { return m_fragments.last(); }
     bool isWhitespaceOnly() const { return m_trailingWhitespaceWidth && m_runsWidth == m_trailingWhitespaceWidth; }
     bool fits(float extra) const { return m_availableWidth >= m_runsWidth + extra; }
     bool firstCharacterFits() const { return m_firstCharacterFits; }
     float width() const { return m_runsWidth; }
-    bool isEmpty() const
-    {
-        if (!m_fragments.size())
-            return true;
-        if (!m_lastCompleteFragment.isEmpty())
-            return false;
-        return m_fragments.last().overlapsToNextRenderer();
-    }
-
-    void appendFragmentAndCreateRunIfNeeded(const TextFragmentIterator::TextFragment& fragment, Layout::RunVector& runs)
+
+    void appendFragment(const TextFragmentIterator::TextFragment& fragment, Layout::RunVector& runs)
     {
         // Adjust end position while collapsing.
         unsigned endPosition = fragment.isCollapsed() ? fragment.start() + 1 : fragment.end();
-        // New line needs new run.
-        if (!m_runsWidth)
+
+        if (m_createNewRun)
             runs.append(Run(fragment.start(), endPosition, m_runsWidth, m_runsWidth + fragment.width(), false));
         else {
-            const auto& lastFragment = m_fragments.last();
-            // Advance last completed fragment when the previous fragment is all set (including multiple parts across renderers)
-            if ((lastFragment.type() != fragment.type()) || !lastFragment.overlapsToNextRenderer())
-                m_lastCompleteFragment = lastFragment;
-            // Collapse neighbouring whitespace, if they are across multiple renderers and are not collapsed yet.
-            if (lastFragment.isCollapsible() && fragment.isCollapsible()) {
-                ASSERT(lastFragment.isLastInRenderer());
-                if (!lastFragment.isCollapsed()) {
-                    // Line width needs to be reset so that now it takes collapsing into consideration.
-                    m_runsWidth -= (lastFragment.width() - m_collapsedWhitespaceWidth);
-                }
-                // This fragment is collapsed completely. No run is needed.
-                return;
-            }
-            if (lastFragment.isLastInRenderer() || lastFragment.isCollapsed())
-                runs.append(Run(fragment.start(), endPosition, m_runsWidth, m_runsWidth + fragment.width(), false));
-            else {
-                Run& lastRun = runs.last();
-                lastRun.end = endPosition;
-                lastRun.logicalRight += fragment.width();
-            }
-        }
-        m_fragments.append(fragment);
+            ASSERT(runs.size());
+            Run& lastRun = runs.last();
+            lastRun.end = endPosition;
+            lastRun.logicalRight = m_runsWidth + fragment.width();
+        }
+        m_createNewRun = fragment.isCollapsed();
         m_runsWidth += fragment.width();
 
@@ -319 +275 @@
     }
 
-    TextFragmentIterator::TextFragment revertToLastCompleteFragment(Layout::RunVector& runs)
-    {
-        ASSERT(m_fragments.size());
-        unsigned revertLength = 0;
-        float revertWidth = 0;
-        while (m_fragments.size()) {
-            const auto& current = m_fragments.last();
-            if (current == m_lastCompleteFragment)
-                break;
-            revertLength += current.end() - current.start();
-            revertWidth += current.width();
-            m_fragments.removeLast();
-        }
-        m_runsWidth -= revertWidth;
-        if (revertLength)
-            revertRuns(runs, revertLength, revertWidth);
-        return m_lastCompleteFragment;
-    }
-
     void removeTrailingWhitespace(Layout::RunVector& runs)
     {
-        // FIXME: sync m_fragment, but it's ok for now as we don't use m_fragment anymore after removeTrailingWhitespace.
-        if (!m_trailingWhitespaceLength)
-            return;
-        revertRuns(runs, m_trailingWhitespaceLength, m_trailingWhitespaceWidth);
+        ASSERT(runs.size());
+        Run& lastRun = runs.last();
+        lastRun.logicalRight -= m_trailingWhitespaceWidth;
+        lastRun.end -= m_trailingWhitespaceLength;
+        if (lastRun.start == lastRun.end)
+            runs.removeLast();
+
         m_runsWidth -= m_trailingWhitespaceWidth;
+        m_trailingWhitespaceWidth = 0;
         m_trailingWhitespaceLength = 0;
-        m_trailingWhitespaceWidth = 0;
     }
 
@@ -354 +294 @@
     TextFragmentIterator::TextFragment m_overflowedFragment;
     float m_runsWidth { 0 };
-    TextFragmentIterator::TextFragment m_lastCompleteFragment;
+    bool m_createNewRun { true };
     float m_trailingWhitespaceWidth { 0 }; // Use this to remove trailing whitespace without re-mesuring the text.
     unsigned m_trailingWhitespaceLength { 0 };
-    float m_collapsedWhitespaceWidth { 0 };
     // Having one character on the line does not necessarily mean it actually fits.
     // First character of the first fragment might be forced on to the current line even if it does not fit.
     bool m_firstCharacterFits { false };
-    Vector<TextFragmentIterator::TextFragment> m_fragments;
 };
 
@@ -452 +390 @@
 
     // Check if we need to skip the leading whitespace.
-    if (style.collapseWhitespace) {
-        while (firstFragment.type() == TextFragmentIterator::TextFragment::Whitespace)
-            firstFragment = textFragmentIterator.nextTextFragment();
-    }
+    if (style.collapseWhitespace && firstFragment.type() == TextFragmentIterator::TextFragment::Whitespace)
+        firstFragment = textFragmentIterator.nextTextFragment();
     return firstFragment;
 }
 
-static void forceFragmentToLine(LineState& line, TextFragmentIterator& textFragmentIterator, Layout::RunVector& runs, const TextFragmentIterator::TextFragment& fragment)
-{
-    line.appendFragmentAndCreateRunIfNeeded(fragment, runs);
-    // Check if there are more fragments to add to the current line.
-    if (!fragment.overlapsToNextRenderer())
-        return;
-    auto nextFragment = textFragmentIterator.nextTextFragment();
-    while (true) {
-        if (nextFragment.type() != fragment.type())
-            break;
-        line.appendFragmentAndCreateRunIfNeeded(nextFragment, runs);
-        // Does it overlap to the next segment?
-        if (!nextFragment.overlapsToNextRenderer())
-            return;
-        nextFragment = textFragmentIterator.nextTextFragment();
-    }
-    line.setOverflowedFragment(nextFragment);
-}
-
 static bool createLineRuns(LineState& line, const LineState& previousLine, Layout::RunVector& runs, TextFragmentIterator& textFragmentIterator)
 {
     const auto& style = textFragmentIterator.style();
-    line.setCollapedWhitespaceWidth(style.spaceWidth);
     bool lineCanBeWrapped = style.wrapLines || style.breakWordOnOverflow;
     auto fragment = firstFragment(textFragmentIterator, previousLine);
@@ -489 +405 @@
             // Add the new line fragment only if there's nothing on the line. (otherwise the extra new line character would show up at the end of the content.)
             if (!line.width())
-                line.appendFragmentAndCreateRunIfNeeded(fragment, runs);
+                line.appendFragment(fragment, runs);
             break;
         }
@@ -498 +414 @@
             // 3. First, non-whitespace fragment is either wrapped or kept on the line. (depends on overflow-wrap)
             // 4. Non-whitespace fragment when there's already another fragment on the line gets pushed to the next line.
-            bool emptyLine = line.isEmpty();
+            bool emptyLine = !line.width();
             // Whitespace fragment.
             if (fragment.type() == TextFragmentIterator::TextFragment::Whitespace) {
@@ -504 +420 @@
                     // Split the fragment; (modified)fragment stays on this line, overflowedFragment is pushed to next line.
                     line.setOverflowedFragment(splitFragmentToFitLine(fragment, line.availableWidth() - line.width(), emptyLine, textFragmentIterator));
-                    line.appendFragmentAndCreateRunIfNeeded(fragment, runs);
+                    line.appendFragment(fragment, runs);
                 }
                 // When whitespace collapse is on, whitespace that doesn't fit is simply skipped.
@@ -513 +429 @@
                 // Split the fragment; (modified)fragment stays on this line, overflowedFragment is pushed to next line.
                 line.setOverflowedFragment(splitFragmentToFitLine(fragment, line.availableWidth() - line.width(), emptyLine, textFragmentIterator));
-                line.appendFragmentAndCreateRunIfNeeded(fragment, runs);
+                line.appendFragment(fragment, runs);
                 break;
             }
             // Non-breakable non-whitespace first fragment. Add it to the current line. -it overflows though.
-            ASSERT(fragment.type() == TextFragmentIterator::TextFragment::NonWhitespace);
             if (emptyLine) {
-                forceFragmentToLine(line, textFragmentIterator, runs, fragment);
+                line.appendFragment(fragment, runs);
                 break;
             }
             // Non-breakable non-whitespace fragment when there's already content on the line. Push it to the next line.
-            if (line.lastFragment().overlapsToNextRenderer()) {
-                // Check if this fragment is a continuation of a previous segment. In such cases, we need to remove them all.
-                const auto& currentFragment = line.revertToLastCompleteFragment(runs);
-                textFragmentIterator.revertToFragment(currentFragment);
-                break;
-            }
             line.setOverflowedFragment(fragment);
             break;
         }
-        line.appendFragmentAndCreateRunIfNeeded(fragment, runs);
+        line.appendFragment(fragment, runs);
         // Find the next text fragment.
         fragment = textFragmentIterator.nextTextFragment(line.width());
     }
-    return (fragment.type() == TextFragmentIterator::TextFragment::ContentEnd && line.overflowedFragment().isEmpty()) || line.overflowedFragment().type() == TextFragmentIterator::TextFragment::ContentEnd;
+    return fragment.type() == TextFragmentIterator::TextFragment::ContentEnd && line.overflowedFragment().isEmpty();
 }
 
@@ -558 +467 @@
 }
 
+static void splitRunsAtRendererBoundary(Layout::RunVector& lineRuns, const TextFragmentIterator& textFragmentIterator)
+{
+    // FIXME: We should probably split during run construction instead of as a separate pass.
+    if (lineRuns.isEmpty())
+        return;
+    unsigned runIndex = 0;
+    do {
+        const Run& run = lineRuns.at(runIndex);
+        ASSERT(run.start != run.end);
+        auto& startSegment = textFragmentIterator.segmentForPosition(run.start);
+        if (run.end <= startSegment.end)
+            continue;
+        // This run overlaps multiple renderers. Split it up.
+        // Split run at the renderer's boundary and create a new run for the left side, while use the current run as the right side.
+        float logicalRightOfLeftRun = run.logicalLeft + textFragmentIterator.textWidth(run.start, startSegment.end, run.logicalLeft);
+        lineRuns.insert(runIndex, Run(run.start, startSegment.end, run.logicalLeft, logicalRightOfLeftRun, false));
+        Run& rightSideRun = lineRuns.at(runIndex + 1);
+        rightSideRun.start = startSegment.end;
+        rightSideRun.logicalLeft = logicalRightOfLeftRun;
+    } while (++runIndex < lineRuns.size());
+}
+
 static void createTextRuns(Layout::RunVector& runs, RenderBlockFlow& flow, unsigned& lineCount)
 {
@@ -575 +506 @@
         closeLineEndingAndAdjustRuns(line, runs, previousRunCount, lineCount, textFragmentIterator);
     } while (!isEndOfContent);
+
+    if (flow.firstChild() != flow.lastChild())
+        splitRunsAtRendererBoundary(runs, textFragmentIterator);
 }
 

trunk/Source/WebCore/rendering/SimpleLineLayoutFlowContents.cpp

@@ -52 +52 @@
 }
 
-unsigned FlowContents::segmentIndexForRunSlow(unsigned start, unsigned end) const
+unsigned FlowContents::segmentIndexForPositionSlow(unsigned position) const
 {
-    auto it = std::lower_bound(m_segments.begin(), m_segments.end(), start, [](const Segment& segment, unsigned start) {
-        return segment.end <= start;
+    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());
-    ASSERT_UNUSED(end, end <= it->end);
     auto index = it - m_segments.begin();
     m_lastSegmentIndex = index;

trunk/Source/WebCore/rendering/SimpleLineLayoutFlowContents.h

@@ -44 +44 @@
         const RenderObject& renderer;
     };
-    const Segment& segmentForRun(unsigned start, unsigned end) const;
+    const Segment& segmentForPosition(unsigned) const;
     const Segment& segmentForRenderer(const RenderObject&) const;
 
-    typedef Vector<Segment, 8>::const_iterator Iterator;
-    Iterator begin() const { return m_segments.begin(); }
-    Iterator end() const { return m_segments.end(); }
+    class Iterator {
+    public:
+        Iterator(const FlowContents& flowContents, unsigned segmentIndex)
+            : m_flowContents(flowContents)
+            , m_segmentIndex(segmentIndex)
+        {
+        }
+
+        Iterator& operator++();
+        bool operator==(const Iterator& other) const;
+        bool operator!=(const Iterator& other) const;
+        const Segment& operator*() const { return m_flowContents.m_segments[m_segmentIndex]; }
+
+    private:
+        const FlowContents& m_flowContents;
+        unsigned m_segmentIndex;
+    };
+
+    Iterator begin() const { return Iterator(*this, 0); }
+    Iterator end() const { return Iterator(*this, m_segments.size()); }
+
+    unsigned length() const { return m_segments.last().end; };
+
+    unsigned segmentIndexForPosition(unsigned position) const;
 
 private:
-    unsigned segmentIndexForRunSlow(unsigned start, unsigned end) const;
+    unsigned segmentIndexForPositionSlow(unsigned position) const;
+
     const Vector<Segment, 8> m_segments;
+
     mutable unsigned m_lastSegmentIndex;
 };
 
-inline const FlowContents::Segment& FlowContents::segmentForRun(unsigned start, unsigned end) const
+inline FlowContents::Iterator& FlowContents::Iterator::operator++()
 {
-    ASSERT(start < end);
+    ++m_segmentIndex;
+    return *this;
+}
+
+inline bool FlowContents::Iterator::operator==(const FlowContents::Iterator& other) const
+{
+    return m_segmentIndex == other.m_segmentIndex;
+}
+
+inline bool FlowContents::Iterator::operator!=(const FlowContents::Iterator& other) const
+{
+    return !(*this == other);
+}
+
+inline unsigned FlowContents::segmentIndexForPosition(unsigned position) const
+{
     auto& lastSegment = m_segments[m_lastSegmentIndex];
-    if (lastSegment.start <= start && end <= lastSegment.end)
-        return m_segments[m_lastSegmentIndex];
-    return m_segments[segmentIndexForRunSlow(start, end)];
+    if (lastSegment.start <= position && position < lastSegment.end)
+        return m_lastSegmentIndex;
+    return segmentIndexForPositionSlow(position);
+}
+
+inline const FlowContents::Segment& FlowContents::segmentForPosition(unsigned position) const
+{
+    return m_segments[segmentIndexForPosition(position)];
 }
 

trunk/Source/WebCore/rendering/SimpleLineLayoutFunctions.cpp

@@ -85 +85 @@
         TextRun textRun(run.text());
         textRun.setTabSize(!style.collapseWhiteSpace(), style.tabSize());
-        textRun.setXPos(run.rect().x());
         FloatPoint textOrigin = run.baseline() + paintOffset;
         textOrigin.setY(roundToDevicePixel(LayoutUnit(textOrigin.y()), flow.document().deviceScaleFactor()));

trunk/Source/WebCore/rendering/SimpleLineLayoutResolver.cpp

@@ -77 +77 @@
     auto& resolver = m_iterator.resolver();
     auto& run = m_iterator.simpleRun();
-    ASSERT(run.start < run.end);
-    auto& segment = resolver.m_flowContents.segmentForRun(run.start, run.end);
+    auto& segment = resolver.m_flowContents.segmentForPosition(run.start);
     // We currently split runs on segment boundaries (different RenderObject).
     ASSERT(run.end <= segment.end);

trunk/Source/WebCore/rendering/SimpleLineLayoutTextFragmentIterator.cpp

@@ -49 +49 @@
 TextFragmentIterator::TextFragmentIterator(const RenderBlockFlow& flow)
     : m_flowContents(flow)
-    , m_currentSegment(m_flowContents.begin())
-    , m_lineBreakIterator(m_currentSegment->text, flow.style().locale())
+    , m_lineBreakIterator((*m_flowContents.begin()).text, flow.style().locale())
     , m_style(flow.style())
 {
@@ -62 +61 @@
     // 3. non-whitespace characters.
     // 4. empty, indicating content end.
-    ASSERT(m_currentSegment != m_flowContents.end());
-    if (m_position == m_currentSegment->end)
-        ++m_currentSegment;
-    // Is it content end?
-    if (m_currentSegment == m_flowContents.end())
+    if (isEnd(m_position))
         return TextFragment(m_position, m_position, 0, TextFragment::ContentEnd);
-    unsigned segmentEndPosition = m_currentSegment->end;
+    if (isLineBreak(m_position)) {
+        TextFragment fragment(m_position, m_position + 1, 0, TextFragment::LineBreak);
+        ++m_position;
+        return fragment;
+    }
     unsigned startPosition = m_position;
-    if (isLineBreak(m_position)) {
-        unsigned endPosition = ++m_position;
-        return TextFragment(startPosition, endPosition, 0, TextFragment::LineBreak);
-    }
-    float width = 0;
-    bool overlappingFragment = false;
-    unsigned endPosition = skipToNextPosition(PositionType::NonWhitespace, startPosition, width, xPosition, overlappingFragment);
+    unsigned endPosition = skipToNextPosition(PositionType::NonWhitespace, startPosition);
     ASSERT(startPosition <= endPosition);
-    if (startPosition < endPosition) {
+    if (endPosition > startPosition) {
         bool multipleWhitespace = startPosition + 1 < endPosition;
         bool isCollapsed = multipleWhitespace && m_style.collapseWhitespace;
         bool isBreakable = !isCollapsed && multipleWhitespace;
+        float width = isCollapsed ? m_style.spaceWidth : textWidth(startPosition, endPosition, xPosition);
         m_position = endPosition;
-        return TextFragment(startPosition, endPosition, width, TextFragment::Whitespace, endPosition == segmentEndPosition, false, isCollapsed, m_style.collapseWhitespace, isBreakable);
+        return TextFragment(startPosition, endPosition, width, TextFragment::Whitespace, isCollapsed, isBreakable);
     }
-    endPosition = skipToNextPosition(PositionType::Breakable, startPosition, width, xPosition, overlappingFragment);
+    endPosition = skipToNextPosition(PositionType::Breakable, startPosition + 1);
     m_position = endPosition;
-    return TextFragment(startPosition, endPosition, width, TextFragment::NonWhitespace, endPosition == segmentEndPosition, overlappingFragment, false, false, m_style.breakWordOnOverflow);
+    return TextFragment(startPosition, endPosition, textWidth(startPosition, endPosition, xPosition), TextFragment::NonWhitespace, false, m_style.breakWordOnOverflow);
 }
 
-void TextFragmentIterator::revertToFragment(const TextFragment& fragment)
+float TextFragmentIterator::textWidth(unsigned from, unsigned to, float xPosition) const
 {
-    ASSERT(m_position >= fragment.end());
-    ASSERT(fragment.start() >= 0);
-    // Revert segment first.
-    while (m_currentSegment->start > fragment.start())
-        --m_currentSegment;
-    // TODO: It reverts to the last fragment on the same position, but that's ok for now as we don't need to
-    // differentiate multiple renderers on the same position.
-    m_position = fragment.start();
+    const auto& fromSegment = m_flowContents.segmentForPosition(from);
+    ASSERT(is<RenderText>(fromSegment.renderer));
+    if ((m_style.font.isFixedPitch() && fromSegment.end >= to) || (from == fromSegment.start && to == fromSegment.end))
+        return downcast<RenderText>(fromSegment.renderer).width(from - fromSegment.start, to - from, m_style.font, xPosition, nullptr, nullptr);
+
+    const auto* segment = &fromSegment;
+    float textWidth = 0;
+    unsigned fragmentEnd = 0;
+    while (true) {
+        fragmentEnd = std::min(to, segment->end);
+        textWidth += segment->text.is8Bit() ? runWidth<LChar>(segment->text, from - segment->start, fragmentEnd - segment->start, xPosition + textWidth) :
+            runWidth<UChar>(segment->text, from - segment->start, fragmentEnd - segment->start, xPosition + textWidth);
+        if (fragmentEnd == to)
+            break;
+        from = fragmentEnd;
+        segment = &m_flowContents.segmentForPosition(fragmentEnd);
+    };
+
+    return textWidth;
 }
 
 template <typename CharacterType>
-unsigned TextFragmentIterator::nextBreakablePosition(const FlowContents::Segment& segment, unsigned startPosition)
+static unsigned nextBreakablePosition(LazyLineBreakIterator& lineBreakIterator, const FlowContents::Segment& segment, unsigned startPosition)
 {
-    ASSERT(startPosition < segment.end);
-    if (segment.text.impl() != m_lineBreakIterator.string().impl()) {
-        const String& currentText = m_lineBreakIterator.string();
-        unsigned textLength = currentText.length();
-        UChar lastCharacter = textLength > 0 ? currentText[textLength - 1] : 0;
-        UChar secondToLastCharacter = textLength > 1 ? currentText[textLength - 2] : 0;
-        m_lineBreakIterator.setPriorContext(lastCharacter, secondToLastCharacter);
-        m_lineBreakIterator.resetStringAndReleaseIterator(segment.text, m_style.locale, LineBreakIteratorModeUAX14);
-    }
-    const auto* characters = segment.text.characters<CharacterType>();
-    unsigned segmentLength = segment.end - segment.start;
-    unsigned segmentPosition = startPosition - segment.start;
-    return segment.start + nextBreakablePositionNonLoosely<CharacterType, NBSPBehavior::IgnoreNBSP>(m_lineBreakIterator, characters, segmentLength, segmentPosition);
+    return nextBreakablePositionNonLoosely<CharacterType, NBSPBehavior::IgnoreNBSP>(lineBreakIterator, segment.text.characters<CharacterType>(), segment.end - segment.start, startPosition);
 }
 
 template <typename CharacterType>
-unsigned TextFragmentIterator::nextNonWhitespacePosition(const FlowContents::Segment& segment, unsigned startPosition)
+static unsigned nextNonWhitespacePosition(const FlowContents::Segment& segment, unsigned startPosition, const TextFragmentIterator::Style& style)
 {
-    ASSERT(startPosition < segment.end);
     const auto* text = segment.text.characters<CharacterType>();
     unsigned position = startPosition;
-    for (; position < segment.end; ++position) {
-        auto character = text[position - segment.start];
-        bool isWhitespace = character == ' ' || character == '\t' || (!m_style.preserveNewline && character == '\n');
+    unsigned length = segment.end - segment.start;
+    for (; position < length; ++position) {
+        auto character = text[position];
+        bool isWhitespace = character == ' ' || character == '\t' || (!style.preserveNewline && character == '\n');
         if (!isWhitespace)
             return position;
@@ -135 +128 @@
 }
 
-float TextFragmentIterator::textWidth(unsigned from, unsigned to, float xPosition) const
+unsigned TextFragmentIterator::skipToNextPosition(PositionType positionType, unsigned startPosition) const
 {
-    auto& segment = *m_currentSegment;
-    ASSERT(segment.start <= from && from <= segment.end && segment.start <= to && to <= segment.end);
-    ASSERT(is<RenderText>(segment.renderer));
-    if (m_style.font.isFixedPitch() || (from == segment.start && to == segment.end))
-        return downcast<RenderText>(segment.renderer).width(from - segment.start, to - from, m_style.font, xPosition, nullptr, nullptr);
-    return segment.text.is8Bit() ? runWidth<LChar>(segment, from, to, xPosition) : runWidth<UChar>(segment, from, to, xPosition);
-}
+    if (isEnd(startPosition))
+        return startPosition;
 
-unsigned TextFragmentIterator::skipToNextPosition(PositionType positionType, unsigned startPosition, float& width, float xPosition, bool& overlappingFragment)
-{
-    overlappingFragment = false;
     unsigned currentPosition = startPosition;
-    unsigned nextPosition = currentPosition;
-    // Collapsed whitespace has constant width. Do not measure it.
-    if (positionType == NonWhitespace)
-        nextPosition = m_currentSegment->text.is8Bit() ? nextNonWhitespacePosition<LChar>(*m_currentSegment, currentPosition) : nextNonWhitespacePosition<UChar>(*m_currentSegment, currentPosition);
-    else if (positionType == Breakable) {
-        // nextBreakablePosition returns the same position for certain characters such as hyphens. Call next again with modified position unless it's the end of the segment.
-        nextPosition = m_currentSegment->text.is8Bit() ? nextBreakablePosition<LChar>(*m_currentSegment, currentPosition) : nextBreakablePosition<UChar>(*m_currentSegment, currentPosition);
-        // We need to know whether the word actually finishes at the end of this renderer or not.
-        if (nextPosition == m_currentSegment->end) {
-            const auto nextSegment = m_currentSegment + 1;
-            if (nextSegment != m_flowContents.end())
-                overlappingFragment = nextPosition < (nextSegment->text.is8Bit() ? nextBreakablePosition<LChar>(*nextSegment, nextPosition) : nextBreakablePosition<UChar>(*nextSegment, nextPosition));
-        } else if (nextPosition == currentPosition) {
-            if (++nextPosition < m_currentSegment->end)
-                nextPosition = m_currentSegment->text.is8Bit() ? nextBreakablePosition<LChar>(*m_currentSegment, currentPosition + 1) : nextBreakablePosition<UChar>(*m_currentSegment, currentPosition + 1);
-        }
+    FlowContents::Iterator it(m_flowContents, m_flowContents.segmentIndexForPosition(currentPosition));
+    for (auto end = m_flowContents.end(); it != end; ++it) {
+        auto& segment = *it;
+        unsigned currentPositonRelativeToSegment = currentPosition - segment.start;
+        unsigned nextPositionRelativeToSegment = 0;
+        if (positionType == NonWhitespace) {
+            nextPositionRelativeToSegment = segment.text.is8Bit() ? nextNonWhitespacePosition<LChar>(segment, currentPositonRelativeToSegment, m_style) :
+                nextNonWhitespacePosition<UChar>(segment, currentPositonRelativeToSegment, m_style);
+        } else if (positionType == Breakable) {
+            if (segment.text.impl() != m_lineBreakIterator.string().impl()) {
+                UChar lastCharacter = segment.start > 0 ? characterAt(segment.start - 1) : 0;
+                UChar secondToLastCharacter = segment.start > 1 ? characterAt(segment.start - 2) : 0;
+                m_lineBreakIterator.setPriorContext(lastCharacter, secondToLastCharacter);
+                m_lineBreakIterator.resetStringAndReleaseIterator(segment.text, m_style.locale, LineBreakIteratorModeUAX14);
+            }
+            nextPositionRelativeToSegment = segment.text.is8Bit() ? nextBreakablePosition<LChar>(m_lineBreakIterator, segment, currentPositonRelativeToSegment) :
+                nextBreakablePosition<UChar>(m_lineBreakIterator, segment, currentPositonRelativeToSegment);
+        } else
+            ASSERT_NOT_REACHED();
+        currentPosition = segment.start + nextPositionRelativeToSegment;
+        if (currentPosition < segment.end)
+            break;
     }
-    width = 0;
-    if (nextPosition == currentPosition)
-        return currentPosition;
-    bool measureText = positionType != NonWhitespace || !m_style.collapseWhitespace;
-    if (measureText) {
-        float textWidth = this->textWidth(currentPosition, nextPosition, xPosition);
-        xPosition += textWidth;
-        width += textWidth;
-    } else if (startPosition < nextPosition)
-        width = m_style.spaceWidth;
-    return nextPosition;
+    return currentPosition;
 }
 
 template <typename CharacterType>
-float TextFragmentIterator::runWidth(const FlowContents::Segment& segment, unsigned startPosition, unsigned endPosition, float xPosition) const
+float TextFragmentIterator::runWidth(const String& text, unsigned from, unsigned to, float xPosition) const
 {
-    ASSERT(startPosition <= endPosition);
-    if (startPosition == endPosition)
+    ASSERT(from <= to);
+    if (from == to)
         return 0;
-    unsigned segmentFrom = startPosition - segment.start;
-    unsigned segmentTo = endPosition - segment.start;
-    bool measureWithEndSpace = m_style.collapseWhitespace && segmentTo < segment.text.length() && segment.text[segmentTo] == ' ';
+    bool measureWithEndSpace = m_style.collapseWhitespace && to < text.length() && text[to] == ' ';
     if (measureWithEndSpace)
-        ++segmentTo;
-    TextRun run(segment.text.characters<CharacterType>() + segmentFrom, segmentTo - segmentFrom);
+        ++to;
+    TextRun run(text.characters<CharacterType>() + from, to - from);
     run.setXPos(xPosition);
     run.setTabSize(!!m_style.tabWidth, m_style.tabWidth);

trunk/Source/WebCore/rendering/SimpleLineLayoutTextFragmentIterator.h

@@ -44 +44 @@
         enum Type { ContentEnd, LineBreak, Whitespace, NonWhitespace };
         TextFragment() = default;
-        TextFragment(unsigned start, unsigned end, float width, Type type, bool isLastInRenderer = false, bool overlapsToNextRenderer = false, bool isCollapsed = false, bool isCollapsible = false, bool isBreakable = false)
+        TextFragment(unsigned start, unsigned end, float width, Type type, bool isCollapsed = false, bool isBreakable = false)
             : m_start(start)
             , m_end(end)
+            , m_type(type)
             , m_width(width)
-            , m_type(type)
-            , m_isLastInRenderer(isLastInRenderer)
-            , m_overlapsToNextRenderer(overlapsToNextRenderer)
             , m_isCollapsed(isCollapsed)
-            , m_isCollapsible(isCollapsible)
             , m_isBreakable(isBreakable)
         {
@@ -61 +58 @@
         float width() const { return m_width; }
         Type type() const { return m_type; }
-        bool isLastInRenderer() const { return m_isLastInRenderer; }
-        bool overlapsToNextRenderer() const { return m_overlapsToNextRenderer; }
         bool isCollapsed() const { return m_isCollapsed; }
-        bool isCollapsible() const { return m_isCollapsible; }
         bool isBreakable() const { return m_isBreakable; }
 
         bool isEmpty() const { return start() == end(); }
         TextFragment split(unsigned splitPosition, const TextFragmentIterator&);
-        bool operator==(const TextFragment& other) const
-        {
-            return m_start == other.m_start
-                && m_end == other.m_end
-                && m_width == other.m_width
-                && m_type == other.m_type
-                && m_isLastInRenderer == other.m_isLastInRenderer
-                && m_overlapsToNextRenderer == other.m_overlapsToNextRenderer
-                && m_isCollapsed == other.m_isCollapsed
-                && m_isCollapsible == other.m_isCollapsible
-                && m_isBreakable == other.m_isBreakable;
-        }
 
     private:
         unsigned m_start { 0 };
         unsigned m_end { 0 };
+        Type m_type { NonWhitespace };
         float m_width { 0 };
-        Type m_type { NonWhitespace };
-        bool m_isLastInRenderer { false };
-        bool m_overlapsToNextRenderer { false };
         bool m_isCollapsed { false };
-        bool m_isCollapsible { false };
         bool m_isBreakable { false };
     };
     TextFragment nextTextFragment(float xPosition = 0);
-    void revertToFragment(const TextFragment&);
-    float textWidth(unsigned startPosition, unsigned endPosition, float xPosition) const;
+    float textWidth(unsigned from, unsigned to, float xPosition) const;
 
     struct Style {
@@ -111 +89 @@
     };
     const Style& style() const { return m_style; }
+    // FIXME: remove splitRunsAtRendererBoundary()
+    const FlowContents::Segment& segmentForPosition(unsigned position) const { return m_flowContents.segmentForPosition(position); };
 
 private:
     enum PositionType { Breakable, NonWhitespace };
-    unsigned skipToNextPosition(PositionType, unsigned startPosition, float& width, float xPosition, bool& overlappingFragment);
+    unsigned skipToNextPosition(PositionType, unsigned startPosition) const;
+    UChar characterAt(unsigned position) const;
     bool isLineBreak(unsigned position) const;
-    template <typename CharacterType> unsigned nextBreakablePosition(const FlowContents::Segment&, unsigned startPosition);
-    template <typename CharacterType> unsigned nextNonWhitespacePosition(const FlowContents::Segment&, unsigned startPosition);
-    template <typename CharacterType> float runWidth(const FlowContents::Segment&, unsigned startPosition, unsigned endPosition, float xPosition) const;
+    bool isEnd(unsigned position) const;
+    template <typename CharacterType> float runWidth(const String&, unsigned from, unsigned to, float xPosition) const;
 
     FlowContents m_flowContents;
-    FlowContents::Iterator m_currentSegment;
-    LazyLineBreakIterator m_lineBreakIterator;
+    mutable LazyLineBreakIterator m_lineBreakIterator;
     const Style m_style;
     unsigned m_position { 0 };
@@ -149 +128 @@
 }
 
+inline UChar TextFragmentIterator::characterAt(unsigned position) const
+{
+    auto& segment = m_flowContents.segmentForPosition(position);
+    return segment.text[position - segment.start];
+}
+
 inline bool TextFragmentIterator::isLineBreak(unsigned position) const
 {
-    const auto& segment = *m_currentSegment;
-    ASSERT(segment.start <= position && position < segment.end);
-    return m_style.preserveNewline && segment.text[position - segment.start] == '\n';
+    if (isEnd(position))
+        return false;
+    return m_style.preserveNewline && characterAt(position) == '\n';
+}
+
+inline bool TextFragmentIterator::isEnd(unsigned position) const
+{
+    return position >= m_flowContents.length();
 }