This caused failures in ui_unittests FontListTest.Fonts_GetHeight_GetBaseline

http://build.chromium.org/p/chromium.mac/buildstatus?builder=Mac%2010.6%20Tests%20%28dbg%29%284%29&number=30356
http://build.chromium.org/p/chromium.mac/buildstatus?builder=Mac%2010.7%20Tests%20%28dbg%29%284%29&number=13543

Revert 211664 "Shows Japanese and English mixed queries correctly."

> Shows Japanese and English mixed queries correctly.
> 
> This CL respects the common height and baseline of the fonts in the given font list, and draw text according to the baseline.
> 
> The cause of vertically-misaligned queries was that
> 1. ASCII characters have 23 pixels in height
> 2. Japanese characters have 17 pixels in height
> 
> pango_layout_get_pixel_size(), which is called in RenderTextLinux::GetStringSize(), returns the above size.
> 
> Also see:
> https://docs.google.com/a/chromium.org/document/d/1e2n9lEM_usn37Pld8tMeo_qpfLRQQjkx-O1hTHi3sPo/edit?usp=sharing
> 
> BUG=244323
> TEST=Test manually.
> 
> Review URL: https://chromiumcodereview.appspot.com/18848002

TBR=yukishiino@chromium.org

Review URL: https://codereview.chromium.org/19270002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@211669 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 11 insertions, 85 deletions

