Remove convenience constructors for TextRun

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

diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
index 829ddda75a5f51e7643eea72995d24c7ab01284e..ec6e884685b57ba4498163fdd34e43f136f4949c 100644 (file)
--- a/Source/WTF/ChangeLog
+++ b/Source/WTF/ChangeLog
@@ -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.

diff --git a/Source/WTF/wtf/text/StringView.h b/Source/WTF/wtf/text/StringView.h
index d9ce74c8aa2124bd62183aa9668700c9986ea740..216d2f5c4890339528d007d36c15985d5da7260c 100644 (file)
--- a/Source/WTF/wtf/text/StringView.h
+++ b/Source/WTF/wtf/text/StringView.h
@@ -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());

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 83a6d2e92e6d02f06ba673801e70edb503668d62..13440891d69104b9d64818817c0d223d61b06eb6 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -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

diff --git a/Source/WebCore/css/CSSPrimitiveValue.cpp b/Source/WebCore/css/CSSPrimitiveValue.cpp
index d889504037f44b3ef5625c79f565b619a1730c11..71798ee8d6b53cd02d1505935698a25e8a1b1310 100644 (file)
--- a/Source/WebCore/css/CSSPrimitiveValue.cpp
+++ b/Source/WebCore/css/CSSPrimitiveValue.cpp
@@ -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();

diff --git a/Source/WebCore/platform/graphics/StringTruncator.cpp b/Source/WebCore/platform/graphics/StringTruncator.cpp
index 66afbea3d1db6415362bd15f161c353bc36985d6..580be09753649406d3eb123cacb712b3f441609a 100644 (file)
--- a/Source/WebCore/platform/graphics/StringTruncator.cpp
+++ b/Source/WebCore/platform/graphics/StringTruncator.cpp
@@ -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);

diff --git a/Source/WebCore/platform/graphics/TextRun.h b/Source/WebCore/platform/graphics/TextRun.h
index 27121e90cbcda1dc8d46842a43eb21a1cccecb14..1f1ef1f667359ce9a1f68e1331b86bc3060ceb88 100644 (file)
--- a/Source/WebCore/platform/graphics/TextRun.h
+++ b/Source/WebCore/platform/graphics/TextRun.h
@@ -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());

diff --git a/Source/WebCore/platform/mac/DragImageMac.mm b/Source/WebCore/platform/mac/DragImageMac.mm
index 353ef8d23854d9a6f51143744465bcab2bfe4b93..66b1eb86763cc90f17a4520fcbf50b7365fc0cf9 100644 (file)
--- a/Source/WebCore/platform/mac/DragImageMac.mm
+++ b/Source/WebCore/platform/mac/DragImageMac.mm
@@ -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;

diff --git a/Source/WebCore/rendering/SimpleLineLayoutTextFragmentIterator.cpp b/Source/WebCore/rendering/SimpleLineLayoutTextFragmentIterator.cpp
index 7af785bcb153c541cc7e830a3929375d9dff6fd4..d01e89d357932cb045d5dca79534fb32463261d7 100644 (file)
--- a/Source/WebCore/rendering/SimpleLineLayoutTextFragmentIterator.cpp
+++ b/Source/WebCore/rendering/SimpleLineLayoutTextFragmentIterator.cpp
@@ -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);

diff --git a/Source/WebCore/rendering/TextPainter.cpp b/Source/WebCore/rendering/TextPainter.cpp
index 12746f645cfab27eb8d959355bbec9b6b9896c2c..e4286c04103ccb79b4e6e66b7bbb5426fc2d6873 100644 (file)
--- a/Source/WebCore/rendering/TextPainter.cpp
+++ b/Source/WebCore/rendering/TextPainter.cpp
@@ -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)

diff --git a/Source/WebKit/mac/ChangeLog b/Source/WebKit/mac/ChangeLog
index d2369ba07671a7cd37bab3ce353c41d29e2aeb3c..e69f38c16dd87297ae282e13706c376da7d676de 100644 (file)
--- a/Source/WebKit/mac/ChangeLog
+++ b/Source/WebKit/mac/ChangeLog
@@ -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.

diff --git a/Source/WebKit/mac/Misc/WebKitNSStringExtras.mm b/Source/WebKit/mac/Misc/WebKitNSStringExtras.mm
index 3b0d0bbc2e5b74713bf3666741ead9b40509b49b..7ba6e933fc09f7d99b87650bd0afa05dbf0565d0 100644 (file)
--- a/Source/WebKit/mac/Misc/WebKitNSStringExtras.mm
+++ b/Source/WebKit/mac/Misc/WebKitNSStringExtras.mm
@@ -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);
     }

diff --git a/Tools/ChangeLog b/Tools/ChangeLog
index b79edc2c4dd4bcabe880e9e7f007d30cc9581834..f85f0ff583caf571bc325d4d0929d2823c0945e3 100644 (file)
--- a/Tools/ChangeLog
+++ b/Tools/ChangeLog
@@ -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

diff --git a/Tools/TestWebKitAPI/Tests/WTF/StringView.cpp b/Tools/TestWebKitAPI/Tests/WTF/StringView.cpp
index 153ce5565019b9aaa6d1104959f49592dc4426bd..ab0feaf40d4a36d107f943bef90ad9a4909b4855 100644 (file)
--- a/Tools/TestWebKitAPI/Tests/WTF/StringView.cpp
+++ b/Tools/TestWebKitAPI/Tests/WTF/StringView.cpp
@@ -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