Changeset 169105 in webkit


Ignore:
Timestamp:
May 20, 2014, 3:57:58 AM (11 years ago)
Author:
mihnea@adobe.com
Message:

[CSS Regions] Crash while painting block selection gaps in regions
​https://bugs.webkit.org/show_bug.cgi?id=132720

Reviewed by David Hyatt.

Source/WebCore:
The fix for WebKit bug ​https://bugs.webkit.org/show_bug.cgi?id=131511
allowed selection highlight to match the DOM selection when the start
and end point of the selection were in different flow threads. In order
to enable that, the selection was performed separately on view and
render flow threads, considered selection subtrees.

However, the start and end points for each selection subtree were computed
by means of Range class but it is not always possible to construct a valid
Range from two pairs of RenderObjects and offsets.

This patch keeps the substrees approach but instead of storing the endpoints
for each subtree in a Range and continuously extending the Range, it stores them
using the already available SelectionSubtreeRoot class. After the end points are
computed for each subtree and before processing the subtree selection, the end points
are adjusted in a similar fashion as the one used in FrameSelection::updateAppearance(),
to make sure we are passing the same expected information to the method implementing
the visible selection processing.

Test: fast/regions/selection-gaps-paint-crash.html

  • rendering/RenderView.cpp:

(WebCore::RenderView::splitSelectionBetweenSubtrees):

  • rendering/SelectionSubtreeRoot.cpp:

(WebCore::SelectionSubtreeRoot::adjustForVisibleSelection):

  • rendering/SelectionSubtreeRoot.h:

(WebCore::SelectionSubtreeRoot::selectionClear):

LayoutTests:

  • TestExpectations: Unskip a test that was crashing
  • fast/regions/selection-gaps-paint-crash-expected.txt: Added.
  • fast/regions/selection-gaps-paint-crash.html: Added.
