Source/WebCore: Fix recently-introduced off-by-one error in centerTruncateToBuffer
authordarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 29 Mar 2014 03:18:27 +0000 (03:18 +0000)
committerdarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 29 Mar 2014 03:18:27 +0000 (03:18 +0000)
https://bugs.webkit.org/show_bug.cgi?id=130889
<rdar://problem/16408694>

Reviewed by Alexey Proskuryakov.

* platform/graphics/StringTruncator.cpp:
(WebCore::centerTruncateToBuffer): Simplified expression that computes truncatedLength.
Removed incorrect "+ 1" from computation of where to write characters.

Source/WebKit/mac: Fix recently-introduced off-by-one error in centerTruncateToBuffer
https://bugs.webkit.org/show_bug.cgi?id=130889

Reviewed by Alexey Proskuryakov.

* Misc/WebStringTruncator.mm:
(defaultMenuFont): Changed to use NeverDestroyed since I had to touch this file anyway.
(fontFromNSFont): Ditto. Also improved variable names a bit.
(+[WebStringTruncator initialize]): Added threading initialization, needed for main
thread assertions in string truncator code.

Tools: Fix recently-introduced off-by-one error in centerTruncateToBuffer
https://bugs.webkit.org/show_bug.cgi?id=130889

Reviewed by Alexey Proskuryakov.

* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj: Added StringTruncator.mm.
* TestWebKitAPI/Tests/mac/StringTruncator.mm: Added. One test for each of the
WebStringTruncator methods; should be good for a start. These are dependent on the
metrics of Helvetica 12, but I am hoping that will be consistent across OS X machines.

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

Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/StringTruncator.cpp
Source/WebKit/mac/ChangeLog
Source/WebKit/mac/Misc/WebStringTruncator.mm
Tools/ChangeLog
Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
Tools/TestWebKitAPI/Tests/mac/StringTruncator.mm [new file with mode: 0644]

index 90089e1..04a7c2f 100644 (file)
@@ -1,3 +1,15 @@
+2014-03-28  Darin Adler  <darin@apple.com>
+
+        Fix recently-introduced off-by-one error in centerTruncateToBuffer
+        https://bugs.webkit.org/show_bug.cgi?id=130889
+        <rdar://problem/16408694>
+
+        Reviewed by Alexey Proskuryakov.
+
+        * platform/graphics/StringTruncator.cpp:
+        (WebCore::centerTruncateToBuffer): Simplified expression that computes truncatedLength.
+        Removed incorrect "+ 1" from computation of where to write characters.
+
 2014-03-28  Benjamin Poulain  <bpoulain@apple.com>
 
         Update the code related to SelectorPseudoTypeMap to reflect its new purpose
