<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[170413] trunk/Source/WebCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/170413">170413</a></dd>
<dt>Author</dt> <dd>mmaxfield@apple.com</dd>
<dt>Date</dt> <dd>2014-06-24 18:19:25 -0700 (Tue, 24 Jun 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre>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):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorerenderingInlineTextBoxcpp">trunk/Source/WebCore/rendering/InlineTextBox.cpp</a></li>
<li><a href="#trunkSourceWebCorerenderingInlineTextBoxh">trunk/Source/WebCore/rendering/InlineTextBox.h</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderTextLineBoxescpp">trunk/Source/WebCore/rendering/RenderTextLineBoxes.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (170412 => 170413)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2014-06-25 00:56:17 UTC (rev 170412)
+++ trunk/Source/WebCore/ChangeLog 2014-06-25 01:19:25 UTC (rev 170413)
</span><span class="lines">@@ -1,3 +1,34 @@
</span><ins>+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):
+
</ins><span class="cx"> 2014-06-24 Ryosuke Niwa <rniwa@webkit.org>
</span><span class="cx">
</span><span class="cx"> Speculative 32-bit Mac build fix after r170402.
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingInlineTextBoxcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/InlineTextBox.cpp (170412 => 170413)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/InlineTextBox.cpp 2014-06-25 00:56:17 UTC (rev 170412)
+++ trunk/Source/WebCore/rendering/InlineTextBox.cpp 2014-06-25 01:19:25 UTC (rev 170413)
</span><span class="lines">@@ -59,7 +59,7 @@
</span><span class="cx">
</span><span class="cx"> struct SameSizeAsInlineTextBox : public InlineBox {
</span><span class="cx"> unsigned variables[1];
</span><del>- unsigned short variables2[2];
</del><ins>+ unsigned variables2[2];
</ins><span class="cx"> void* pointers[2];
</span><span class="cx"> };
</span><span class="cx">
</span><span class="lines">@@ -201,7 +201,7 @@
</span><span class="cx"> unsigned sPos = startPos > m_start ? startPos - m_start : 0;
</span><span class="cx"> if (endPos <= m_start)
</span><span class="cx"> return false;
</span><del>- unsigned ePos = std::min(endPos - m_start, static_cast<unsigned>(m_len));
</del><ins>+ unsigned ePos = std::min(endPos - m_start, m_len);
</ins><span class="cx"> return sPos < ePos;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -278,7 +278,7 @@
</span><span class="cx"> if (endPos < m_start)
</span><span class="cx"> return LayoutRect();
</span><span class="cx">
</span><del>- unsigned ePos = std::min(endPos - m_start, static_cast<unsigned>(m_len));
</del><ins>+ unsigned ePos = std::min(endPos - m_start, m_len);
</ins><span class="cx">
</span><span class="cx"> if (sPos > ePos)
</span><span class="cx"> return LayoutRect();
</span><span class="lines">@@ -640,8 +640,8 @@
</span><span class="cx"> selectionStartEnd(sPos, ePos);
</span><span class="cx">
</span><span class="cx"> if (m_truncation != cNoTruncation) {
</span><del>- sPos = std::min(sPos, static_cast<unsigned>(m_truncation));
- ePos = std::min(ePos, static_cast<unsigned>(m_truncation));
</del><ins>+ sPos = std::min(sPos, m_truncation);
+ ePos = std::min(ePos, m_truncation);
</ins><span class="cx"> length = m_truncation;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -721,7 +721,7 @@
</span><span class="cx">
</span><span class="cx"> sPos = startPos > m_start ? startPos - m_start : 0;
</span><span class="cx"> ePos = endPos > m_start ? endPos - m_start : 0;
</span><del>- ePos = std::min(ePos, static_cast<unsigned>(m_len));
</del><ins>+ ePos = std::min(ePos, m_len);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void InlineTextBox::paintSelection(GraphicsContext* context, const FloatPoint& boxOrigin, const RenderStyle& style, const Font& font, Color textColor)
</span><span class="lines">@@ -753,8 +753,8 @@
</span><span class="cx"> unsigned length = m_truncation != cNoTruncation ? m_truncation : m_len;
</span><span class="cx"> String string = renderer().text();
</span><span class="cx">
</span><del>- if (string.length() != static_cast<unsigned>(length) || m_start) {
- ASSERT_WITH_SECURITY_IMPLICATION(static_cast<unsigned>(m_start + length) <= string.length());
</del><ins>+ if (string.length() != length || m_start) {
+ ASSERT_WITH_SECURITY_IMPLICATION(m_start + length <= string.length());
</ins><span class="cx"> string = string.substringSharingImpl(m_start, length);
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -789,7 +789,7 @@
</span><span class="cx"> unsigned offset = m_start;
</span><span class="cx"> unsigned sPos = startPos > offset ? startPos - offset : 0;
</span><span class="cx"> ASSERT(endPos >= offset);
</span><del>- unsigned ePos = std::min(endPos - offset, static_cast<unsigned>(m_len));
</del><ins>+ unsigned ePos = std::min(endPos - offset, m_len);
</ins><span class="cx">
</span><span class="cx"> if (sPos >= ePos)
</span><span class="cx"> return;
</span><span class="lines">@@ -1139,10 +1139,10 @@
</span><span class="cx"> if (!markerSpansWholeBox || grammar || isDictationMarker) {
</span><span class="cx"> unsigned startPosition = marker->startOffset() > m_start ? marker->startOffset() - m_start : 0;
</span><span class="cx"> ASSERT(marker->endOffset() >= m_start);
</span><del>- unsigned endPosition = std::min(marker->endOffset() - m_start, static_cast<unsigned>(m_len));
</del><ins>+ unsigned endPosition = std::min(marker->endOffset() - m_start, m_len);
</ins><span class="cx">
</span><span class="cx"> if (m_truncation != cNoTruncation)
</span><del>- endPosition = std::min(endPosition, static_cast<unsigned>(m_truncation));
</del><ins>+ endPosition = std::min(endPosition, m_truncation);
</ins><span class="cx">
</span><span class="cx"> // Calculate start & width
</span><span class="cx"> int deltaY = renderer().style().isFlippedLinesWritingMode() ? selectionBottom() - logicalBottom() : logicalTop() - selectionTop();
</span><span class="lines">@@ -1191,7 +1191,7 @@
</span><span class="cx">
</span><span class="cx"> unsigned sPos = marker->startOffset() > m_start ? marker->startOffset() - m_start : 0;
</span><span class="cx"> ASSERT(marker->endOffset() >= m_start);
</span><del>- unsigned ePos = std::min(marker->endOffset() - m_start, static_cast<unsigned>(m_len));
</del><ins>+ unsigned ePos = std::min(marker->endOffset() - m_start, m_len);
</ins><span class="cx"> TextRun run = constructTextRun(style, font);
</span><span class="cx">
</span><span class="cx"> // Always compute and store the rect associated with this marker. The computed rect is in absolute coordinates.
</span><span class="lines">@@ -1225,7 +1225,7 @@
</span><span class="cx">
</span><span class="cx"> unsigned sPos = marker->startOffset() > m_start ? marker->startOffset() - m_start : 0;
</span><span class="cx"> ASSERT(marker->endOffset() >= m_start);
</span><del>- unsigned ePos = std::min(marker->endOffset() - m_start, (unsigned)m_len);
</del><ins>+ unsigned ePos = std::min(marker->endOffset() - m_start, m_len);
</ins><span class="cx"> TextRun run = constructTextRun(style, font);
</span><span class="cx">
</span><span class="cx"> // Compute and store the rect associated with this marker.
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingInlineTextBoxh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/InlineTextBox.h (170412 => 170413)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/InlineTextBox.h 2014-06-25 00:56:17 UTC (rev 170412)
+++ trunk/Source/WebCore/rendering/InlineTextBox.h 2014-06-25 01:19:25 UTC (rev 170413)
</span><span class="lines">@@ -34,8 +34,8 @@
</span><span class="cx"> class DocumentMarker;
</span><span class="cx"> class TextPainter;
</span><span class="cx">
</span><del>-const unsigned short cNoTruncation = USHRT_MAX;
-const unsigned short cFullTruncation = USHRT_MAX - 1;
</del><ins>+const unsigned cNoTruncation = std::numeric_limits<unsigned>::max();
+const unsigned cFullTruncation = std::numeric_limits<unsigned>::max() - 1;
</ins><span class="cx">
</span><span class="cx"> class BufferForAppendingHyphen : public StringBuilder {
</span><span class="cx"> public:
</span><span class="lines">@@ -75,7 +75,7 @@
</span><span class="cx">
</span><span class="cx"> void offsetRun(int d) { ASSERT(!isDirty()); m_start += d; }
</span><span class="cx">
</span><del>- unsigned short truncation() const { return m_truncation; }
</del><ins>+ unsigned truncation() const { return m_truncation; }
</ins><span class="cx">
</span><span class="cx"> virtual void markDirty(bool dirty = true) override final;
</span><span class="cx">
</span><span class="lines">@@ -188,11 +188,11 @@
</span><span class="cx"> InlineTextBox* m_nextTextBox; // The next box that also uses our RenderObject
</span><span class="cx">
</span><span class="cx"> unsigned m_start;
</span><del>- unsigned short m_len;
</del><ins>+ unsigned m_len;
</ins><span class="cx">
</span><span class="cx"> // Where to truncate when text overflow is applied. We use special constants to
</span><span class="cx"> // denote no truncation (the whole run paints) and full truncation (nothing paints at all).
</span><del>- unsigned short m_truncation;
</del><ins>+ unsigned m_truncation;
</ins><span class="cx"> };
</span><span class="cx">
</span><span class="cx"> INLINE_BOX_OBJECT_TYPE_CASTS(InlineTextBox, isInlineTextBox())
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderTextLineBoxescpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderTextLineBoxes.cpp (170412 => 170413)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderTextLineBoxes.cpp 2014-06-25 00:56:17 UTC (rev 170412)
+++ trunk/Source/WebCore/rendering/RenderTextLineBoxes.cpp 2014-06-25 01:19:25 UTC (rev 170413)
</span><span class="lines">@@ -464,7 +464,7 @@
</span><span class="cx">
</span><span class="cx"> static IntRect ellipsisRectForBox(const InlineTextBox& box, unsigned start, unsigned end)
</span><span class="cx"> {
</span><del>- unsigned short truncation = box.truncation();
</del><ins>+ unsigned truncation = box.truncation();
</ins><span class="cx"> if (truncation == cNoTruncation)
</span><span class="cx"> return IntRect();
</span><span class="cx">
</span><span class="lines">@@ -473,8 +473,9 @@
</span><span class="cx"> return IntRect();
</span><span class="cx">
</span><span class="cx"> IntRect rect;
</span><del>- int ellipsisStartPosition = std::max<int>(start - box.start(), 0);
- int ellipsisEndPosition = std::min<int>(end - box.start(), box.len());
</del><ins>+ unsigned ellipsisStartPosition = start > box.start() ? start - box.start() : 0;
+ ASSERT(end >= box.start());
+ unsigned ellipsisEndPosition = std::min(end - box.start(), box.len());
</ins><span class="cx">
</span><span class="cx"> // The ellipsis should be considered to be selected if the end of
</span><span class="cx"> // the selection is past the beginning of the truncation and the
</span></span></pre>
</div>
</div>
</body>
</html>