[omnibox] Reverse answer content & description, with styling

This CL changes the way answers reverse content and description
fields (the position/layout approach was incomplete). Font sizes
and colors are also applied to answers according to redline spec.

Bug: 855783
Change-Id: I39d619bbb729d2637f08d9c37fdfd5fb4938361a
Reviewed-on: https://chromium-review.googlesource.com/1137341Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575510}

chrome/browser/ui/omnibox/omnibox_theme.cc

@@ -246,7 +246,7 @@ SkColor GetOmniboxColor(OmniboxPart part,
           gfx::ToRoundedInt(GetOmniboxStateAlpha(state) * 0xff));
     case OmniboxPart::LOCATION_BAR_TEXT_DEFAULT:
     case OmniboxPart::RESULTS_TEXT_DEFAULT:
-      return dark ? gfx::kGoogleGrey100 : gfx::kGoogleGrey800;
+      return dark ? gfx::kGoogleGrey100 : gfx::kGoogleGrey900;
 
     case OmniboxPart::LOCATION_BAR_TEXT_DIMMED:
     case OmniboxPart::RESULTS_ICON:

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

@@ -318,9 +318,6 @@ void OmniboxMatchCellView::OnMatchUpdate(const OmniboxResultView* result_view,
        !match.image_url.empty());
   is_search_type_ = AutocompleteMatch::IsSearchType(match.type);
   has_tab_match_ = match.has_tab_match;
-  is_definition_ =
-      !!match.answer &&
-      match.answer->type() == SuggestionAnswer::ANSWER_TYPE_DICTIONARY;
 
   // Set up the small icon.
   if (is_rich_suggestion_) {
@@ -483,15 +480,9 @@ void OmniboxMatchCellView::LayoutNewStyleTwoLineSuggestion() {
     description_view_->SetSize(gfx::Size());
   } else {
     const int text_height = content_view_->GetLineHeight();
-    int content_y = y;
-    int description_y = y + text_height;
-    if (OmniboxFieldTrial::IsReverseAnswersEnabled() && !is_definition_) {
-      std::swap(content_y, description_y);
-    }
-    content_view_->SetBounds(x + kTextIndent, content_y, text_width,
-                             text_height);
+    content_view_->SetBounds(x + kTextIndent, y, text_width, text_height);
     description_view_->SetBounds(
-        x + kTextIndent, description_y, text_width,
+        x + kTextIndent, y + text_height, text_width,
         description_view_->GetHeightForWidth(text_width));
   }
 }

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

@@ -46,7 +46,6 @@ class OmniboxMatchCellView : public views::View {
   void LayoutNewStyleTwoLineSuggestion();
   void LayoutSplit(int icon_view_width, int text_indent);
 
-  bool is_definition_ = false;
   bool is_old_style_answer_;
   bool is_rich_suggestion_;
   bool is_search_type_;

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

@@ -120,8 +120,9 @@ void OmniboxResultView::Invalidate() {
   }
 
   // Reapply the dim color to account for the highlight state.
-  suggestion_view_->separator()->Dim();
-  keyword_view_->separator()->Dim();
+  const OmniboxPart dim = OmniboxPart::RESULTS_TEXT_DIMMED;
+  suggestion_view_->separator()->ApplyTextColor(dim);
+  keyword_view_->separator()->ApplyTextColor(dim);
   if (suggestion_tab_switch_button_)
     suggestion_tab_switch_button_->UpdateBackground();
 
@@ -135,15 +136,33 @@ void OmniboxResultView::Invalidate() {
       omnibox::kKeywordSearchIcon, GetLayoutConstant(LOCATION_BAR_ICON_SIZE),
       GetColor(OmniboxPart::RESULTS_ICON)));
 
-  // The content text is set to the match text and calculated classifications.
   // Answers use their own styling for additional content text and the
   // description text, whereas non-answer suggestions use the match text and
   // calculated classifications for the description text.
