Supports FontList in Textfield.

chrome/browser/ui/views/location_bar/location_bar_view.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/location_bar/location_bar_view.h"
 
 #include <algorithm>
 #include <map>
 
 #include "base/command_line.h"
@@ skipping 76 matching lines @@
 #if defined(OS_WIN)
 #include "base/win/scoped_hdc.h"
 #include "base/win/scoped_select_object.h"
 #include "ui/native_theme/native_theme_win.h"
 #endif
 
 #if defined(OS_WIN) && !defined(USE_AURA)
 #include "chrome/browser/ui/views/omnibox/omnibox_view_win.h"
 #endif
 
+#if defined(OS_CHROMEOS)
+#include "grit/app_locale_settings.h"
+#endif
+
 #if !defined(OS_CHROMEOS)
 #include "chrome/browser/ui/views/first_run_bubble.h"
 #endif
 
 #if defined(USE_AURA)
 #include "ui/compositor/layer.h"
 #include "ui/compositor/scoped_layer_animation_settings.h"
 #endif
 
 using content::WebContents;
 using views::View;
 
 
 namespace {
 
 Browser* GetBrowserFromDelegate(LocationBarView::Delegate* delegate) {
   WebContents* contents = delegate->GetWebContents();
   return contents ? chrome::FindBrowserWithWebContents(contents) : NULL;
 }
 
-// Given a containing |height| and a base |font|, shrinks the font until it will
-// fit within |height| while having its cap height vertically centered.  Returns
-// the |font_y_offset| needed to produce this centering.
+// Given a containing |height| and a base |font_list|, shrinks the fonts until
+// it will fit within |height| while having its cap height vertically centered.

[Peter Kasting, 2013/07/26 20:54:27]

Nit: it -> the primary font

[Yuki, 2013/07/29 15:27:21]

Done.

+// Returns the |font_y_offset| needed to produce this centering.
 void CalculateFontAndOffsetForHeight(int height,
-                                     gfx::Font* font,
+                                     gfx::FontList* font_list,
                                      int* font_y_offset) {
 #if defined(OS_WIN)
   base::win::ScopedGetDC screen_dc(NULL);
 #endif
 
   while (true) {
     // TODO(pkasting): Expand the gfx::Font metrics (and underlying Skia
     // metrics) enough to expose the cap height directly.
 #if defined(OS_WIN)
-    base::win::ScopedSelectObject font_in_dc(screen_dc, font->GetNativeFont());
+    const gfx::Font& font = font_list->GetPrimaryFont();
+    base::win::ScopedSelectObject font_in_dc(screen_dc, font.GetNativeFont());
     TEXTMETRIC tm = {0};
     GetTextMetrics(screen_dc, &tm);
-    int cap_height = font->GetBaseline() - tm.tmInternalLeading;
+    int cap_height = font.GetBaseline() - tm.tmInternalLeading;
     *font_y_offset = ((height - cap_height) / 2) - tm.tmInternalLeading;
 #else
     // Without cap height available, we fall back to centering the full height.
-    *font_y_offset = (height - font->GetHeight()) / 2;
+    *font_y_offset = (height - font_list->GetHeight()) / 2;
 #endif
 
     if (((*font_y_offset >= 0) &&
-         ((*font_y_offset + font->GetHeight()) <= height)) ||
-        (font->GetFontSize() <= 1))
+         ((*font_y_offset + font_list->GetHeight()) <= height)) ||
+        (font_list->GetPrimaryFont().GetFontSize() <= 1))

[Peter Kasting, 2013/07/26 20:54:27]

Nit: Pull font_list->GetPrimaryFont().GetFontSize() out into a temp

[msw, 2013/07/26 21:19:57]

Should FontList have a GetFontSize()?

[Yuki, 2013/07/29 15:27:21]

Done.

[Yuki, 2013/07/29 15:27:21]

Done.

       return;
-    *font = font->DeriveFont(-1);
+    *font_list = font_list->DeriveFontListWithSize(
+        font_list->GetPrimaryFont().GetFontSize() - 1);
   }
 }
 
 }  // namespace
 
 
 // LocationBarView -----------------------------------------------------------
 
 // static
 const int LocationBarView::kNormalEdgeThickness = 2;
