[omnibox] Use new text context instead of an explicit size delta

To deemphasize answer descriptions, a font with explicit size
delta was being used, resulting in two font fetches.  Now there
is a new entry in ChromeTextContext so only one is needed.
The appearance and behavior should remain the same, but this
follow-up CL simplifies API usage, speed, and maintainability.

Change-Id: I3a171cddf7b6e1620879b76239cfdf4f975615a1

Bug: 838733
Change-Id: I3a171cddf7b6e1620879b76239cfdf4f975615a1
Reviewed-on: https://chromium-review.googlesource.com/1142019Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576412}

chrome/browser/ui/views/harmony/chrome_typography.cc

@@ -51,7 +51,8 @@ void ApplyCommonFontStyles(int context,
                            gfx::Font::Weight* weight) {
   switch (context) {
 #if !defined(OS_MACOSX) || BUILDFLAG(MAC_VIEWS_BROWSER)
-    case CONTEXT_OMNIBOX_PRIMARY: {
+    case CONTEXT_OMNIBOX_PRIMARY:
+    case CONTEXT_OMNIBOX_DEEMPHASIZED: {
       constexpr int kDesiredFontSizeRegular = 14;
       constexpr int kDesiredFontSizeTouchable = 15;
       static const int omnibox_primary_delta =
@@ -61,6 +62,9 @@ void ApplyCommonFontStyles(int context,
                   ? kDesiredFontSizeTouchable
                   : kDesiredFontSizeRegular);
       *size_delta = omnibox_primary_delta;
+      if (context == CONTEXT_OMNIBOX_DEEMPHASIZED) {
+        (*size_delta)--;
+      }
       break;
     }
     case CONTEXT_OMNIBOX_DECORATION: {

chrome/browser/ui/views/harmony/chrome_typography.h

@@ -37,6 +37,9 @@ enum ChromeTextContext {
   // Text that goes inside location bar decorations such as the keyword hint.
   CONTEXT_OMNIBOX_DECORATION,
 
+  // Text in omnibox answer results that is slightly smaller than primary font.
+  CONTEXT_OMNIBOX_DEEMPHASIZED,
+
   // Text for titles, body text and buttons that appear in dialogs attempting to
   // mimic the native Windows 10 look and feel.
   CONTEXT_WINDOWS10_NATIVE,

chrome/browser/ui/views/omnibox/omnibox_result_view.cc

@@ -145,7 +145,7 @@ void OmniboxResultView::Invalidate() {
     if (reverse) {
       suggestion_view_->content()->SetText(match_.answer->second_line());
       suggestion_view_->description()->SetText(match_.contents,
-                                               match_.contents_class, -1);
+                                               match_.contents_class, true);
       suggestion_view_->description()->AppendExtraText(
           match_.answer->first_line());
     } else {
@@ -153,7 +153,7 @@ void OmniboxResultView::Invalidate() {
                                            match_.contents_class);
       suggestion_view_->content()->AppendExtraText(match_.answer->first_line());
       suggestion_view_->description()->SetText(match_.answer->second_line(),
-                                               -1);
+                                               true);
     }
     // AppendExtraText has side effect on color, so explicitly set color.
     // TODO(orinj): Consolidate text color specification in one place.

chrome/browser/ui/views/omnibox/omnibox_text_view.cc

@@ -139,7 +139,7 @@ const gfx::FontList& GetFontForType(int text_type) {
 OmniboxTextView::OmniboxTextView(OmniboxResultView* result_view)
     : result_view_(result_view),
       font_height_(0),
-      font_size_delta_(0),
+      use_deemphasized_font_(false),
       wrap_text_lines_(false) {}
 
 OmniboxTextView::~OmniboxTextView() {}
@@ -193,17 +193,17 @@ const base::string16& OmniboxTextView::text() const {
   return render_text_->text();
 }
 
-void OmniboxTextView::SetText(const base::string16& text, int size_delta) {
+void OmniboxTextView::SetText(const base::string16& text, bool deemphasize) {
   if (cached_classifications_) {
     cached_classifications_.reset();
   } else if (render_text_ && render_text_->text() == text &&
-             size_delta == font_size_delta_) {
+             deemphasize == use_deemphasized_font_) {
     // Only exit early if |cached_classifications_| was empty,
     // i.e. the last time text was set was through this method.
     return;
   }
 
-  font_size_delta_ = size_delta;
+  use_deemphasized_font_ = deemphasize;
   render_text_.reset();
   render_text_ = CreateRenderText(text);
   UpdateLineHeight();
@@ -212,13 +212,13 @@ void OmniboxTextView::SetText(const base::string16& text, int size_delta) {
 
 void OmniboxTextView::SetText(const base::string16& text,
                               const ACMatchClassifications& classifications,
-                              int size_delta) {
+                              bool deemphasize) {
   if (render_text_ && render_text_->text() == text && cached_classifications_ &&
       classifications == *cached_classifications_ &&
-      size_delta == font_size_delta_)
+      deemphasize == use_deemphasized_font_)
     return;
 
-  font_size_delta_ = size_delta;
+  use_deemphasized_font_ = deemphasize;
 
   cached_classifications_ =
       std::make_unique<ACMatchClassifications>(classifications);
@@ -257,8 +257,8 @@ void OmniboxTextView::SetText(const base::string16& text,
 }
 
 void OmniboxTextView::SetText(const SuggestionAnswer::ImageLine& line,
-                              int size_delta) {
-  font_size_delta_ = size_delta;
+                              bool deemphasize) {
+  use_deemphasized_font_ = deemphasize;
   cached_classifications_.reset();
   wrap_text_lines_ = line.num_text_lines() > 1;
   render_text_.reset();
@@ -315,18 +315,11 @@ std::unique_ptr<gfx::RenderText> OmniboxTextView::CreateRenderText(
   render_text->SetDisplayRect(gfx::Rect(gfx::Size(INT_MAX, 0)));
   render_text->SetCursorEnabled(false);
   render_text->SetElideBehavior(gfx::ELIDE_TAIL);
-  const gfx::FontList& font =
-      views::style::GetFont(CONTEXT_OMNIBOX_PRIMARY, kTextStyle);
-  if (font_size_delta_ == 0) {
-    render_text->SetFontList(font);
-  } else {
-    const gfx::FontList& omnibox_font =
-        views::style::GetFont(CONTEXT_OMNIBOX_PRIMARY, kTextStyle);
-    render_text->SetFontList(
-        ui::ResourceBundle::GetSharedInstance().GetFontListWithDelta(
-            omnibox_font.GetFontSize() - gfx::FontList().GetFontSize() +
-            font_size_delta_));
-  }
+  const gfx::FontList& font = views::style::GetFont(
+      (use_deemphasized_font_ ? CONTEXT_OMNIBOX_DEEMPHASIZED
+                              : CONTEXT_OMNIBOX_PRIMARY),
+      kTextStyle);
+  render_text->SetFontList(font);
   render_text->SetText(text);
   return render_text;
 }

chrome/browser/ui/views/omnibox/omnibox_text_view.h

@@ -48,13 +48,13 @@ class OmniboxTextView : public views::View {
   // Sets the render text with default rendering for the given |text|. The
   // |classifications| are used to style the text. An ImageLine incorporates
   // both the text and the styling.
-  // The size_delta is specified here so it can be known in advance of creating
-  // the render text. Applying later would kill bold (clear weights BreakList).
-  void SetText(const base::string16& text, int size_delta = 0);
+  // |deemphasize| specifies whether to use a slightly smaller font than normal.
+  void SetText(const base::string16& text, bool deemphasize = false);
   void SetText(const base::string16& text,
                const ACMatchClassifications& classifications,
-               int font_size_delta = 0);
-  void SetText(const SuggestionAnswer::ImageLine& line, int size_delta = 0);
+               bool deemphasize = false);
+  void SetText(const SuggestionAnswer::ImageLine& line,
+               bool deemphasize = false);
 
   // Adds the "additional" and "status" text from |line|, if any.
   void AppendExtraText(const SuggestionAnswer::ImageLine& line);
@@ -82,8 +82,10 @@ class OmniboxTextView : public views::View {
   // Font settings for this view.
   int font_height_;
 
-  // Delta (in px) from default font size.
-  int font_size_delta_;
+  // Whether to apply deemphasized font instead of primary omnibox font.
+  // TODO(orinj): Use a more general ChromeTextContext for flexibility, or
+  // otherwise clean up & unify the different ways of selecting fonts & styles.
+  bool use_deemphasized_font_;
 
   // Whether to wrap lines if the width is too narrow for the whole string.
   bool wrap_text_lines_;