-  suggestion_view_->content()->SetText(match_.contents, match_.contents_class);
   if (match_.answer) {
-    suggestion_view_->content()->AppendExtraText(match_.answer->first_line());
-    suggestion_view_->description()->SetText(match_.answer->second_line());
+    if (OmniboxFieldTrial::IsReverseAnswersEnabled()) {
+      // Answers may swap the content and description fields to change emphasis.
+      // But even when fields swap, the font size and color changes should not.
+      OmniboxTextView* primary = suggestion_view_->content();
+      OmniboxTextView* secondary = suggestion_view_->description();
+      bool swap = !match_.IsExceptedFromLineReversal();
+      if (swap)
+        std::swap(primary, secondary);
+      primary->SetText(match_.contents, match_.contents_class, swap ? -1 : 0);
+      primary->AppendExtraText(match_.answer->first_line());
+      primary->ApplyTextColor(swap ? dim : OmniboxPart::RESULTS_TEXT_DEFAULT);
+      secondary->SetText(match_.answer->second_line(), swap ? 0 : -1);
+      secondary->ApplyTextColor(swap ? OmniboxPart::RESULTS_TEXT_DEFAULT : dim);
+    } else {
+      suggestion_view_->content()->SetText(match_.contents,
+                                           match_.contents_class);
+      suggestion_view_->content()->AppendExtraText(match_.answer->first_line());
+      suggestion_view_->description()->SetText(match_.answer->second_line());
+    }
   } else {
+    // Content and description use match text and calculated classifications.
+    suggestion_view_->content()->SetText(match_.contents,
+                                         match_.contents_class);
     suggestion_view_->description()->SetText(match_.description,
                                              match_.description_class);
   }
@@ -157,7 +176,7 @@ void OmniboxResultView::Invalidate() {
                                       keyword_match->contents_class);
     keyword_view_->description()->SetText(keyword_match->description,
                                           keyword_match->description_class);
-    keyword_view_->description()->Dim();
+    keyword_view_->description()->ApplyTextColor(dim);
   }
 }
 

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

@@ -137,7 +137,10 @@ const gfx::FontList& GetFontForType(int text_type) {
 }  // namespace
 
 OmniboxTextView::OmniboxTextView(OmniboxResultView* result_view)
-    : result_view_(result_view), font_height_(0), wrap_text_lines_(false) {}
+    : result_view_(result_view),
+      font_height_(0),
+      font_size_delta_(0),
+      wrap_text_lines_(false) {}
 
 OmniboxTextView::~OmniboxTextView() {}
 
@@ -179,9 +182,8 @@ void OmniboxTextView::OnPaint(gfx::Canvas* canvas) {
   render_text_->Draw(canvas);
 }
 