index 15e85d5..effd425 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2005, 2006, 2007 Apple Inc.  All rights reserved.
+ * Copyright (C) 2005, 2006, 2007, 2014 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -70,6 +70,8 @@ static unsigned centerTruncateToBuffer(const String& string, unsigned length, un
 
 #if PLATFORM(IOS)
     // FIXME: We should guard this code behind an editing behavior. Then we can remove the PLATFORM(IOS)-guard.
+    // Or just turn it on for all platforms. It seems like good behavior everywhere. Might be better to generalize
+    // it to handle all whitespace, not just "space".
 
     // Strip single character before ellipsis character, when that character is preceded by a space
     if (omitStart > 1 && string[omitStart - 1] != space && omitStart > 2 && string[omitStart - 2] == space)
@@ -87,13 +89,13 @@ static unsigned centerTruncateToBuffer(const String& string, unsigned length, un
         ++omitEnd;
 #endif
 
-    unsigned truncatedLength = shouldInsertEllipsis ? omitStart + 1 + (length - omitEnd) : length - (omitEnd - omitStart);
+    unsigned truncatedLength = omitStart + shouldInsertEllipsis + (length - omitEnd);
     ASSERT(truncatedLength <= length);
 
     StringView(string).substring(0, omitStart).getCharactersWithUpconvert(buffer);
     if (shouldInsertEllipsis)
         buffer[omitStart++] = horizontalEllipsis;
-    StringView(string).substring(omitEnd, length - omitEnd).getCharactersWithUpconvert(&buffer[omitStart + 1]);
+    StringView(string).substring(omitEnd, length - omitEnd).getCharactersWithUpconvert(&buffer[omitStart]);
     return truncatedLength;
 }
 
@@ -104,6 +106,8 @@ static unsigned rightTruncateToBuffer(const String& string, unsigned length, uns
 
 #if PLATFORM(IOS)
     // FIXME: We should guard this code behind an editing behavior. Then we can remove the PLATFORM(IOS)-guard.
+    // Or just turn it on for all platforms. It seems like good behavior everywhere. Might be better to generalize
+    // it to handle all whitespace, not just "space".
 
     // Strip single character before ellipsis character, when that character is preceded by a space
     if (keepCount > 1 && string[keepCount - 1] != space && keepCount > 2 && string[keepCount - 2] == space)
@@ -148,10 +152,14 @@ static unsigned rightClipToWordBuffer(const String& string, unsigned length, uns
 
 #if PLATFORM(IOS)
     // FIXME: We should guard this code behind an editing behavior. Then we can remove the PLATFORM(IOS)-guard.
+    // Or just turn it on for all platforms. It seems like good behavior everywhere. Might be better to generalize
+    // it to handle all whitespace, not just "space".
+
     // Motivated by <rdar://problem/7439327> truncation should not include a trailing space
-    while ((keepLength > 0) && (string[keepLength - 1] == space))
+    while (keepLength && string[keepLength - 1] == space)
         --keepLength;
 #endif
+
     return keepLength;
 }
 
@@ -249,11 +257,10 @@ static String truncateString(const String& string, float maxWidth, const Font& f
             / (widthForSmallestKnownToNotFit - widthForLargestKnownToFit);
         keepCount = static_cast<unsigned>(maxWidth * ratio);
         
-        if (keepCount <= keepCountForLargestKnownToFit) {
+        if (keepCount <= keepCountForLargestKnownToFit)
             keepCount = keepCountForLargestKnownToFit + 1;
-        } else if (keepCount >= keepCountForSmallestKnownToNotFit) {
+        else if (keepCount >= keepCountForSmallestKnownToNotFit)
             keepCount = keepCountForSmallestKnownToNotFit - 1;
-        }
         
         ASSERT_WITH_SECURITY_IMPLICATION(keepCount < length);
         ASSERT(keepCount > 0);
index ddf6c83..b7ca57b 100644 (file)
@@ -1,3 +1,16 @@
+2014-03-28  Darin Adler  <darin@apple.com>
+
+        Fix recently-introduced off-by-one error in centerTruncateToBuffer
+        https://bugs.webkit.org/show_bug.cgi?id=130889
+
+        Reviewed by Alexey Proskuryakov.
+
+        * Misc/WebStringTruncator.mm:
+        (defaultMenuFont): Changed to use NeverDestroyed since I had to touch this file anyway.
+        (fontFromNSFont): Ditto. Also improved variable names a bit.
+        (+[WebStringTruncator initialize]): Added threading initialization, needed for main
+        thread assertions in string truncator code.
+
 2014-03-27  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r166360.
index 1fb4fe1..bf89c81 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2005, 2006 Apple Inc.  All rights reserved.
+ * Copyright (C) 2005, 2006, 2014 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
 #import <WebCore/FontCache.h>
 #import <WebCore/FontPlatformData.h>
 #import <WebCore/StringTruncator.h>
-#import <wtf/StdLibExtras.h>
-#import <wtf/text/WTFString.h>
+#import <runtime/InitializeThreading.h>
+#import <wtf/MainThread.h>
+#import <wtf/NeverDestroyed.h>
 
-using namespace WebCore;
-
-static NSFont *defaultMenuFont()
+static WebCore::Font& fontFromNSFont(NSFont *font)
 {
-    static NSFont *defaultMenuFont = nil;
-    if (!defaultMenuFont) {
-        defaultMenuFont = [NSFont menuFontOfSize:0];
-        CFRetain(defaultMenuFont);
-    }
-    return defaultMenuFont;
-}
-
-static Font& fontFromNSFont(NSFont *font)
-{
-    static NSFont *currentFont;
-    DEPRECATED_DEFINE_STATIC_LOCAL(Font, currentRenderer, ());
-
-    if ([font isEqual:currentFont])
-        return currentRenderer;
-    if (currentFont)
-        CFRelease(currentFont);
-    currentFont = font;
-    CFRetain(currentFont);
-    FontPlatformData f(font, [font pointSize]);
-    currentRenderer = Font(f, ![[NSGraphicsContext currentContext] isDrawingToScreen]);
-    return currentRenderer;
+    static NeverDestroyed<RetainPtr<NSFont>> currentNSFont;
+    static NeverDestroyed<WebCore::Font> currentFont;
+    if ([font isEqual:currentNSFont.get().get()])
+        return currentFont;
+    currentNSFont.get() = font;
+    currentFont.get() = WebCore::Font(WebCore::FontPlatformData(font, [font pointSize]), ![[NSGraphicsContext currentContext] isDrawingToScreen]);
+    return currentFont;
 }
 
 @implementation WebStringTruncator
@@ -69,30 +53,33 @@ static Font& fontFromNSFont(NSFont *font)
 + (void)initialize
 {
     InitWebCoreSystemInterface();
+    JSC::initializeThreading();
+    WTF::initializeMainThreadToProcessMainThread();
 }
 
 + (NSString *)centerTruncateString:(NSString *)string toWidth:(float)maxWidth
 {
-    FontCachePurgePreventer fontCachePurgePreventer;
-    return StringTruncator::centerTruncate(string, maxWidth, fontFromNSFont(defaultMenuFont()));
+    static NeverDestroyed<RetainPtr<NSFont>> menuFont = [NSFont menuFontOfSize:0];
+    WebCore::FontCachePurgePreventer fontCachePurgePreventer;
+    return WebCore::StringTruncator::centerTruncate(string, maxWidth, fontFromNSFont(menuFont.get().get()));
 }
 
 + (NSString *)centerTruncateString:(NSString *)string toWidth:(float)maxWidth withFont:(NSFont *)font
 {
-    FontCachePurgePreventer fontCachePurgePreventer;
-    return StringTruncator::centerTruncate(string, maxWidth, fontFromNSFont(font));
+    WebCore::FontCachePurgePreventer fontCachePurgePreventer;
+    return WebCore::StringTruncator::centerTruncate(string, maxWidth, fontFromNSFont(font));
 }
 
 + (NSString *)rightTruncateString:(NSString *)string toWidth:(float)maxWidth withFont:(NSFont *)font
 {
-    FontCachePurgePreventer fontCachePurgePreventer;
-    return StringTruncator::rightTruncate(string, maxWidth, fontFromNSFont(font));
+    WebCore::FontCachePurgePreventer fontCachePurgePreventer;
+    return WebCore::StringTruncator::rightTruncate(string, maxWidth, fontFromNSFont(font));
 }
 
 + (float)widthOfString:(NSString *)string font:(NSFont *)font
 {
-    FontCachePurgePreventer fontCachePurgePreventer;
-    return StringTruncator::width(string, fontFromNSFont(font));
+    WebCore::FontCachePurgePreventer fontCachePurgePreventer;
+    return WebCore::StringTruncator::width(string, fontFromNSFont(font));
 }
 
 @end
index 70379cd..3913eb9 100644 (file)
@@ -1,3 +1,15 @@
+2014-03-28  Darin Adler  <darin@apple.com>
+
+        Fix recently-introduced off-by-one error in centerTruncateToBuffer
+        https://bugs.webkit.org/show_bug.cgi?id=130889
+
+        Reviewed by Alexey Proskuryakov.
+
+        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj: Added StringTruncator.mm.
+        * TestWebKitAPI/Tests/mac/StringTruncator.mm: Added. One test for each of the
+        WebStringTruncator methods; should be good for a start. These are dependent on the
+        metrics of Helvetica 12, but I am hoping that will be consistent across OS X machines.
+
 2014-03-28  Martin Hock  <mhock@apple.com>
 
         Unreviewed. Add myself as a committer.
index 8bbb570..d2805d9 100644 (file)
                9331407C17B4419000F083B1 /* DidNotHandleKeyDown.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 9331407B17B4419000F083B1 /* DidNotHandleKeyDown.cpp */; };
                9361002914DC95A70061379D /* lots-of-iframes.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 9361002814DC957B0061379D /* lots-of-iframes.html */; };
                939BA91714103412001A01BD /* DeviceScaleFactorOnBack.mm in Sources */ = {isa = PBXBuildFile; fileRef = 939BA91614103412001A01BD /* DeviceScaleFactorOnBack.mm */; };
+               939BFE3A18E5548900883275 /* StringTruncator.mm in Sources */ = {isa = PBXBuildFile; fileRef = 939BFE3918E5548900883275 /* StringTruncator.mm */; };
                93A427A9180D9B0700CD24D7 /* RefPtr.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 93A427A8180D9B0700CD24D7 /* RefPtr.cpp */; };
                93A427AB180DA26400CD24D7 /* Ref.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 93A427AA180DA26400CD24D7 /* Ref.cpp */; };
                93ABA80916DDAB91002DB2FA /* StringHasher.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 93ABA80816DDAB91002DB2FA /* StringHasher.cpp */; };
                9331407B17B4419000F083B1 /* DidNotHandleKeyDown.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = DidNotHandleKeyDown.cpp; sourceTree = "<group>"; };
                9361002814DC957B0061379D /* lots-of-iframes.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "lots-of-iframes.html"; sourceTree = "<group>"; };
                939BA91614103412001A01BD /* DeviceScaleFactorOnBack.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = DeviceScaleFactorOnBack.mm; sourceTree = "<group>"; };
+               939BFE3918E5548900883275 /* StringTruncator.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = StringTruncator.mm; sourceTree = "<group>"; };
                93A427A8180D9B0700CD24D7 /* RefPtr.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = RefPtr.cpp; sourceTree = "<group>"; };
                93A427AA180DA26400CD24D7 /* Ref.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = Ref.cpp; sourceTree = "<group>"; };
                93A427AC180DA60F00CD24D7 /* MoveOnly.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MoveOnly.h; sourceTree = "<group>"; };
                                C540F775152E4DA000A40C8C /* SimplifyMarkup.mm */,
                                291861FD17BD4DC700D4E41E /* StopLoadingFromDidFinishLoading.mm */,
                                E194E1BA177E5145009C4D4E /* StopLoadingFromDidReceiveResponse.mm */,
