Remove convenience constructors for TextRun
authormmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 May 2015 17:18:00 +0000 (17:18 +0000)
committermmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 May 2015 17:18:00 +0000 (17:18 +0000)
https://bugs.webkit.org/show_bug.cgi?id=144752

Source/WebCore:

These convenience constructors are unnecessary. Moving the code that makes the StringView
back to the call site will also help us make things more elegant in future refactoring.

Reviewed by Darin Adler.

No new tests because there is no behavior change.

* css/CSSPrimitiveValue.cpp:
(WebCore::CSSPrimitiveValue::formatNumberForCustomCSSText): Remove ambiguous call.
* platform/graphics/StringTruncator.cpp:
(WebCore::stringWidth):
* platform/graphics/TextRun.h:
(WebCore::TextRun::TextRun):
* platform/mac/DragImageMac.mm:
(WebCore::widthWithFont):
(WebCore::drawAtPoint):
* rendering/SimpleLineLayout.cpp:
(WebCore::SimpleLineLayout::canUseFor):
* rendering/SimpleLineLayoutTextFragmentIterator.cpp:
(WebCore::SimpleLineLayout::TextFragmentIterator::Style::Style):
(WebCore::SimpleLineLayout::TextFragmentIterator::runWidth):
* rendering/TextPainter.cpp:
(WebCore::TextPainter::paintText):

Source/WebKit/mac:

These convenience constructors are unnecessary. Moving the code that makes the StringView
back to the call site will also help us make things more elegant in future refactoring.

Reviewed by Darin Adler.

No new tests because there is no behavior change.

* Misc/WebKitNSStringExtras.mm:
(-[NSString _web_drawAtPoint:font:textColor:allowingFontSmoothing:]):
(-[NSString _web_widthWithFont:]):

Source/WTF:

Reviewed by Anders Carlsson.

No reason why StringView shouldn't have a StringImpl* constructor.

Test: StringView8Bit in TestWebKitAPI

* wtf/text/StringView.h: Add the constructor.

Tools:

Reviewed by Anders Carlsson.

Test the StringView which takes a StringImpl*.

* TestWebKitAPI/Tests/WTF/StringView.cpp:
(StringView8Bit): Testing is8Bit() on StringView

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

13 files changed:
Source/WTF/ChangeLog
Source/WTF/wtf/text/StringView.h
Source/WebCore/ChangeLog
Source/WebCore/css/CSSPrimitiveValue.cpp
Source/WebCore/platform/graphics/StringTruncator.cpp
Source/WebCore/platform/graphics/TextRun.h
Source/WebCore/platform/mac/DragImageMac.mm
Source/WebCore/rendering/SimpleLineLayoutTextFragmentIterator.cpp
Source/WebCore/rendering/TextPainter.cpp
Source/WebKit/mac/ChangeLog
Source/WebKit/mac/Misc/WebKitNSStringExtras.mm
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WTF/StringView.cpp

index 829ddda..ec6e884 100644 (file)
@@ -1,3 +1,16 @@
+2015-05-08  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Remove convenience constructors for TextRun
+        https://bugs.webkit.org/show_bug.cgi?id=144752
+
+        Reviewed by Anders Carlsson.
+
+        No reason why StringView shouldn't have a StringImpl* constructor.
+
+        Test: StringView8Bit in TestWebKitAPI
+
+        * wtf/text/StringView.h: Add the constructor.
+
 2015-05-08  Andreas Kling  <akling@apple.com>
 
         Optimize serialization of quoted JSON strings.
index d9ce74c..216d2f5 100644 (file)
@@ -58,6 +58,7 @@ public:
 
     StringView(const String&);
     StringView(const StringImpl&);
+    StringView(const StringImpl*);
     StringView(const LChar*, unsigned length);
     StringView(const UChar*, unsigned length);
 
@@ -271,6 +272,18 @@ inline StringView::StringView(const StringImpl& string)
         initialize(string.characters16(), string.length());
 }
 
