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 710d6eb..8db1b93 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 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 f9aab50..5bed66c 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 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 d78f379..a369a24 100644 (file)
@@ -1,3 +1,4 @@
+
 /*
  * Copyright (C) 2010, 2011, 2014-2015 Apple Inc. All rights reserved.
  *
index 7af785b..89bd4cd 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 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 3a9405f..7627730 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 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 7238c0f..c8d017e 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 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