Remove convenience constructors for TextRun
authormmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 9 May 2015 06:33:02 +0000 (06:33 +0000)
committermmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 9 May 2015 06:33:02 +0000 (06:33 +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@184037 268f45cc-cd09-0410-ab3c-d52691b4dbfc

14 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/platform/mac/ScrollAnimatorMac.h
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 710d6eb5b3eefd4123d568d42eb3eb25eae35de2..8db1b93dc00e6ec6c1bb3c3c1f015b2b13d28a5b 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  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r183996.
index d9ce74c8aa2124bd62183aa9668700c9986ea740..216d2f5c4890339528d007d36c15985d5da7260c 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 f9aab50ceaf7c9d42fd5528711eb88b94e77b470..5bed66c646d3fc909a6ca9680f231cf96e6cdd8e 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  Sam Weinig  <sam@webkit.org>
 
         Element Traversal is not just Elements anymore
index d889504037f44b3ef5625c79f565b619a1730c11..71798ee8d6b53cd02d1505935698a25e8a1b1310 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 66afbea3d1db6415362bd15f161c353bc36985d6..580be09753649406d3eb123cacb712b3f441609a 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 27121e90cbcda1dc8d46842a43eb21a1cccecb14..1f1ef1f667359ce9a1f68e1331b86bc3060ceb88 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 353ef8d23854d9a6f51143744465bcab2bfe4b93..66b1eb86763cc90f17a4520fcbf50b7365fc0cf9 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 d78f3793f85e40571296f21e2924292dc7d81c8f..a369a244cac8a2fd57d33103916acfebbb385ed5 100644 (file)
@@ -1,3 +1,4 @@
+
 /*
  * Copyright (C) 2010, 2011, 2014-2015 Apple Inc. All rights reserved.
  *
index 7af785bcb153c541cc7e830a3929375d9dff6fd4..89bd4cd2a33cede7455ff082abc3164b76592ac2 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,8 @@ 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);
+    auto string = segment.text.substring(segmentFrom, segmentTo - segmentFrom);
+    TextRun run(string);
     run.setXPos(xPosition);
     run.setTabSize(!!m_style.tabWidth, m_style.tabWidth);
     float width = m_style.font.width(run);
index 12746f645cfab27eb8d959355bbec9b6b9896c2c..e4286c04103ccb79b4e6e66b7bbb5426fc2d6873 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 3a9405fded99204a889c681b61e0fd04550eb787..7627730aeac7a6e95793f16afc98e9e606ec8725 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-08  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r183996.
index 3b0d0bbc2e5b74713bf3666741ead9b40509b49b..7ba6e933fc09f7d99b87650bd0afa05dbf0565d0 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 7238c0f710a22f8d948835b5dd5b93007303a9c8..c8d017efdfc954813fee681f83b421fb0df642f2 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-07  Sam Weinig  <sam@webkit.org>
 
         [Content Extensions] Add simple tester that takes an extension and compiles it
index 153ce5565019b9aaa6d1104959f49592dc4426bd..ab0feaf40d4a36d107f943bef90ad9a4909b4855 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