+inline StringView::StringView(const StringImpl* string)
+{
+    if (!string)
+        return;
+
+    setUnderlyingString(string);
+    if (string->is8Bit())
+        initialize(string->characters8(), string->length());
+    else
+        initialize(string->characters16(), string->length());
+}
+
 inline StringView::StringView(const String& string)
 {
     setUnderlyingString(string.impl());
index 83a6d2e..1344089 100644 (file)
@@ -1,3 +1,32 @@
+2015-05-08  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Remove convenience constructors for TextRun
+        https://bugs.webkit.org/show_bug.cgi?id=144752
+
+        These convenience constructors are unnecessary. Moving the code that makes the StringView
+        back to the call site will also help us make things more elegant in future refactoring.
+
+        Reviewed by Darin Adler.
+
+        No new tests because there is no behavior change.
+
+        * css/CSSPrimitiveValue.cpp:
+        (WebCore::CSSPrimitiveValue::formatNumberForCustomCSSText): Remove ambiguous call.
+        * platform/graphics/StringTruncator.cpp:
+        (WebCore::stringWidth):
+        * platform/graphics/TextRun.h:
+        (WebCore::TextRun::TextRun):
+        * platform/mac/DragImageMac.mm:
+        (WebCore::widthWithFont):
+        (WebCore::drawAtPoint):
+        * rendering/SimpleLineLayout.cpp:
+        (WebCore::SimpleLineLayout::canUseFor):
+        * rendering/SimpleLineLayoutTextFragmentIterator.cpp:
+        (WebCore::SimpleLineLayout::TextFragmentIterator::Style::Style):
+        (WebCore::SimpleLineLayout::TextFragmentIterator::runWidth):
+        * rendering/TextPainter.cpp:
+        (WebCore::TextPainter::paintText):
+
 2015-05-08  Xabier Rodriguez Calvar  <calvaris@igalia.com> and Youenn Fablet <youenn.fablet@crf.canon.fr>
 
         [Streams API] ReadableStream constructor start function should be able to error the stream
index d889504..71798ee 100644 (file)
@@ -1051,7 +1051,7 @@ ALWAYS_INLINE String CSSPrimitiveValue::formatNumberForCustomCSSText() const
         StringBuilder result;
         result.reserveCapacity(6 + m_value.string->length());
         result.appendLiteral("attr(");
-        result.append(m_value.string);
+        result.append(String(m_value.string));
         result.append(')');
 
         return result.toString();
index 66afbea..580be09 100644 (file)
@@ -194,7 +194,7 @@ static unsigned leftTruncateToBuffer(const String& string, unsigned length, unsi
 
 static float stringWidth(const FontCascade& renderer, const UChar* characters, unsigned length, bool disableRoundingHacks)
 {
-    TextRun run(characters, length);
+    TextRun run(StringView(characters, length));
     if (disableRoundingHacks)
         run.disableRoundingHacks();
     return renderer.width(run);
index 27121e9..1f1ef1f 100644 (file)
@@ -52,9 +52,9 @@ public:
 
     typedef unsigned RoundingHacks;
 
-    explicit TextRun(StringView s, float xpos = 0, float expansion = 0, ExpansionBehavior expansionBehavior = AllowTrailingExpansion | ForbidLeadingExpansion, TextDirection direction = LTR, bool directionalOverride = false, bool characterScanForCodePath = true, RoundingHacks roundingHacks = RunRounding | WordRounding)
-        : m_text(s)
-        , m_charactersLength(s.length())
+    explicit TextRun(StringView text, float xpos = 0, float expansion = 0, ExpansionBehavior expansionBehavior = AllowTrailingExpansion | ForbidLeadingExpansion, TextDirection direction = LTR, bool directionalOverride = false, bool characterScanForCodePath = true, RoundingHacks roundingHacks = RunRounding | WordRounding)
+        : m_text(text)
+        , m_charactersLength(text.length())
         , m_tabSize(0)
         , m_xpos(xpos)
         , m_horizontalGlyphStretch(1)
@@ -70,21 +70,6 @@ public:
     {
     }
 
-    explicit TextRun(const String& s)
-        : TextRun(StringView(s))
-    {
-    }
-
-    TextRun(const LChar* c, unsigned len)
-        : TextRun(StringView(c, len))
-    {
-    }
-
-    TextRun(const UChar* c, unsigned len)
-        : TextRun(StringView(c, len))
-    {
-    }
-
     TextRun subRun(unsigned startOffset, unsigned length) const
     {
         ASSERT_WITH_SECURITY_IMPLICATION(startOffset < m_text.length());
index 353ef8d..66b1eb8 100644 (file)
@@ -192,7 +192,7 @@ static float widthWithFont(NSString *string, NSFont *font)
     
     if (canUseFastRenderer(buffer.data(), length)) {
         FontCascade webCoreFont(FontPlatformData(reinterpret_cast<CTFontRef>(font), [font pointSize]));
-        TextRun run(buffer.data(), length);
+        TextRun run(StringView(buffer.data(), length));
         run.disableRoundingHacks();
         return webCoreFont.width(run);
     }
@@ -224,7 +224,7 @@ static void drawAtPoint(NSString *string, NSPoint point, NSFont *font, NSColor *
             CGContextScaleCTM(cgContext, 1, -1);
             
         FontCascade webCoreFont(FontPlatformData(reinterpret_cast<CTFontRef>(font), [font pointSize]), Antialiased);
-        TextRun run(buffer.data(), length);
+        TextRun run(StringView(buffer.data(), length));
         run.disableRoundingHacks();
 
         CGFloat red;
index 7af785b..d01e89d 100644 (file)
@@ -40,7 +40,7 @@ TextFragmentIterator::Style::Style(const RenderStyle& style)
     , preserveNewline(style.preserveNewline())
     , wrapLines(style.autoWrap())
     , breakWordOnOverflow(style.overflowWrap() == BreakOverflowWrap && (wrapLines || preserveNewline))
-    , spaceWidth(font.width(TextRun(&space, 1)))
+    , spaceWidth(font.width(TextRun(StringView(&space, 1))))
     , tabWidth(collapseWhitespace ? 0 : style.tabSize())
     , locale(style.locale())
 {
@@ -197,7 +197,7 @@ float TextFragmentIterator::runWidth(const FlowContents::Segment& segment, unsig
     bool measureWithEndSpace = m_style.collapseWhitespace && segmentTo < segment.text.length() && segment.text[segmentTo] == ' ';
     if (measureWithEndSpace)
         ++segmentTo;
-    TextRun run(segment.text.characters<CharacterType>() + segmentFrom, segmentTo - segmentFrom);
+    TextRun run(StringView(segment.text.substring(segmentFrom, segmentTo - segmentFrom)));
     run.setXPos(xPosition);
     run.setTabSize(!!m_style.tabWidth, m_style.tabWidth);
     float width = m_style.font.width(run);
index 12746f6..e4286c0 100644 (file)
@@ -170,7 +170,7 @@ void TextPainter::paintText()
         if (!m_emphasisMark.isEmpty()) {
             updateGraphicsContext(m_context, m_textPaintStyle, UseEmphasisMarkColor);
 
-            static NeverDestroyed<TextRun> objectReplacementCharacterTextRun(&objectReplacementCharacter, 1);
+            static NeverDestroyed<TextRun> objectReplacementCharacterTextRun(StringView(&objectReplacementCharacter, 1));
             TextRun& emphasisMarkTextRun = m_combinedText ? objectReplacementCharacterTextRun.get() : m_textRun;
             FloatPoint emphasisMarkTextOrigin = m_combinedText ? FloatPoint(boxOrigin.x() + m_boxRect.width() / 2, boxOrigin.y() + m_font.fontMetrics().ascent()) : m_textOrigin;
             if (m_combinedText)
@@ -196,7 +196,7 @@ void TextPainter::paintText()
         if (!m_emphasisMark.isEmpty()) {
             updateGraphicsContext(m_context, m_selectionPaintStyle, UseEmphasisMarkColor);
 
-            DEPRECATED_DEFINE_STATIC_LOCAL(TextRun, objectReplacementCharacterTextRun, (&objectReplacementCharacter, 1));
+            DEPRECATED_DEFINE_STATIC_LOCAL(TextRun, objectReplacementCharacterTextRun, (StringView(&objectReplacementCharacter, 1)));
             TextRun& emphasisMarkTextRun = m_combinedText ? objectReplacementCharacterTextRun : m_textRun;
             FloatPoint emphasisMarkTextOrigin = m_combinedText ? FloatPoint(boxOrigin.x() + m_boxRect.width() / 2, boxOrigin.y() + m_font.fontMetrics().ascent()) : m_textOrigin;
             if (m_combinedText)
index d2369ba..e69f38c 100644 (file)
@@ -1,3 +1,19 @@
+2015-05-08  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Remove convenience constructors for TextRun
+        https://bugs.webkit.org/show_bug.cgi?id=144752
+
+        These convenience constructors are unnecessary. Moving the code that makes the StringView
+        back to the call site will also help us make things more elegant in future refactoring.
+
+        Reviewed by Darin Adler.
+
+        No new tests because there is no behavior change.
+
+        * Misc/WebKitNSStringExtras.mm:
+        (-[NSString _web_drawAtPoint:font:textColor:allowingFontSmoothing:]):
+        (-[NSString _web_widthWithFont:]):
+
 2015-05-07  Anders Carlsson  <andersca@apple.com>
 
         Build fixes.
index 3b0d0bb..7ba6e93 100644 (file)
@@ -93,7 +93,7 @@ static BOOL canUseFastRenderer(const UniChar *buffer, unsigned length)
             CGContextScaleCTM(cgContext, 1, -1);
 
         FontCascade webCoreFont(FontPlatformData(reinterpret_cast<CTFontRef>(font), [font pointSize]), fontSmoothingIsAllowed ? AutoSmoothing : Antialiased);
-        TextRun run(buffer.data(), length);
+        TextRun run(StringView(buffer.data(), length));
         run.disableRoundingHacks();
 
         CGFloat red;
@@ -139,7 +139,7 @@ static BOOL canUseFastRenderer(const UniChar *buffer, unsigned length)
 
     if (canUseFastRenderer(buffer.data(), length)) {
         FontCascade webCoreFont(FontPlatformData(reinterpret_cast<CTFontRef>(font), [font pointSize]));
-        TextRun run(buffer.data(), length);
+        TextRun run(StringView(buffer.data(), length));
         run.disableRoundingHacks();
         return webCoreFont.width(run);
     }
index b79edc2..f85f0ff 100644 (file)
@@ -1,3 +1,15 @@
+2015-05-08  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Remove convenience constructors for TextRun
+        https://bugs.webkit.org/show_bug.cgi?id=144752
+
+        Reviewed by Anders Carlsson.
+
+        Test the StringView which takes a StringImpl*.
+
+        * TestWebKitAPI/Tests/WTF/StringView.cpp:
+        (StringView8Bit): Testing is8Bit() on StringView
+
 2015-05-08  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         [GTK] WTR doesn't correctly handle the Escape key
index 153ce55..ab0feaf 100644 (file)
@@ -697,4 +697,23 @@ TEST(WTF, StringViewEndsWithIgnoringASCIICaseWithLatin1Characters)
     EXPECT_FALSE(referenceUTF8.endsWithIgnoringASCIICase(reference));
 }
 
+TEST(WTF, StringView8Bit)
+{
+    StringView nullView;
+    StringView emptyView = StringView::empty();
+    EXPECT_TRUE(StringView().is8Bit());
+    EXPECT_TRUE(StringView::empty().is8Bit());
+
+    LChar* lcharPtr = nullptr;
+    UChar* ucharPtr = nullptr;
+    EXPECT_TRUE(StringView(lcharPtr, 0).is8Bit());
+    EXPECT_FALSE(StringView(ucharPtr, 0).is8Bit());
+
+    EXPECT_TRUE(StringView(String(lcharPtr, 0)).is8Bit());
+    EXPECT_TRUE(StringView(String(ucharPtr, 0)).is8Bit());
+
+    EXPECT_TRUE(StringView(String().impl()).is8Bit());
+    EXPECT_TRUE(StringView(emptyString().impl()).is8Bit());
+}
+
 } // namespace TestWebKitAPI