-void OmniboxTextView::Dim() {
-  render_text_->SetColor(
-      result_view_->GetColor(OmniboxPart::RESULTS_TEXT_DIMMED));
+void OmniboxTextView::ApplyTextColor(OmniboxPart part) {
+  render_text_->SetColor(result_view_->GetColor(part));
 }
 
 const base::string16& OmniboxTextView::text() const {
@@ -191,15 +193,17 @@ const base::string16& OmniboxTextView::text() const {
   return render_text_->text();
 }
 
-void OmniboxTextView::SetText(const base::string16& text) {
+void OmniboxTextView::SetText(const base::string16& text, int size_delta) {
   if (cached_classifications_) {
     cached_classifications_.reset();
-  } else if (render_text_ && render_text_->text() == text) {
+  } else if (render_text_ && render_text_->text() == text &&
+             size_delta == font_size_delta_) {
     // 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;
   render_text_.reset();
   render_text_ = CreateRenderText(text);
   UpdateLineHeight();
@@ -207,11 +211,15 @@ void OmniboxTextView::SetText(const base::string16& text) {
 }
 
 void OmniboxTextView::SetText(const base::string16& text,
-                              const ACMatchClassifications& classifications) {
+                              const ACMatchClassifications& classifications,
+                              int size_delta) {
   if (render_text_ && render_text_->text() == text && cached_classifications_ &&
-      classifications == *cached_classifications_)
+      classifications == *cached_classifications_ &&
+      size_delta == font_size_delta_)
     return;
 
+  font_size_delta_ = size_delta;
+
   cached_classifications_ =
       std::make_unique<ACMatchClassifications>(classifications);
   render_text_ = CreateRenderText(text);
@@ -248,11 +256,14 @@ void OmniboxTextView::SetText(const base::string16& text,
   SetPreferredSize(CalculatePreferredSize());
 }
 
-void OmniboxTextView::SetText(const SuggestionAnswer::ImageLine& line) {
+void OmniboxTextView::SetText(const SuggestionAnswer::ImageLine& line,
+                              int size_delta) {
+  font_size_delta_ = size_delta;
   cached_classifications_.reset();
   wrap_text_lines_ = line.num_text_lines() > 1;
   render_text_.reset();
   render_text_ = CreateRenderText(base::string16());
+
   if (!OmniboxFieldTrial::IsNewAnswerLayoutEnabled()) {
     // This assumes that the first text type in the line can be used to specify
     // the font for all the text fields in the line.  For now this works but
@@ -304,8 +315,13 @@ 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);
-  render_text->SetFontList(
-      views::style::GetFont(CONTEXT_OMNIBOX_PRIMARY, kTextStyle));
+  const gfx::FontList& font =
+      views::style::GetFont(CONTEXT_OMNIBOX_PRIMARY, kTextStyle);
+  if (font_size_delta_ == 0) {
+    render_text->SetFontList(font);
+  } else {
+    render_text->SetFontList(font.DeriveWithSizeDelta(font_size_delta_));
+  }
   render_text->SetText(text);
   return render_text;
 }

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

@@ -36,9 +36,11 @@ class OmniboxTextView : public views::View {
   int GetHeightForWidth(int width) const override;
   void OnPaint(gfx::Canvas* canvas) override;
 
-  // Dims the text (i.e. makes it gray). This is used for secondary text (so
-  // that the non-dimmed text stands out more).
-  void Dim();
+  // Applies given part's theme color to underlying render text. Using
+  // OmniboxPart::RESULTS_TEXT_DIMMED gives the gray used by Dim() in the past.
+  // This is called Apply* instead of Set* because the only state kept is in
+  // render_text, so call this after setting text with methods below.
+  void ApplyTextColor(OmniboxPart part);
 
   // Returns the render text, or an empty string if there is none.
   const base::string16& text() const;
@@ -46,10 +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.
-  void SetText(const base::string16& text);
+  // 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);
   void SetText(const base::string16& text,
-               const ACMatchClassifications& classifications);
-  void SetText(const SuggestionAnswer::ImageLine& line);
+               const ACMatchClassifications& classifications,
+               int font_size_delta = 0);
+  void SetText(const SuggestionAnswer::ImageLine& line, int size_delta = 0);
 
   // Adds the "additional" and "status" text from |line|, if any.
   void AppendExtraText(const SuggestionAnswer::ImageLine& line);
@@ -77,6 +82,9 @@ 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 wrap lines if the width is too narrow for the whole string.
   bool wrap_text_lines_;
 

components/omnibox/browser/autocomplete_match.cc

@@ -775,6 +775,10 @@ size_t AutocompleteMatch::EstimateMemoryUsage() const {
   return res;
 }
 
+bool AutocompleteMatch::IsExceptedFromLineReversal() const {
+  return !!answer && answer->type() == SuggestionAnswer::ANSWER_TYPE_DICTIONARY;
+}
+
 #if DCHECK_IS_ON()
 void AutocompleteMatch::Validate() const {
   ValidateClassifications(contents, contents_class);

components/omnibox/browser/autocomplete_match.h

@@ -350,6 +350,10 @@ struct AutocompleteMatch {
   // See base/trace_event/memory_usage_estimator.h for more info.
   size_t EstimateMemoryUsage() const;
 
+  // Some types of matches (answers for dictionary definitions, e.g.) do not
+  // follow the common rules for reversing lines.
+  bool IsExceptedFromLineReversal() const;
+
   // The provider of this match, used to remember which provider the user had
   // selected when the input changes. This may be NULL, in which case there is
   // no provider (or memory of the user's selection).