Always aligns text at vertically center (Textfield, Label).

ui/gfx/render_text_unittest.cc

Index: ui/gfx/render_text_unittest.cc
diff --git a/ui/gfx/render_text_unittest.cc b/ui/gfx/render_text_unittest.cc
index 75e985e01a8dc48870f31bb66397007e14adc145..b27d66d7db3cd9f2edacaf6ec31e3b9e9d6833a2 100644
--- a/ui/gfx/render_text_unittest.cc
+++ b/ui/gfx/render_text_unittest.cc
@@ -5,6 +5,7 @@
 #include "ui/gfx/render_text.h"
 
 #include <algorithm>
+#include <limits>
 
 #include "base/format_macros.h"
 #include "base/memory/scoped_ptr.h"
@@ -1139,6 +1140,8 @@ TEST_F(RenderTextTest, StringSizeEmptyString) {
   const FontList font_list("Arial,Symbol, 16px");
   scoped_ptr<RenderText> render_text(RenderText::CreateInstance());
   render_text->SetFontList(font_list);
+  render_text->SetDisplayRect(Rect(0, 0, std::numeric_limits<int>::max(),

[msw, 2013/10/23 01:18:00]

Hmm, an int max width shouldn't break anything, but you could just use 0 or some
arbitrary constant like 100 instead, for simplicity here and below.

[Yuki, 2013/10/24 14:32:54]

Done.

+                                   font_list.GetHeight()));
 
   // The empty string respects FontList metrics for non-zero height
   // and baseline.
@@ -1180,7 +1183,9 @@ TEST_F(RenderTextTest, StringSizeRespectsFontListMetrics) {
   // Check |smaller_font_text| is rendered with the smaller font.
   scoped_ptr<RenderText> render_text(RenderText::CreateInstance());
   render_text->SetText(UTF8ToUTF16(smaller_font_text));
-  render_text->SetFont(smaller_font);
+  render_text->SetFontList(FontList(smaller_font));
+  render_text->SetDisplayRect(Rect(0, 0, std::numeric_limits<int>::max(),
+                                   render_text->font_list().GetHeight()));
   EXPECT_EQ(smaller_font.GetHeight(), render_text->GetStringSize().height());
   EXPECT_EQ(smaller_font.GetBaseline(), render_text->GetBaseline());
 
@@ -1192,6 +1197,8 @@ TEST_F(RenderTextTest, StringSizeRespectsFontListMetrics) {
   fonts.push_back(larger_font);
   const FontList font_list(fonts);
   render_text->SetFontList(font_list);
+  render_text->SetDisplayRect(Rect(0, 0, std::numeric_limits<int>::max(),
+                                   render_text->font_list().GetHeight()));
   EXPECT_LT(smaller_font.GetHeight(), render_text->GetStringSize().height());
   EXPECT_LT(smaller_font.GetBaseline(), render_text->GetBaseline());
   EXPECT_EQ(font_list.GetHeight(), render_text->GetStringSize().height());
@@ -1300,14 +1307,13 @@ TEST_F(RenderTextTest, GetTextOffset) {
   Vector2d offset = render_text->GetLineOffset(0);
   EXPECT_TRUE(offset.IsZero());
 
-  // Set display area's size greater than font size.
-  const int kEnlargement = 2;
-  display_rect.Inset(0, 0, -kEnlargement, -kEnlargement);
+  // Widen display area's size by kEnlargementX.

[msw, 2013/10/23 01:18:00]

nit: "Widen the display area by |kEnlargementX|", but this comment is
superfluous anyway.

[Yuki, 2013/10/24 14:32:54]

Done.

+  const int kEnlargementX = 2;
+  display_rect.Inset(0, 0, -kEnlargementX, 0);
   render_text->SetDisplayRect(display_rect);
 
-  // Check the default horizontal and vertical alignment.
+  // Check the default horizontal alignment.
   offset = render_text->GetLineOffset(0);
-  EXPECT_EQ(kEnlargement / 2, offset.y());
   EXPECT_EQ(0, offset.x());
 
   // Check explicitly setting the horizontal alignment.
@@ -1316,21 +1322,23 @@ TEST_F(RenderTextTest, GetTextOffset) {
   EXPECT_EQ(0, offset.x());
   render_text->SetHorizontalAlignment(ALIGN_CENTER);
   offset = render_text->GetLineOffset(0);
-  EXPECT_EQ(kEnlargement / 2, offset.x());
+  EXPECT_EQ(kEnlargementX / 2, offset.x());
   render_text->SetHorizontalAlignment(ALIGN_RIGHT);
   offset = render_text->GetLineOffset(0);
-  EXPECT_EQ(kEnlargement, offset.x());
+  EXPECT_EQ(kEnlargementX, offset.x());
 
-  // Check explicitly setting the vertical alignment.
-  render_text->SetVerticalAlignment(ALIGN_TOP);
-  offset = render_text->GetLineOffset(0);
-  EXPECT_EQ(0, offset.y());
-  render_text->SetVerticalAlignment(ALIGN_VCENTER);
-  offset = render_text->GetLineOffset(0);
-  EXPECT_EQ(kEnlargement / 2, offset.y());
-  render_text->SetVerticalAlignment(ALIGN_BOTTOM);
+  // Enlarge the display height twice.  This allows the |render_text| puts the

[msw, 2013/10/23 01:18:00]

nit: simplify this comment to "Check that text is vertically centered within
taller display rects." and remove the other comment.

[Yuki, 2013/10/24 14:32:54]

Done.

+  // text at vertical center respecting cap height and regardless of height
+  // limit.
+  const int kEnlargementY = display_rect.height();
+  display_rect.Inset(0, 0, 0, -kEnlargementY);
+  render_text->SetDisplayRect(display_rect);
+  const Vector2d prev_offset = render_text->GetLineOffset(0);
+  // Enlarge the display height more and test the y offset.
+  display_rect.Inset(0, 0, 0, -2 * kEnlargementY);
+  render_text->SetDisplayRect(display_rect);
   offset = render_text->GetLineOffset(0);
-  EXPECT_EQ(kEnlargement, offset.y());
+  EXPECT_EQ(prev_offset.y() + kEnlargementY, offset.y());
 
   SetRTL(was_rtl);
 }