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

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 95 matching lines @@
 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_list|, shrinks the fonts until
-// the primary font will fit within |height| while having its cap height
-// vertically centered.  Returns the |font_y_offset| needed to produce this
-// centering.
-void CalculateFontAndOffsetForHeight(int height,
-                                     gfx::FontList* font_list,
-                                     int* font_y_offset) {
-#if defined(OS_WIN)
-  base::win::ScopedGetDC screen_dc(NULL);
-#endif
-
+// Given a containing |height| and a |base_font_list|, shrinks the font size
+// until the font list will fit within |height| while having its cap height
+// vertically centered.  Returns the correctly-sized font list.
+gfx::FontList GetLargestFontListWithHeightBound(
+    const gfx::FontList& base_font_list,
+    int height) {
+  gfx::FontList font_list = base_font_list;
   while (true) {

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

nit: rewrite to avoid while(true), and/or return base_font_list at the bottom.

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

Done.

-    // TODO(pkasting): Expand the gfx::Font metrics (and underlying Skia
-    // metrics) enough to expose the cap height directly.
-#if defined(OS_WIN)
-    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;
-    *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_list->GetHeight()) / 2;
-#endif
-
-    const int font_size = font_list->GetFontSize();
-    if (((*font_y_offset >= 0) &&
-         ((*font_y_offset + font_list->GetHeight()) <= height)) ||
-        (font_size <= 1))
-      return;
-    *font_list = font_list->DeriveFontListWithSize(font_size - 1);
+    const int y_offset = (height - font_list.GetCapHeight()) / 2 -

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

nit: this calculation isn't entirely straightforward, can you add a comment?
An ASCII diagram of these measurements (in ui/gfx/font.h?) would be even better.

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

Done.

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

Done.

+        (font_list.GetBaseline() - font_list.GetCapHeight());
+    if (((y_offset >= 0) &&
+         (y_offset + font_list.GetHeight() <= height)) ||
+        (font_list.GetFontSize() <= 1))
+      return font_list;
+    font_list = font_list.DeriveFontListWithSizeDelta(-1);
   }
 }
 
 }  // namespace
 
 
 // LocationBarView -----------------------------------------------------------
 
 // static
 const int LocationBarView::kNormalEdgeThickness = 2;
@@ skipping 83 matching lines @@
 
   // Determine the main font.
   gfx::FontList font_list = ResourceBundle::GetSharedInstance().GetFontList(
       ResourceBundle::BaseFont);
   const int current_font_size = font_list.GetFontSize();
   const int desired_font_size = browser_defaults::kOmniboxFontPixelSize;
   if (current_font_size < desired_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);

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

nit: const.

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

Done.

-  int font_y_offset;
-  CalculateFontAndOffsetForHeight(location_height, &font_list, &font_y_offset);
+  font_list = GetLargestFontListWithHeightBound(font_list, location_height);
 
   // Determine the font for use inside the bubbles.
-  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

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

nit: combine this comment with the one above (wrap this on the line above).

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

Done.

   // overlap.
   const int kBubbleInteriorVerticalPadding = 1;
-  CalculateFontAndOffsetForHeight(
-      location_height - ((kBubblePadding + kBubbleInteriorVerticalPadding) * 2),
-      &bubble_font_list, &bubble_font_y_offset);
-  bubble_font_y_offset += kBubbleInteriorVerticalPadding;
+  const int bubble_vertical_padding =
+      (kBubblePadding + kBubbleInteriorVerticalPadding) * 2;
+  const gfx::FontList bubble_font_list(
+      GetLargestFontListWithHeightBound(
+          font_list, location_height - bubble_vertical_padding));
 
   const SkColor background_color =
       GetColor(ToolbarModel::NONE, LocationBarView::BACKGROUND);
   ev_bubble_view_ = new EVBubbleView(
-      bubble_font_list, bubble_font_y_offset,
-      GetColor(ToolbarModel::EV_SECURE, SECURITY_TEXT), background_color, this);
+      bubble_font_list, 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, profile_, command_updater(),
-                                          is_popup_mode_, this, font_list,
-                                          font_y_offset));
+                                          is_popup_mode_, this, font_list));
   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_list);
-  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_list, bubble_font_y_offset, text_color, background_color,
-      profile_);
+      bubble_font_list, text_color, background_color, profile_);
   AddChildView(selected_keyword_view_);
 
   suggested_text_view_ = new views::Label(string16(), font_list);