Location:
trunk
Files:
2 added
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r169104 r169105  
     12014-05-20  Mihnea Ovidenie  <mihnea@adobe.com>
     2
     3        [CSS Regions] Crash while painting block selection gaps in regions
     4        https://bugs.webkit.org/show_bug.cgi?id=132720
     5
     6        Reviewed by David Hyatt.
     7
     8        * TestExpectations: Unskip a test that was crashing
     9        * fast/regions/selection-gaps-paint-crash-expected.txt: Added.
     10        * fast/regions/selection-gaps-paint-crash.html: Added.
     11
    1122014-05-20  Lorenzo Tilve  <ltilve@igalia.com>
    213

  • trunk/LayoutTests/TestExpectations

    r168630 r169105  
    116116css2.1/20110323/replaced-intrinsic-002.htm [ Failure ]
    117117
    118 # Regression from r167652:
    119 webkit.org/b/131982 [ Debug ] fast/regions/cssom/region-range-for-box-crash.html [ Crash ]
    120118
    121119webkit.org/b/132791 svg/as-object/sizing/svg-in-object-placeholder-height-fixed.html [ Skip ]

  • trunk/Source/WebCore/ChangeLog

    r169097 r169105  
     12014-05-20  Mihnea Ovidenie  <mihnea@adobe.com>
     2
     3        [CSS Regions] Crash while painting block selection gaps in regions
     4        https://bugs.webkit.org/show_bug.cgi?id=132720
     5
     6        Reviewed by David Hyatt.
     7
     8        The fix for WebKit bug https://bugs.webkit.org/show_bug.cgi?id=131511
     9        allowed selection highlight to match the DOM selection when the start
     10        and end point of the selection were in different flow threads. In order
     11        to enable that, the selection was performed separately on view and
     12        render flow threads, considered selection subtrees.
     13
     14        However, the start and end points for each selection subtree were computed
     15        by means of Range class but it is not always possible to construct a valid
     16        Range from two pairs of RenderObjects and offsets.
     17
     18        This patch keeps the substrees approach but instead of storing the endpoints
     19        for each subtree in a Range and continuously extending the Range, it stores them
     20        using the already available SelectionSubtreeRoot class. After the end points are
     21        computed for each subtree and before processing the subtree selection, the end points
     22        are adjusted in a similar fashion as the one used in FrameSelection::updateAppearance(),
     23        to make sure we are passing the same expected information to the method implementing
     24        the visible selection processing.
     25
     26        Test: fast/regions/selection-gaps-paint-crash.html
     27
     28        * rendering/RenderView.cpp:
     29        (WebCore::RenderView::splitSelectionBetweenSubtrees):
     30        * rendering/SelectionSubtreeRoot.cpp:
     31        (WebCore::SelectionSubtreeRoot::adjustForVisibleSelection):
     32        * rendering/SelectionSubtreeRoot.h:
     33        (WebCore::SelectionSubtreeRoot::selectionClear):
     34
    1352014-05-19  Simon Fraser  <simon.fraser@apple.com>
    236

  • trunk/Source/WebCore/rendering/RenderView.cpp

    r168993 r169105  
    868868void RenderView::splitSelectionBetweenSubtrees(RenderObject* start, int startPos, RenderObject* end, int endPos, SelectionRepaintMode blockRepaintMode)
    869869{
    870     // Get ranges.
    871     typedef HashMap<SelectionSubtreeRoot*, RefPtr<Range> > RenderSubtreesMap;
     870    // Compute the visible selection end points for each of the subtrees.
     871    typedef HashMap<SelectionSubtreeRoot*, SelectionSubtreeRoot> RenderSubtreesMap;
    872872    RenderSubtreesMap renderSubtreesMap;
    873873
    874     // Initialize map for RenderView and all RenderNamedFlowThreads.
    875     renderSubtreesMap.set(this, nullptr);
     874    SelectionSubtreeRoot initialSelection;
     875    renderSubtreesMap.set(this, initialSelection);
    876876    for (auto* namedFlowThread : *flowThreadController().renderNamedFlowThreadList())
    877         renderSubtreesMap.set(namedFlowThread, nullptr);
     877        renderSubtreesMap.set(namedFlowThread, initialSelection);
    878878
    879879    if (start && end) {
    880         RefPtr<Range> initialRange = Range::create(document(), start->node(), startPos, end->node(), endPos);
    881 
    882         Node* startNode = initialRange->startContainer();
    883         Node* endNode = initialRange->endContainer();
    884         Node* stopNode = initialRange->pastLastNode();
     880        Node* startNode = start->node();
     881        Node* endNode = end->node();
     882        Node* stopNode = NodeTraversal::nextSkippingChildren(endNode);
    885883
    886884        for (Node* node = startNode; node != stopNode; node = NodeTraversal::next(node)) {
    … …  
    890888
    891889            SelectionSubtreeRoot& root = renderer->selectionRoot();
    892             RefPtr<Range> range = renderSubtreesMap.get(&root);
    893             if (!range) {
    894                 range = Range::create(document());
    895 
    896                 if (node == startNode)
    897                     range->setStart(node, startPos);
    898                 else
    899                     range->setStart(node, 0);
    900 
    901                 renderSubtreesMap.set(&root, range);
     890            SelectionSubtreeRoot selectionData = renderSubtreesMap.get(&root);
     891            if (selectionData.selectionClear()) {
     892                selectionData.setSelectionStart(node->renderer());
     893                selectionData.setSelectionStartPos(node == startNode ? startPos : 0);
    902894            }
    903895
     896            selectionData.setSelectionEnd(node->renderer());
    904897            if (node == endNode)
    905                 range->setEnd(node, endPos);
     898                selectionData.setSelectionEndPos(endPos);
    906899            else
    907                 range->setEnd(node, node->offsetInCharacters() ? node->maxCharacterOffset() : node->childNodeCount());
     900                selectionData.setSelectionEndPos(node->offsetInCharacters() ? node->maxCharacterOffset() : node->childNodeCount());
     901
     902            renderSubtreesMap.set(&root, selectionData);
    908903        }
    909904    }
    910905
    911906    for (RenderSubtreesMap::iterator i = renderSubtreesMap.begin(); i != renderSubtreesMap.end(); ++i) {
    912         RefPtr<Range> range = i->value;
    913 
    914         RenderObject* newStart;
    915         int newStartPos;
    916         RenderObject* newEnd;
    917         int newEndPos;
    918 
    919         if (range) {
    920             newStart = range->startContainer()->renderer();
    921             newStartPos = range->startOffset();
    922             newEnd = range->endContainer()->renderer();
    923             newEndPos = range->endOffset();
    924         } else {
    925             newStart = 0;
    926             newStartPos = -1;
    927             newEnd = 0;
    928             newEndPos = -1;
    929         }
    930 
    931         setSubtreeSelection(*i->key, newStart, newStartPos, newEnd, newEndPos, blockRepaintMode);
     907        SelectionSubtreeRoot subtreeSelectionData = i->value;
     908        subtreeSelectionData.adjustForVisibleSelection(document());
     909        setSubtreeSelection(*i->key, subtreeSelectionData.selectionStart(), subtreeSelectionData.selectionStartPos(),
     910            subtreeSelectionData.selectionEnd(), subtreeSelectionData.selectionEndPos(), blockRepaintMode);
    932911    }
    933912}

  • trunk/Source/WebCore/rendering/SelectionSubtreeRoot.cpp

    r167652 r169105  
    3131#include "SelectionSubtreeRoot.h"
    3232
     33#include "Document.h"
     34#include "Position.h"
     35#include "Range.h"
     36#include "VisibleSelection.h"
     37
    3338namespace WebCore {
    3439
    … …  
    4752}
    4853
     54void SelectionSubtreeRoot::adjustForVisibleSelection(Document& document)
     55{
     56    if (selectionClear())
     57        return;
     58
     59    // Create a range based on the cached end points
     60    Position startPosition = createLegacyEditingPosition(m_selectionStart->node(), m_selectionStartPos);
     61    Position endPosition = createLegacyEditingPosition(m_selectionEnd->node(), m_selectionEndPos);
     62
     63    RefPtr<Range> range = Range::create(document, startPosition.parentAnchoredEquivalent(), endPosition.parentAnchoredEquivalent());
     64    VisibleSelection selection(range.get());
     65    Position startPos = selection.start();
     66    Position candidate = startPos.downstream();
     67    if (candidate.isCandidate())
     68        startPos = candidate;
     69
     70    Position endPos = selection.end();
     71    candidate = endPos.upstream();
     72    if (candidate.isCandidate())
     73        endPos = candidate;
     74
     75    m_selectionStart = nullptr;
     76    m_selectionStartPos = -1;
     77    m_selectionEnd = nullptr;
     78    m_selectionEndPos = -1;
     79
     80    if (startPos.isNotNull() && endPos.isNotNull() && selection.visibleStart() != selection.visibleEnd()) {
     81        m_selectionStart = startPos.deprecatedNode()->renderer();
     82        m_selectionStartPos = startPos.deprecatedEditingOffset();
     83        m_selectionEnd = endPos.deprecatedNode()->renderer();
     84        m_selectionEndPos = endPos.deprecatedEditingOffset();
     85    }
     86}
     87
    4988} // namespace WebCore

  • trunk/Source/WebCore/rendering/SelectionSubtreeRoot.h

    r167652 r169105  
    3535namespace WebCore {
    3636
     37class Document;
     38
    3739class SelectionSubtreeRoot {
    3840public:
    … …  
    4446    RenderObject* selectionEnd() const { return m_selectionEnd; }
    4547    int selectionEndPos() const { return m_selectionEndPos; }
     48    void selectionStartEndPositions(int& startPos, int& endPos) const;
     49    bool selectionClear() const
     50    {
     51        return !m_selectionStart
     52        && (m_selectionStartPos == -1)
     53        && !m_selectionEnd
     54        && (m_selectionEndPos == -1);
     55    }
    4656
    4757    void setSelectionStart(RenderObject* selectionStart) { m_selectionStart = selectionStart; }
    … …  
    5060    void setSelectionEndPos(int selectionEndPos) { m_selectionEndPos = selectionEndPos; }
    5161
    52     void selectionStartEndPositions(int& startPos, int& endPos) const;
     62    void adjustForVisibleSelection(Document&);
    5363
    5464private:
Note: See TracChangeset for help on using the changeset viewer.