InlineTextBox's m_len can be an unsigned (rather than an unsigned short)
authormmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Jun 2014 01:19:25 +0000 (01:19 +0000)
committermmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Jun 2014 01:19:25 +0000 (01:19 +0000)
https://bugs.webkit.org/show_bug.cgi?id=134173

Reviewed by Daniel Bates.

After Zalan's talks with Kling, it seems that the simple line layout code
might alleviate the need for the space savings in InlineTextBox. Given this,
it would be beneficial to be a little more safe by using unsigneds throughout.

For example, we have code like "void setLen(unsigned len) { m_len = len; }"
which might silently break if given particular inputs.

No new tests because there is no behavior change.

* rendering/InlineTextBox.cpp:
(WebCore::InlineTextBox::isSelected):
(WebCore::InlineTextBox::localSelectionRect):
(WebCore::InlineTextBox::paint):
(WebCore::InlineTextBox::selectionStartEnd):
(WebCore::InlineTextBox::paintSelection):
(WebCore::InlineTextBox::paintCompositionBackground):
(WebCore::InlineTextBox::paintDocumentMarker):
(WebCore::InlineTextBox::paintTextMatchMarker):
(WebCore::InlineTextBox::computeRectForReplacementMarker):
* rendering/InlineTextBox.h:
(WebCore::InlineTextBox::truncation):
* rendering/RenderTextLineBoxes.cpp:
(WebCore::ellipsisRectForBox):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@170413 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/rendering/InlineTextBox.cpp
Source/WebCore/rendering/InlineTextBox.h
Source/WebCore/rendering/RenderTextLineBoxes.cpp

index 3c507cc..a0b1a23 100644 (file)
@@ -1,3 +1,34 @@
+2014-06-24  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        InlineTextBox's m_len can be an unsigned (rather than an unsigned short)
+        https://bugs.webkit.org/show_bug.cgi?id=134173
+
+        Reviewed by Daniel Bates.
+
+        After Zalan's talks with Kling, it seems that the simple line layout code
+        might alleviate the need for the space savings in InlineTextBox. Given this,
+        it would be beneficial to be a little more safe by using unsigneds throughout.
+
+        For example, we have code like "void setLen(unsigned len) { m_len = len; }"
+        which might silently break if given particular inputs.
+
+        No new tests because there is no behavior change.
+
+        * rendering/InlineTextBox.cpp:
+        (WebCore::InlineTextBox::isSelected):
+        (WebCore::InlineTextBox::localSelectionRect):
+        (WebCore::InlineTextBox::paint):
+        (WebCore::InlineTextBox::selectionStartEnd):
+        (WebCore::InlineTextBox::paintSelection):
+        (WebCore::InlineTextBox::paintCompositionBackground):
+        (WebCore::InlineTextBox::paintDocumentMarker):
+        (WebCore::InlineTextBox::paintTextMatchMarker):
+        (WebCore::InlineTextBox::computeRectForReplacementMarker):
+        * rendering/InlineTextBox.h:
+        (WebCore::InlineTextBox::truncation):
+        * rendering/RenderTextLineBoxes.cpp:
+        (WebCore::ellipsisRectForBox):
+
 2014-06-24  Ryosuke Niwa  <rniwa@webkit.org>
 
         Speculative 32-bit Mac build fix after r170402.
index c3c96a3..4649bb4 100644 (file)
@@ -59,7 +59,7 @@ namespace WebCore {
 
 struct SameSizeAsInlineTextBox : public InlineBox {
     unsigned variables[1];
-    unsigned short variables2[2];
+    unsigned variables2[2];
     void* pointers[2];
 };
 
@@ -201,7 +201,7 @@ bool InlineTextBox::isSelected(unsigned startPos, unsigned endPos) const
     unsigned sPos = startPos > m_start ? startPos - m_start : 0;
     if (endPos <= m_start)
         return false;
-    unsigned ePos = std::min(endPos - m_start, static_cast<unsigned>(m_len));
+    unsigned ePos = std::min(endPos - m_start, m_len);
     return sPos < ePos;
 }
 
@@ -278,7 +278,7 @@ LayoutRect InlineTextBox::localSelectionRect(unsigned startPos, unsigned endPos)
     if (endPos < m_start)
         return LayoutRect();
 
-    unsigned ePos = std::min(endPos - m_start, static_cast<unsigned>(m_len));
+    unsigned ePos = std::min(endPos - m_start, m_len);
     
     if (sPos > ePos)
         return LayoutRect();
@@ -640,8 +640,8 @@ void InlineTextBox::paint(PaintInfo& paintInfo, const LayoutPoint& paintOffset,
         selectionStartEnd(sPos, ePos);
 
     if (m_truncation != cNoTruncation) {
-        sPos = std::min(sPos, static_cast<unsigned>(m_truncation));
-        ePos = std::min(ePos, static_cast<unsigned>(m_truncation));
+        sPos = std::min(sPos, m_truncation);
+        ePos = std::min(ePos, m_truncation);
         length = m_truncation;
     }
 
@@ -721,7 +721,7 @@ void InlineTextBox::selectionStartEnd(unsigned& sPos, unsigned& ePos)
     
     sPos = startPos > m_start ? startPos - m_start : 0;
     ePos = endPos > m_start ? endPos - m_start : 0;
-    ePos = std::min(ePos, static_cast<unsigned>(m_len));
+    ePos = std::min(ePos, m_len);
 }
 
 void InlineTextBox::paintSelection(GraphicsContext* context, const FloatPoint& boxOrigin, const RenderStyle& style, const Font& font, Color textColor)