diff --git a/ui/gfx/font_list.cc b/ui/gfx/font_list.cc
index bbf79af..93bf73e 100644
--- a/ui/gfx/font_list.cc
+++ b/ui/gfx/font_list.cc
@@ -69,21 +69,19 @@ std::string BuildFontDescription(const std::vector<std::string>& font_names,
 
 namespace gfx {
 
-FontList::FontList() : common_height_(-1), common_baseline_(-1) {
+FontList::FontList() {
   fonts_.push_back(Font());
 }
 
 FontList::FontList(const std::string& font_description_string)
-    : font_description_string_(font_description_string),
-      common_height_(-1),
-      common_baseline_(-1) {
+    : font_description_string_(font_description_string) {
   DCHECK(!font_description_string.empty());
   // DCHECK description string ends with "px" for size in pixel.
   DCHECK(EndsWith(font_description_string, "px", true));
 }
 
 FontList::FontList(const std::vector<Font>& fonts)
-    : fonts_(fonts), common_height_(-1), common_baseline_(-1) {
+    : fonts_(fonts) {
   DCHECK(!fonts.empty());
   if (DCHECK_IS_ON()) {
     int style = fonts[0].GetStyle();
@@ -95,8 +93,7 @@ FontList::FontList(const std::vector<Font>& fonts)
   }
 }
 
-FontList::FontList(const Font& font)
-    : common_height_(-1), common_baseline_(-1) {
+FontList::FontList(const Font& font) {
   fonts_.push_back(font);
 }
 
@@ -149,34 +146,6 @@ FontList FontList::DeriveFontListWithSize(int size) const {
   return FontList(BuildFontDescription(font_names, font_style, size));
 }
 
-int FontList::GetHeight() const {
-  if (common_height_ < 0) {
-    int ascent = 0;
-    int descent = 0;
-    const std::vector<Font>& fonts = GetFonts();
-    for (std::vector<Font>::const_iterator i = fonts.begin();
-         i != fonts.end(); ++i) {
-      ascent = std::max(ascent, i->GetBaseline());
-      descent = std::max(descent, i->GetHeight() - i->GetBaseline());
-    }
-    common_height_ = ascent + descent;
-  }
-  return common_height_;
-}
-
-int FontList::GetBaseline() const {
-  if (common_baseline_ < 0) {
-    int baseline = 0;
-    const std::vector<Font>& fonts = GetFonts();
-    for (std::vector<Font>::const_iterator i = fonts.begin();
-         i != fonts.end(); ++i) {
-      baseline = std::max(baseline, i->GetBaseline());
-    }
-    common_baseline_ = baseline;
-  }
-  return common_baseline_;
-}
-
 int FontList::GetFontStyle() const {
   if (!fonts_.empty())
     return fonts_[0].GetStyle();
diff --git a/ui/gfx/font_list.h b/ui/gfx/font_list.h
index f573c1e..d070b5b 100644
--- a/ui/gfx/font_list.h
+++ b/ui/gfx/font_list.h
@@ -8,7 +8,6 @@
 #include <string>
 #include <vector>
 
-#include "base/gtest_prod_util.h"
 #include "ui/base/ui_export.h"
 #include "ui/gfx/font.h"
 
@@ -58,14 +57,6 @@ class UI_EXPORT FontList {
   // given font |size| in pixels.
   FontList DeriveFontListWithSize(int size) const;
 
-  // Returns the height of this font list, which is max(ascent) + max(descent)
-  // for all the fonts in the font list.
-  int GetHeight() const;
-
-  // Returns the baseline of this font list, which is max(baseline) for all the
-  // fonts in the font list.
-  int GetBaseline() const;
-
   // Returns the |gfx::Font::FontStyle| style flags for this font list.
   int GetFontStyle() const;
 
@@ -78,8 +69,6 @@ class UI_EXPORT FontList {
   const std::vector<Font>& GetFonts() const;
 
  private:
-  FRIEND_TEST_ALL_PREFIXES(FontListTest, Fonts_GetHeight_GetBaseline);
-
   // A vector of Font. If FontList is constructed with font description string,
   // |fonts_| is not initialized during construction. Instead, it is computed
   // lazily when user asked to get the font vector.
@@ -92,10 +81,6 @@ class UI_EXPORT FontList {
   // |font_description_string_| is not initialized during construction. Instead,
   // it is computed lazily when user asked to get the font description string.
   mutable std::string font_description_string_;
-
-  // The cached common height and baseline of the fonts in the font list.
-  mutable int common_height_;
-  mutable int common_baseline_;
 };
 
 }  // namespace gfx
diff --git a/ui/gfx/font_list_unittest.cc b/ui/gfx/font_list_unittest.cc
index b66cd8a..ed34a0d 100644
--- a/ui/gfx/font_list_unittest.cc
+++ b/ui/gfx/font_list_unittest.cc
@@ -232,30 +232,4 @@ TEST(FontListTest, Fonts_DeriveFontListWithSize) {
   EXPECT_EQ("Sans serif|5", FontToString(derived_fonts[1]));
 }
 
-TEST(FontListTest, Fonts_GetHeight_GetBaseline) {
-  // If the font list has only one font, the height and baseline must be
-  // the same.
-  Font font_arial8("Arial", 8);
-  FontList font_list_arial8("Arial, 8px");
-  EXPECT_EQ(font_arial8.GetHeight(), font_list_arial8.GetHeight());
-  EXPECT_EQ(font_arial8.GetBaseline(), font_list_arial8.GetBaseline());
-
-  Font font_sansserif10("Sans serif", 10);
-  FontList font_list_sansserif10("Sans serif, 10px");
-  EXPECT_EQ(font_sansserif10.GetHeight(), font_list_sansserif10.GetHeight());
-  EXPECT_EQ(font_sansserif10.GetBaseline(),
-            font_list_sansserif10.GetBaseline());
-
-  // If there are two different fonts, the font list returns the max value.
-  FontList font_list_mix(font_arial8);
-  // Bypass DCHECK in a constructor of FontList which checks that all the fonts
-  // have the same font size.
-  font_list_mix.fonts_.push_back(font_sansserif10);
-  // Helvetica 20px must be larger than Arial 10px.
-  EXPECT_GT(font_sansserif10.GetHeight(), font_arial8.GetHeight());
-  EXPECT_EQ(font_sansserif10.GetHeight(), font_list_mix.GetHeight());
-  EXPECT_GT(font_sansserif10.GetBaseline(), font_arial8.GetBaseline());
-  EXPECT_EQ(font_sansserif10.GetBaseline(), font_list_mix.GetBaseline());
-}
-
 }  // namespace gfx
diff --git a/ui/gfx/render_text.cc b/ui/gfx/render_text.cc
index 5c822e8..d3c6bea 100644
--- a/ui/gfx/render_text.cc
+++ b/ui/gfx/render_text.cc
@@ -357,7 +357,9 @@ void RenderText::SetFont(const Font& font) {
 }
 
 void RenderText::SetFontSize(int size) {
-  SetFontList(font_list_.DeriveFontListWithSize(size));
+  font_list_ = font_list_.DeriveFontListWithSize(size);
+  cached_bounds_and_offset_valid_ = false;
+  ResetLayout();
 }
 
 void RenderText::SetCursorEnabled(bool cursor_enabled) {
diff --git a/ui/gfx/render_text_linux.cc b/ui/gfx/render_text_linux.cc
index 004064d..e6a1ab7 100644
--- a/ui/gfx/render_text_linux.cc
+++ b/ui/gfx/render_text_linux.cc
@@ -83,17 +83,12 @@ Size RenderTextLinux::GetStringSize() {
   EnsureLayout();
   int width = 0, height = 0;
   pango_layout_get_pixel_size(layout_, &width, &height);
-  // Keep a consistent height between this particular string's PangoLayout and
-  // potentially larger text supported by the FontList.
-  return Size(width, std::max(height, font_list().GetHeight()));
+  return Size(width, height);
 }
 
 int RenderTextLinux::GetBaseline() {
   EnsureLayout();
-  // Keep a consistent baseline between this particular string's PangoLayout and
-  // potentially larger text supported by the FontList.
-  return std::max(PANGO_PIXELS(pango_layout_get_baseline(layout_)),
-                  font_list().GetBaseline());
+  return PANGO_PIXELS(pango_layout_get_baseline(layout_));
 }
 
 SelectionModel RenderTextLinux::FindCursorPosition(const Point& point) {
@@ -369,7 +364,8 @@ void RenderTextLinux::DrawVisualText(Canvas* canvas) {
   DCHECK(layout_);
 
   // Skia will draw glyphs with respect to the baseline.
-  Vector2d offset(GetTextOffset() + Vector2d(0, GetBaseline()));
+  Vector2d offset(GetTextOffset() +
+      Vector2d(0, PANGO_PIXELS(pango_layout_get_baseline(layout_))));
 
   SkScalar x = SkIntToScalar(offset.x());
   SkScalar y = SkIntToScalar(offset.y());