-  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_list, font_y_offset,
+      profile_, font_list,
       GetColor(ToolbarModel::NONE, LocationBarView::DEEMPHASIZED_TEXT),
       background_color);
   AddChildView(keyword_hint_view_);
 
   mic_search_view_ = new views::ImageButton(this);
   mic_search_view_->set_id(VIEW_ID_MIC_SEARCH_BUTTON);
   mic_search_view_->set_accessibility_focusable(true);
   mic_search_view_->SetTooltipText(
       l10n_util::GetStringUTF16(IDS_TOOLTIP_MIC_SEARCH));
   mic_search_view_->SetImage(
       views::Button::STATE_NORMAL,
       ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
           IDR_OMNIBOX_MIC_SEARCH));
   mic_search_view_->SetImageAlignment(views::ImageButton::ALIGN_CENTER,
                                       views::ImageButton::ALIGN_MIDDLE);
   mic_search_view_->SetVisible(false);
   InitTouchableLocationBarChildView(mic_search_view_);
   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_list, bubble_font_y_offset,
-                                    text_color, background_color);
+                                    bubble_font_list, text_color,
+                                    background_color);
     content_setting_views_.push_back(content_blocked_view);
     content_blocked_view->SetVisible(false);
     AddChildView(content_blocked_view);
   }
 
   generated_credit_card_view_ = new GeneratedCreditCardView(delegate_);
   AddChildView(generated_credit_card_view_);
 
   zoom_view_ = new ZoomView(delegate_);
   zoom_view_->set_id(VIEW_ID_ZOOM_BUTTON);
@@ skipping 448 matching lines @@
     gfx::Size suggested_text_size(suggested_text_view_->GetPreferredSize());
     if (suggested_text_size.width() > available_width ||
         text_direction == base::i18n::UNKNOWN_DIRECTION) {
       // Hide the suggested text if the user has scrolled or we can't fit all
       // the suggested text, or we have a mix of RTL and LTR characters.
       suggested_text_view_->SetBounds(0, 0, 0, 0);
     } else {
       location_needed_width =
           std::min(location_needed_width,
                    location_bounds.width() - suggested_text_size.width());
-      gfx::Rect suggested_text_bounds(location_bounds.origin(),
-                                      suggested_text_size);
+      gfx::Rect suggested_text_bounds(location_bounds.x(), location_bounds.y(),
+                                      suggested_text_size.width(),
+                                      location_bounds.height());
       // TODO(sky): figure out why this needs the -1.
       suggested_text_bounds.Offset(location_needed_width - 1, 0);
       // For non-views the omnibox needs to be shrunk so that the suggest text
       // is visible.
       if (!omnibox_views)
         location_bounds.set_width(location_needed_width);
 
       // We reverse the order of the location entry and suggested text if:
       // - Chrome is RTL but the text is fully LTR, or
       // - Chrome is LTR but the text is fully RTL.
@@ skipping 29 matching lines @@
     // All the target languages (IMEs) are LTR, and we do not need to support
     // RTL so far.  In other words, no testable RTL environment so far.
     int x = location_needed_width;
     if (width > entry_width)
       x = 0;
     else if (location_needed_width + width > entry_width)
       x = entry_width - width;
     location_bounds.set_width(x);
     ime_inline_autocomplete_view_->SetBounds(
         location_bounds.right(), location_bounds.y(),
-        std::min(width, entry_width),
-        ime_inline_autocomplete_view_->GetPreferredSize().height());
+        std::min(width, entry_width), location_bounds.height());
   }
 
   location_entry_view_->SetBoundsRect(location_bounds);
 }
 
 void LocationBarView::OnPaint(gfx::Canvas* canvas) {
   View::OnPaint(canvas);
 
   // Fill the location bar background color behind the border.  Parts of the
   // border images are meant to rest atop the toolbar background and parts atop
@@ skipping 629 matching lines @@
     bounds.Inset(-(horizontal_padding + 1) / 2, 0);
     location_bar_util::PaintExtensionActionBackground(
         *(*page_action_view)->image_view()->page_action(),
         tab_id, canvas, bounds, text_color, background_color);
   }
 }
 
 void LocationBarView::AccessibilitySetValue(const string16& new_value) {
   location_entry_->SetUserText(new_value);
 }