@@ -753,8 +753,8 @@ void InlineTextBox::paintSelection(GraphicsContext* context, const FloatPoint& b
     unsigned length = m_truncation != cNoTruncation ? m_truncation : m_len;
     String string = renderer().text();
 
-    if (string.length() != static_cast<unsigned>(length) || m_start) {
-        ASSERT_WITH_SECURITY_IMPLICATION(static_cast<unsigned>(m_start + length) <= string.length());
+    if (string.length() != length || m_start) {
+        ASSERT_WITH_SECURITY_IMPLICATION(m_start + length <= string.length());
         string = string.substringSharingImpl(m_start, length);
     }
 
@@ -789,7 +789,7 @@ void InlineTextBox::paintCompositionBackground(GraphicsContext* context, const F
     unsigned offset = m_start;
     unsigned sPos = startPos > offset ? startPos - offset : 0;
     ASSERT(endPos >= offset);
-    unsigned ePos = std::min(endPos - offset, static_cast<unsigned>(m_len));
+    unsigned ePos = std::min(endPos - offset, m_len);
 
     if (sPos >= ePos)
         return;
@@ -1139,10 +1139,10 @@ void InlineTextBox::paintDocumentMarker(GraphicsContext* pt, const FloatPoint& b
     if (!markerSpansWholeBox || grammar || isDictationMarker) {
         unsigned startPosition = marker->startOffset() > m_start ? marker->startOffset() - m_start : 0;
         ASSERT(marker->endOffset() >= m_start);
-        unsigned endPosition = std::min(marker->endOffset() - m_start, static_cast<unsigned>(m_len));
+        unsigned endPosition = std::min(marker->endOffset() - m_start, m_len);
         
         if (m_truncation != cNoTruncation)
-            endPosition = std::min(endPosition, static_cast<unsigned>(m_truncation));
+            endPosition = std::min(endPosition, m_truncation);
 
         // Calculate start & width
         int deltaY = renderer().style().isFlippedLinesWritingMode() ? selectionBottom() - logicalBottom() : logicalTop() - selectionTop();
@@ -1191,7 +1191,7 @@ void InlineTextBox::paintTextMatchMarker(GraphicsContext* context, const FloatPo
 
     unsigned sPos = marker->startOffset() > m_start ? marker->startOffset() - m_start : 0;
     ASSERT(marker->endOffset() >= m_start);
-    unsigned ePos = std::min(marker->endOffset() - m_start, static_cast<unsigned>(m_len));
+    unsigned ePos = std::min(marker->endOffset() - m_start, m_len);
     TextRun run = constructTextRun(style, font);
 
     // Always compute and store the rect associated with this marker. The computed rect is in absolute coordinates.
@@ -1225,7 +1225,7 @@ void InlineTextBox::computeRectForReplacementMarker(DocumentMarker* marker, cons
     
     unsigned sPos = marker->startOffset() > m_start ? marker->startOffset() - m_start : 0;
     ASSERT(marker->endOffset() >= m_start);
-    unsigned ePos = std::min(marker->endOffset() - m_start, (unsigned)m_len);
+    unsigned ePos = std::min(marker->endOffset() - m_start, m_len);
     TextRun run = constructTextRun(style, font);
 
     // Compute and store the rect associated with this marker.
index 7d3f5b6..6fc0173 100644 (file)
@@ -34,8 +34,8 @@ struct CompositionUnderline;
 class DocumentMarker;
 class TextPainter;
 
-const unsigned short cNoTruncation = USHRT_MAX;
-const unsigned short cFullTruncation = USHRT_MAX - 1;
+const unsigned cNoTruncation = std::numeric_limits<unsigned>::max();
+const unsigned cFullTruncation = std::numeric_limits<unsigned>::max() - 1;
 
 class BufferForAppendingHyphen : public StringBuilder {
 public:
@@ -75,7 +75,7 @@ public:
 
     void offsetRun(int d) { ASSERT(!isDirty()); m_start += d; }
 
-    unsigned short truncation() const { return m_truncation; }
+    unsigned truncation() const { return m_truncation; }
 
     virtual void markDirty(bool dirty = true) override final;
 
@@ -188,11 +188,11 @@ private:
     InlineTextBox* m_nextTextBox; // The next box that also uses our RenderObject
 
     unsigned m_start;
-    unsigned short m_len;
+    unsigned m_len;
 
     // Where to truncate when text overflow is applied. We use special constants to
     // denote no truncation (the whole run paints) and full truncation (nothing paints at all).
-    unsigned short m_truncation;
+    unsigned m_truncation;
 };
 
 INLINE_BOX_OBJECT_TYPE_CASTS(InlineTextBox, isInlineTextBox())
index 8ae0d60..3ac68a4 100644 (file)
@@ -464,7 +464,7 @@ void RenderTextLineBoxes::setSelectionState(RenderText& renderer, RenderObject::
 
 static IntRect ellipsisRectForBox(const InlineTextBox& box, unsigned start, unsigned end)
 {
-    unsigned short truncation = box.truncation();
+    unsigned truncation = box.truncation();
     if (truncation == cNoTruncation)
         return IntRect();
 
@@ -473,8 +473,9 @@ static IntRect ellipsisRectForBox(const InlineTextBox& box, unsigned start, unsi
         return IntRect();
     
     IntRect rect;
-    int ellipsisStartPosition = std::max<int>(start - box.start(), 0);
-    int ellipsisEndPosition = std::min<int>(end - box.start(), box.len());
+    unsigned ellipsisStartPosition = start > box.start() ? start - box.start() : 0;
+    ASSERT(end >= box.start());
+    unsigned ellipsisEndPosition = std::min(end - box.start(), box.len());
     
     // The ellipsis should be considered to be selected if the end of
     // the selection is past the beginning of the truncation and the