-                               CE32C7C718184C4900CD8C28 /* WillPerformClientRedirectToURLCrash.mm */,
                                3799AD3914120A43005EB0C6 /* StringByEvaluatingJavaScriptFromString.mm */,
+                               939BFE3918E5548900883275 /* StringTruncator.mm */,
                                37A6895D148A9B50005100FA /* SubresourceErrorCrash.mm */,
                                E490296714E2E3A4002BEDD1 /* TypingStyleCrash.mm */,
                                51FBBB4C1513D4E900822738 /* WebViewCanPasteURL.mm */,
                                C2EB2DD116CAC7AC009B52EE /* WebViewDidCreateJavaScriptContext.mm */,
                                37E38C33169B7D010084C28C /* WebViewDidRemoveFrameFromHierarchy.mm */,
+                               CE32C7C718184C4900CD8C28 /* WillPerformClientRedirectToURLCrash.mm */,
                                1A7BFC0A171A0BDB00BC5F64 /* WillSendSubmitEvent.mm */,
                                A5E2027215B2181900C13E14 /* WindowlessWebViewWithMedia.mm */,
                        );
                                14F3B11315E45EAB00210069 /* SaturatedArithmeticOperations.cpp in Sources */,
                                261516D615B0E60500A2C201 /* SetAndUpdateCacheModel.mm in Sources */,
                                52B8CF9615868CF000281053 /* SetDocumentURI.mm in Sources */,
