Uses and returns the fractional width in text eliding

ui/gfx/canvas_unittest_mac.mm

Index: ui/gfx/canvas_unittest_mac.mm
diff --git a/ui/gfx/canvas_unittest_mac.mm b/ui/gfx/canvas_unittest_mac.mm
index d607218c64fe432969faf70f02858dc14f4c3115..aceec00535cd50d6bd539455aa75d9dca2512990 100644
--- a/ui/gfx/canvas_unittest_mac.mm
+++ b/ui/gfx/canvas_unittest_mac.mm
@@ -4,8 +4,6 @@
 
 #include "ui/gfx/canvas.h"
 
-#include <cmath>
-
 #import <Cocoa/Cocoa.h>
 
 #include "base/strings/utf_string_conversions.h"
@@ -16,69 +14,65 @@
 
 namespace gfx {
 
-namespace {
-
-// Mac-specific code for string size computations. This is a verbatim copy
-// of the old implementation that used to be in canvas_mac.mm.
-void CanvasMac_SizeStringInt(const base::string16& text,

[Alexei Svitkine (slow), 2013/10/01 19:04:55]

I'd rather you keep this as a standalone function.

The goal of this test is to compare that this implementation is consistent with
the canvas_skia.cc one.

[msw, 2013/10/01 19:14:53]

Why do we care? What's so great about this implementation? It's not in actual
production use elsewhere, so it's functionality has no real value. If we wish to
check that we match sizeWithAttributes, that's a bit more worthwhile, but why
would we need a standalone function for that? (that's all I have to say, I'll
concede to whatever Alexei recommends at this point)

[Alexei Svitkine (slow), 2013/10/01 19:19:50]

I'm okay with changing the signature / simplifying this function - I agree that
ultimately we want to check vs -sizeWithAttributes. However, CompareSizes()
below with all of that inlines is quite messy - so I'd rather have a separate
function that does the Mac code and returns the result of -sizeWithAttributes.

[jianli, 2013/10/01 21:34:28]

Added back the helper function with simplification change.

-                             const FontList& font_list,
-                             int* width,
-                             int* height,
-                             int line_height,
-                             int flags) {
-  DLOG_IF(WARNING, line_height != 0) << "Line heights not implemented.";
-  DLOG_IF(WARNING, flags & Canvas::MULTI_LINE) << "Multi-line not implemented.";
-
-  NSFont* native_font = font_list.GetPrimaryFont().GetNativeFont();
-  NSString* ns_string = base::SysUTF16ToNSString(text);
-  NSDictionary* attributes =
-      [NSDictionary dictionaryWithObject:native_font
-                                  forKey:NSFontAttributeName];
-  NSSize string_size = [ns_string sizeWithAttributes:attributes];
-  *width = std::ceil(string_size.width);
-  *height = font_list.GetHeight();
-}
-
-}  // namespace
-
 class CanvasTestMac : public testing::Test {
  protected:
-  // Compare the size returned by Canvas::SizeStringInt to the size generated
-  // by the platform-specific version in CanvasMac_SizeStringInt. Will generate
-  // expectation failure on any mismatch. Only works for single-line text
-  // without specified line height, since that is all the platform
+  // Compare the size returned by Canvas::SizeStringToFit to the size generated
+  // by the platform-specific version in CanvasMac_SizeStringToFit. Will
+  // generate expectation failure on any mismatch. Only works for single-line
+  // text without specified line height, since that is all the platform
   // implementation supports.
   void CompareSizes(const char* text) {
-    const int kReallyLargeNumber = 12345678;
+    const float kReallyLargeNumber = 12345678;
     FontList font_list(font_);
     base::string16 text16 = base::UTF8ToUTF16(text);
 
-    int mac_width = kReallyLargeNumber;
-    int mac_height = kReallyLargeNumber;
-    CanvasMac_SizeStringInt(text16, font_list, &mac_width, &mac_height, 0, 0);
-
-    int canvas_width = kReallyLargeNumber;
-    int canvas_height = kReallyLargeNumber;
-    Canvas::SizeStringInt(
+    // Compute the size via Mac-specific code.
+    NSFont* native_font = font_list.GetPrimaryFont().GetNativeFont();
+    NSString* ns_string = base::SysUTF16ToNSString(text16);
+    NSDictionary* attributes =
+        [NSDictionary dictionaryWithObject:native_font
+                                    forKey:NSFontAttributeName];
+    float mac_width = [ns_string sizeWithAttributes:attributes].width;
+    int mac_height = font_list.GetHeight();
+
+    // Compute the size via Canvas function.
+    float canvas_width = kReallyLargeNumber;
+    float canvas_height = kReallyLargeNumber;
+    Canvas::SizeStringToFit(
         text16, font_list, &canvas_width, &canvas_height, 0, 0);
 
     EXPECT_NE(kReallyLargeNumber, mac_width) << "no width for " << text;
     EXPECT_NE(kReallyLargeNumber, mac_height) << "no height for " << text;
     EXPECT_EQ(mac_width, canvas_width) << " width for " << text;
-    EXPECT_EQ(mac_height, canvas_height) << " height for " << text;
+    // FontList::GetHeight returns a truncated height.
+    EXPECT_EQ(mac_height,
+              static_cast<int>(canvas_height)) << " height for " << text;
   }
 
  private:
   Font font_;
 };
 
- // Tests that Canvas' SizeStringInt yields result consistent with a native
- // implementation.
- TEST_F(CanvasTestMac, StringSizeIdenticalForSkia) {
+// Tests that Canvas' SizeStringToFit yields result consistent with a native
+// implementation.
+TEST_F(CanvasTestMac, StringSizeIdenticalForSkia) {
   CompareSizes("");
   CompareSizes("Foo");
   CompareSizes("Longword");
   CompareSizes("This is a complete sentence.");
 }
 
+TEST_F(CanvasTestMac, FractionalWidth) {
+  const float kReallyLargeNumber = 12345678;
+  float width = kReallyLargeNumber;
+  float height = kReallyLargeNumber;
+
+  Font font;

[msw, 2013/10/01 19:14:53]

nit: Just construct a FontList here instead of constructing a FontList from a
Font in the call below.

[jianli, 2013/10/01 21:34:28]

Done.

+  Canvas::SizeStringToFit(
+        base::UTF8ToUTF16("Test"), FontList(font), &width, &height, 0, 0);
+
+  EXPECT_GT(width - static_cast<int>(width), 0);
+

[Alexei Svitkine (slow), 2013/10/01 19:04:55]

Nit: Remove empty line.

[jianli, 2013/10/01 21:34:28]

Done.

+}
+
 }  // namespace gfx