[Omnibox] Allow rich suggestion descriptions to wrap lines

Most answers and entities have single line descriptions, but dictionary
definitions are allowed to wrap. This CL allows for that wrapping.

Also, fixes bug where the cached font_height_ wasn't updated properly.
(Which would occasionally cause a line to be too tall).

Bug: 838733
Change-Id: Ic1478acd6c62420f3bfd426b206973590b625729
Reviewed-on: https://chromium-review.googlesource.com/1048865Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Dave Schuyler <dschuyler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557264}

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

@@ -4,6 +4,8 @@
 
 #include "chrome/browser/ui/views/omnibox/omnibox_match_cell_view.h"
 
+#include <algorithm>
+
 #include "base/macros.h"
 #include "base/metrics/field_trial_params.h"
 #include "chrome/browser/ui/layout_constants.h"
@@ -150,7 +152,6 @@ class OmniboxImageView : public views::ImageView {
 // OmniboxMatchCellView:
 
 OmniboxMatchCellView::OmniboxMatchCellView(OmniboxResultView* result_view,
-                                           const gfx::FontList& font_list,
                                            int text_height)
     : is_old_style_answer_(false),
       is_rich_suggestion_(false),
@@ -158,9 +159,9 @@ OmniboxMatchCellView::OmniboxMatchCellView(OmniboxResultView* result_view,
       text_height_(text_height) {
   AddChildView(icon_view_ = new OmniboxImageView());
   AddChildView(image_view_ = new OmniboxImageView());
-  AddChildView(content_view_ = new OmniboxTextView(result_view, font_list));
-  AddChildView(description_view_ = new OmniboxTextView(result_view, font_list));
-  AddChildView(separator_view_ = new OmniboxTextView(result_view, font_list));
+  AddChildView(content_view_ = new OmniboxTextView(result_view));
+  AddChildView(description_view_ = new OmniboxTextView(result_view));
+  AddChildView(separator_view_ = new OmniboxTextView(result_view));
 
   const base::string16& separator =
       l10n_util::GetStringUTF16(IDS_AUTOCOMPLETE_MATCH_DESCRIPTION_SEPARATOR);
@@ -172,10 +173,8 @@ OmniboxMatchCellView::~OmniboxMatchCellView() = default;
 gfx::Size OmniboxMatchCellView::CalculatePreferredSize() const {
   int height = text_height_ +
                GetVerticalInsets(text_height_, is_old_style_answer_).height();
-  if (is_rich_suggestion_) {
-    height += text_height_;
-  } else if (is_old_style_answer_) {
-    height += GetOldStyleAnswerHeight();
+  if (is_rich_suggestion_ || is_old_style_answer_) {
+    height += GetDescriptionHeight();
   }
   return gfx::Size(0, height);
 }
@@ -184,7 +183,7 @@ bool OmniboxMatchCellView::CanProcessEventsWithinSubtree() const {
   return false;
 }
 
-int OmniboxMatchCellView::GetOldStyleAnswerHeight() const {
+int OmniboxMatchCellView::GetDescriptionHeight() const {
   int icon_width = icon_view_->width();
   int answer_icon_size = image_view_->visible()
                              ? image_view_->height() + kAnswerIconToTextPadding
@@ -276,7 +275,11 @@ void OmniboxMatchCellView::LayoutRichSuggestion() {
   x += kRichImageSize + HorizontalPadding();
   content_view_->SetBounds(x, y, width() - x, text_height_);
   y += text_height_;
-  description_view_->SetBounds(x, y, width() - x, text_height_);
+  int description_width = width() - x;
+  description_view_->SetBounds(
+      x, y, description_width,
+      description_view_->GetHeightForWidth(description_width) +
+          kVerticalPadding);
 }
 
 void OmniboxMatchCellView::LayoutSplit() {

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

@@ -7,10 +7,6 @@
 
 #include "ui/views/view.h"
 
-namespace gfx {
-class FontList;
-}
-
 namespace views {
 class ImageView;
 }
@@ -22,7 +18,6 @@ class OmniboxTextView;
 class OmniboxMatchCellView : public views::View {
  public:
   explicit OmniboxMatchCellView(OmniboxResultView* result_view,
-                                const gfx::FontList& font_list,
                                 int text_height);
   ~OmniboxMatchCellView() override;
 
@@ -45,7 +40,7 @@ class OmniboxMatchCellView : public views::View {
   const char* GetClassName() const override;
 
   // Returns the height of the the description section of answer suggestions.
-  int GetOldStyleAnswerHeight() const;
+  int GetDescriptionHeight() const;
 
   void LayoutOldStyleAnswer();
   void LayoutRichSuggestion();

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

@@ -97,9 +97,8 @@ OmniboxResultView::OmniboxResultView(OmniboxPopupContentsView* model,
   CHECK_GE(model_index, 0);
 
   AddChildView(suggestion_view_ =
-                   new OmniboxMatchCellView(this, font_list, GetTextHeight()));
-  AddChildView(keyword_view_ =
-                   new OmniboxMatchCellView(this, font_list, GetTextHeight()));
+                   new OmniboxMatchCellView(this, GetTextHeight()));
+  AddChildView(keyword_view_ = new OmniboxMatchCellView(this, GetTextHeight()));
 
   keyword_view_->icon()->EnableCanvasFlippingForRTLUI(true);
   keyword_view_->icon()->SetImage(gfx::CreateVectorIcon(

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

@@ -126,13 +126,8 @@ const gfx::FontList& GetFontForType(int text_type) {
 
 }  // namespace
 
-OmniboxTextView::OmniboxTextView(OmniboxResultView* result_view,
-                                 const gfx::FontList& font_list)
-    : result_view_(result_view),
-      font_height_(std::max(
-          font_list.GetHeight(),
-          font_list.DeriveWithWeight(gfx::Font::Weight::BOLD).GetHeight())),
-      wrap_text_lines_(false) {}
+OmniboxTextView::OmniboxTextView(OmniboxResultView* result_view)
+    : result_view_(result_view), font_height_(0), wrap_text_lines_(false) {}
 
 OmniboxTextView::~OmniboxTextView() {}
 
@@ -155,7 +150,7 @@ int OmniboxTextView::GetHeightForWidth(int width) const {
     return 0;
   // If text wrapping is not called for we can simply return the font height.
   if (!wrap_text_lines_) {
-    return font_height_;
+    return GetLineHeight();
   }
   render_text_->SetDisplayRect(gfx::Rect(width, 0));
   gfx::Size string_size = render_text_->GetStringSize();
@@ -227,11 +222,13 @@ void OmniboxTextView::SetText(const base::string16& text,
                               const ACMatchClassifications& classifications) {
   render_text_.reset();
   render_text_ = CreateClassifiedRenderText(text, classifications);
+  UpdateLineHeight();
 }
 
 void OmniboxTextView::SetText(const base::string16& text) {
   render_text_.reset();
   render_text_ = CreateRenderText(text);
+  UpdateLineHeight();
 }
 
 void OmniboxTextView::SetText(const SuggestionAnswer::ImageLine& line) {
@@ -242,7 +239,7 @@ void OmniboxTextView::SetText(const SuggestionAnswer::ImageLine& line) {
   // sizes or use multiple RenderTexts.
   render_text_.reset();
   render_text_ = CreateText(line, GetFontForType(line.text_fields()[0].type()));
-  font_height_ = render_text_->font_list().GetHeight();
+  UpdateLineHeight();
 }
 
 std::unique_ptr<gfx::RenderText> OmniboxTextView::CreateText(
@@ -334,3 +331,10 @@ void OmniboxTextView::AppendTextHelper(gfx::RenderText* destination,
 int OmniboxTextView::GetLineHeight() const {
   return font_height_;
 }
+
+void OmniboxTextView::UpdateLineHeight() {
+  font_height_ = std::max(render_text_->font_list().GetHeight(),
+                          render_text_->font_list()
+                              .DeriveWithWeight(gfx::Font::Weight::BOLD)
+                              .GetHeight());
+}

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

@@ -26,8 +26,7 @@ class RenderText;
 // as selection) and more specific features (such as suggestion answer styling).
 class OmniboxTextView : public views::View {
  public:
-  explicit OmniboxTextView(OmniboxResultView* result_view,
-                           const gfx::FontList& font_list);
+  explicit OmniboxTextView(OmniboxResultView* result_view);
   ~OmniboxTextView() override;
 
   // views::View.
@@ -82,6 +81,8 @@ class OmniboxTextView : public views::View {
                         int text_type,
                         bool is_bold) const;
 
+  void UpdateLineHeight();
+
   // To get color values.
   OmniboxResultView* result_view_;