+                               939BFE3A18E5548900883275 /* StringTruncator.mm in Sources */,
                                7C8DDAAB1735DEEE00EA5AC0 /* CloseThenTerminate.cpp in Sources */,
                                CDC2C71517970DDB00E627FB /* TimeRanges.cpp in Sources */,
                                51FCF79A1534AC6D00104491 /* ShouldGoToBackForwardListItem.cpp in Sources */,
diff --git a/Tools/TestWebKitAPI/Tests/mac/StringTruncator.mm b/Tools/TestWebKitAPI/Tests/mac/StringTruncator.mm
new file mode 100644 (file)
index 0000000..252eb05
--- /dev/null
@@ -0,0 +1,46 @@
+/*
+ * Copyright (C) 2014 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#import "config.h"
+#import "PlatformUtilities.h"
+
+#import <WebKit/WebStringTruncator.h>
+
+namespace TestWebKitAPI {
+
+TEST(WebKit1, StringTruncator)
+{
+    @autoreleasepool {
+        EXPECT_STREQ([[WebStringTruncator centerTruncateString:@"abcdefghijklmnopqrstuvwxyz" toWidth:100 withFont:[NSFont fontWithName:@"Helvetica" size:12]] UTF8String], "abcdef…tuvwxyz");
+        EXPECT_STREQ([[WebStringTruncator centerTruncateString:@"abcdefghijklmnopqrstuvwxyz" toWidth:100] UTF8String], "abcde…uvwxyz");
+        EXPECT_STREQ([[WebStringTruncator rightTruncateString:@"abcdefghijklmnopqrstuvwxyz" toWidth:100 withFont:[NSFont fontWithName:@"Helvetica" size:12]] UTF8String], "abcdefghijklmno…");
+        EXPECT_STREQ([[WebStringTruncator centerTruncateString:@"ābcdefghijklmnopqrstuvwxyz" toWidth:100 withFont:[NSFont fontWithName:@"Helvetica" size:12]] UTF8String], "ābcdefg…tuvwxyz");
+        EXPECT_STREQ([[WebStringTruncator rightTruncateString:@"ābcdefghijklmnopqrstuvwxyz" toWidth:100 withFont:[NSFont fontWithName:@"Helvetica" size:12]] UTF8String], "ābcdefghijklmno…");
+        EXPECT_EQ([WebStringTruncator widthOfString:@"abcdefghijklmnopqrstuvwxyz" font:[NSFont fontWithName:@"Helvetica" size:12]], 152.736328125);
+        EXPECT_EQ([WebStringTruncator widthOfString:@"ābcdefghijklmnopqrstuvwxyz" font:[NSFont fontWithName:@"Helvetica" size:12]], 152.736328125);
+    }
+}
+
+} // namespace TestWebKitAPI