@@ skipping 71 matching lines @@
 void LocationBarView::Init() {
   // We need to be in a Widget, otherwise GetNativeTheme() may change and we're
   // not prepared for that.
   DCHECK(GetWidget());
 
   location_icon_view_ = new LocationIconView(this);
   location_icon_view_->set_drag_controller(this);
   AddChildView(location_icon_view_);
 
   // Determine the main font.
-  gfx::Font font(ui::ResourceBundle::GetSharedInstance().GetFont(
+#if defined(OS_CHROMEOS)
+  gfx::FontList font_list(l10n_util::GetStringUTF8(IDS_UI_FONT_FAMILY_CROS));
+#else
+  gfx::FontList font_list(ui::ResourceBundle::GetSharedInstance().GetFont(

[Peter Kasting, 2013/07/26 20:54:27]

We make this sort of call many places in the frontend.  Perhaps instead of an
#if here there should be a different interface somewhere to get the "base font
list", that all views code should call?

[msw, 2013/07/26 21:19:57]

I agree; this code should not choose/create the initial FontList in any way
different from other Views::Textfields (and eventually other controls/UI).
Instead, this code should start with the same initial FontList value that any
other Views::Textfield would, and then make the necessary modifications.

It seems like there are 3 ways of doing this:
1) Duplicate this FontList init logic in Views::Textfield or
Views::NativeTextfieldViews (please don't duplicate...)
2) Create a static Views::Textfield::GetDefaultFontList() or similar to use here
and in the Views::Textfield or Views::NativeTextfieldViews init.
3) Restructure this code to init the Omnibox View first without a FontList and
font_y_offset arg, then get the Omnibox's default FontList (via its
Views::Textfield subclass' new font_list() accessor) and modify it as needed
here, then set the modified value on the Omnibox instance.

I highly discourage (1), I like (3) but it might be harder to get right, so (2)
seems okay. What do you think?

[Peter Kasting, 2013/07/26 22:17:43]

I think (2) and (3) are both workable.  I didn't do something like (3) in the
existing code mostly because I didn't need it and I wanted to keep the API as
simple and direct as possible, and passing arguments in the constructor is
always better than setting them later.

The only consideration I'd keep in mind here would be that if we ultimately
rewrite RenderText or Textfields or whatever to do the vertical centering the
way the omnibox wants it automatically (as part of fixing the issues where big
glyphs push everything way far down), it might be easier to ask these classes
for an appropriate height/baseline instead of trying to do the calculation from
a font list directly.  But I don't know if that should influence the immediate
solution here.

[Yuki, 2013/07/29 15:27:21]

I've introduced ResourceBundle::GetFontList(style) which returns a font list of
the specified style.  The method uses IDS_UI_FONT_FAMILY_CROS #if CrOS.

I'll fix unit tests which use ResourceBundle::Delegate tomorrow (or at the
soonest possible time).

       ui::ResourceBundle::BaseFont));
-  const int current_font_size = font.GetFontSize();
+#endif
+  const int current_font_size = font_list.GetPrimaryFont().GetFontSize();
   const int desired_font_size = browser_defaults::kOmniboxFontPixelSize;
   if (current_font_size < desired_font_size)
-    font = font.DeriveFont(desired_font_size - current_font_size);
+    font_list = font_list.DeriveFontListWithSize(desired_font_size);
   // Shrink large fonts to make them fit.
   // TODO(pkasting): Stretch the location bar instead in this case.
   int location_height = GetInternalHeight(true);
   int font_y_offset;
-  CalculateFontAndOffsetForHeight(location_height, &font, &font_y_offset);
+  CalculateFontAndOffsetForHeight(location_height, &font_list, &font_y_offset);
 
   // Determine the font for use inside the bubbles.
-  gfx::Font bubble_font(font);
+  gfx::FontList bubble_font_list(font_list);
   int bubble_font_y_offset;
   // The bubble background images have 1 px thick edges, which we don't want to
   // overlap.
   const int kBubbleInteriorVerticalPadding = 1;
   CalculateFontAndOffsetForHeight(
       location_height - ((kBubblePadding + kBubbleInteriorVerticalPadding) * 2),
-      &bubble_font, &bubble_font_y_offset);
+      &bubble_font_list, &bubble_font_y_offset);
   bubble_font_y_offset += kBubbleInteriorVerticalPadding;
 
   const SkColor background_color =
       GetColor(ToolbarModel::NONE, LocationBarView::BACKGROUND);
   ev_bubble_view_ = new EVBubbleView(
-      bubble_font, bubble_font_y_offset,
+      bubble_font_list.GetPrimaryFont(), bubble_font_y_offset,
       GetColor(ToolbarModel::EV_SECURE, SECURITY_TEXT), background_color, this);
   ev_bubble_view_->set_drag_controller(this);
   AddChildView(ev_bubble_view_);
 
   // Initialize the Omnibox view.
   location_entry_.reset(CreateOmniboxView(this, model_, profile_,
-      command_updater_, is_popup_mode_, this, font, font_y_offset));
+      command_updater_, is_popup_mode_, this, font_list, font_y_offset));
   SetLocationEntryFocusable(true);
   location_entry_view_ = location_entry_->AddToView(this);
 
   // Initialize the inline autocomplete view which is visible only when IME is
   // turned on.  Use the same font with the omnibox and highlighted background.
