Fix layout of omnibox rows for material design

chrome/browser/ui/views/omnibox/omnibox_result_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.
 
 // For WinDDK ATL compatibility, these ATL headers must come first.
 #include "build/build_config.h"
 #if defined(OS_WIN)
 #include <atlbase.h>  // NOLINT
 #include <atlwin.h>  // NOLINT
 #endif
@@ skipping 341 matching lines @@
   int contents_max_width, description_max_width;
   OmniboxPopupModel::ComputeMatchMaxWidths(
       contents->GetContentWidth(),
       separator_width_,
       description ? description->GetContentWidth() : 0,
       mirroring_context_->remaining_width(x),
       !AutocompleteMatch::IsSearchType(match.type),
       &contents_max_width,
       &description_max_width);
 
-  int after_contents_x =
-      DrawRenderText(match, contents, true, canvas, x, y, contents_max_width);
+  int after_contents_x = DrawRenderText(match, contents, CONTENTS, canvas,
+                                        x, y, contents_max_width);
 
   if (description_max_width != 0) {
     if (match.answer) {
       y += GetContentLineHeight();
       if (!answer_image_.isNull()) {
         int answer_icon_size = GetAnswerLineHeight();
         canvas->DrawImageInt(
             answer_image_,
             0, 0, answer_image_.width(), answer_image_.height(),
             GetMirroredXInView(x), y, answer_icon_size, answer_icon_size, true);
         // See TODO in Layout().
         x += answer_icon_size +
              GetLayoutConstant(ICON_LABEL_VIEW_TRAILING_PADDING);
       }
     } else {
-      x = DrawRenderText(match, separator_rendertext_.get(), false, canvas,
+      x = DrawRenderText(match, separator_rendertext_.get(), SEPARATOR, canvas,
                          after_contents_x, y, separator_width_);
     }
 
-    DrawRenderText(match, description, false, canvas, x, y,
+    DrawRenderText(match, description, DESCRIPTION, canvas, x, y,
                    description_max_width);
   }
 }
 
 int OmniboxResultView::DrawRenderText(
     const AutocompleteMatch& match,
     gfx::RenderText* render_text,
-    bool contents,
+    RenderTextType render_text_type,
     gfx::Canvas* canvas,
     int x,
     int y,
     int max_width) const {
   DCHECK(!render_text->text().empty());
 
   const int remaining_width = mirroring_context_->remaining_width(x);
   int right_x = x + max_width;
 
   // Infinite suggestions should appear with the leading ellipses vertically
   // stacked.
-  if (contents &&
-      (match.type == AutocompleteMatchType::SEARCH_SUGGEST_TAIL)) {
+  if (render_text_type == CONTENTS &&
+      match.type == AutocompleteMatchType::SEARCH_SUGGEST_TAIL) {
     // When the directionality of suggestion doesn't match the UI, we try to
     // vertically stack the ellipsis by restricting the end edge (right_x).
     const bool is_ui_rtl = base::i18n::IsRTL();
     const bool is_match_contents_rtl =
         (render_text->GetDisplayTextDirection() == base::i18n::RIGHT_TO_LEFT);
     const int offset =
         GetDisplayOffset(match, is_ui_rtl, is_match_contents_rtl);
 
     scoped_ptr<gfx::RenderText> prefix_render_text(
         CreateRenderText(base::UTF8ToUTF16(
@@ skipping 33 matching lines @@
         gfx::DIRECTIONALITY_FORCE_RTL : gfx::DIRECTIONALITY_FORCE_LTR);
     prefix_render_text->SetHorizontalAlignment(
           is_match_contents_rtl ? gfx::ALIGN_RIGHT : gfx::ALIGN_LEFT);
     prefix_render_text->SetDisplayRect(
         gfx::Rect(mirroring_context_->mirrored_left_coord(
                       prefix_x, prefix_x + prefix_width),
                   y, prefix_width, GetContentLineHeight()));
     prefix_render_text->Draw(canvas);
   }
 
+  int height = (render_text_type == DESCRIPTION && match.answer) ?

[Peter Kasting, 2015/10/19 20:47:11]

Nit: const

I'd probably also place this line just below the comment below since it's really
"part of" the SetDisplayRect() call.

[tdanderson, 2015/10/20 14:20:15]

Done.

+      GetAnswerLineHeight() : GetContentLineHeight();
+
   // Set the display rect to trigger eliding.
   render_text->SetDisplayRect(
       gfx::Rect(mirroring_context_->mirrored_left_coord(x, right_x), y,
-                right_x - x, GetContentLineHeight()));
+                right_x - x, height));

[Peter Kasting, 2015/10/19 20:47:11]

Wow, wouldn't this have made the non-MD answer content mispositioned before? 
Unless GetContentsLineHeight() and GetAnswerLineHeight() had been returning the
same value...

[tdanderson, 2015/10/20 14:20:15]

Yes, without MD GetContentsLineHeight() was returning 25
and GetAnswerLineHeight() was returning 24, making this
go unnoticed. It showed up when I switched to hybrid where
the contents line height is 32.

   render_text->Draw(canvas);
   return right_x;
 }
 
 scoped_ptr<gfx::RenderText> OmniboxResultView::CreateRenderText(
     const base::string16& text) const {
   scoped_ptr<gfx::RenderText> render_text(gfx::RenderText::CreateInstance());
   render_text->SetDisplayRect(gfx::Rect(gfx::Size(INT_MAX, 0)));
   render_text->SetCursorEnabled(false);
   render_text->SetElideBehavior(gfx::ELIDE_TAIL);
@@ skipping 331 matching lines @@
 
 int OmniboxResultView::StartMargin() const {
   return ui::MaterialDesignController::IsModeMaterial() ?
       model_->start_margin() : 0;
 }
 
 int OmniboxResultView::EndMargin() const {
   return ui::MaterialDesignController::IsModeMaterial() ?
       model_->end_margin() : 0;
 }