-  ime_inline_autocomplete_view_ = new views::Label(string16(), font);
+  ime_inline_autocomplete_view_ = new views::Label(string16(),
+                                                   font_list.GetPrimaryFont());

[Peter Kasting, 2013/07/26 20:54:27]

Nit: We seem to call this repeatedly, consider creating a gfx::Font (or const
gfx::Font&? not sure) temp to hold it called |font|.  This will reduce the diff
too.

Maybe do the same for |bubble_font|.

[Yuki, 2013/07/29 15:27:21]

Done.

   ime_inline_autocomplete_view_->set_border(
       views::Border::CreateEmptyBorder(font_y_offset, 0, 0, 0));
   ime_inline_autocomplete_view_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
   ime_inline_autocomplete_view_->SetAutoColorReadabilityEnabled(false);
   ime_inline_autocomplete_view_->set_background(
       views::Background::CreateSolidBackground(GetNativeTheme()->GetSystemColor(
           ui::NativeTheme::kColorId_TextfieldSelectionBackgroundFocused)));
   ime_inline_autocomplete_view_->SetEnabledColor(
       GetNativeTheme()->GetSystemColor(
           ui::NativeTheme::kColorId_TextfieldSelectionColor));
   ime_inline_autocomplete_view_->SetVisible(false);
   AddChildView(ime_inline_autocomplete_view_);
 
   const SkColor text_color = GetColor(ToolbarModel::NONE, TEXT);
   selected_keyword_view_ = new SelectedKeywordView(
-      bubble_font, bubble_font_y_offset, text_color, background_color,
-      profile_);
+      bubble_font_list.GetPrimaryFont(), bubble_font_y_offset, text_color,
+      background_color, profile_);
   AddChildView(selected_keyword_view_);
 
-  suggested_text_view_ = new views::Label(string16(), font);
+  suggested_text_view_ = new views::Label(string16(),
+                                          font_list.GetPrimaryFont());
   suggested_text_view_->set_border(
       views::Border::CreateEmptyBorder(font_y_offset, 0, 0, 0));
   suggested_text_view_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
   suggested_text_view_->SetAutoColorReadabilityEnabled(false);
   suggested_text_view_->SetEnabledColor(GetColor(
       ToolbarModel::NONE, LocationBarView::DEEMPHASIZED_TEXT));
   suggested_text_view_->SetVisible(false);
   AddChildView(suggested_text_view_);
 
   keyword_hint_view_ = new KeywordHintView(
-      profile_, font, font_y_offset,
+      profile_, font_list.GetPrimaryFont(), font_y_offset,
       GetColor(ToolbarModel::NONE, LocationBarView::DEEMPHASIZED_TEXT),
       background_color);
   AddChildView(keyword_hint_view_);
 
   mic_search_view_ = new MicSearchView(this);
   mic_search_view_->SetVisible(false);
   AddChildView(mic_search_view_);
 
   for (int i = 0; i < CONTENT_SETTINGS_NUM_TYPES; ++i) {
     ContentSettingImageView* content_blocked_view =
         new ContentSettingImageView(static_cast<ContentSettingsType>(i), this,
-                                    bubble_font, bubble_font_y_offset,
-                                    text_color, background_color);
+                                    bubble_font_list.GetPrimaryFont(),
+                                    bubble_font_y_offset, text_color,
+                                    background_color);
     content_setting_views_.push_back(content_blocked_view);
     content_blocked_view->SetVisible(false);
     AddChildView(content_blocked_view);
   }
 
   autofill_credit_card_view_ = new AutofillCreditCardView(model_, delegate_);
   AddChildView(autofill_credit_card_view_);
 
   zoom_view_ = new ZoomView(model_, delegate_);
   zoom_view_->set_id(VIEW_ID_ZOOM_BUTTON);
@@ skipping 1170 matching lines @@
 int LocationBarView::GetInternalHeight(bool use_preferred_size) {
   int total_height =
       use_preferred_size ? GetPreferredSize().height() : height();
   return std::max(total_height - (vertical_edge_thickness() * 2), 0);
 }
 
 bool LocationBarView::HasValidSuggestText() const {
   return suggested_text_view_->visible() &&
       !suggested_text_view_->size().IsEmpty();
 }