Change width of rows in material design omnibox dropdown

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

Index: chrome/browser/ui/views/omnibox/omnibox_result_view.cc
diff --git a/chrome/browser/ui/views/omnibox/omnibox_result_view.cc b/chrome/browser/ui/views/omnibox/omnibox_result_view.cc
index 7e6815f9c76e949838f94804fb6dc0f2817af578..81eb6747ac5e6e4b5491987eafd32d3082628db3 100644
--- a/chrome/browser/ui/views/omnibox/omnibox_result_view.cc
+++ b/chrome/browser/ui/views/omnibox/omnibox_result_view.cc
@@ -437,6 +437,10 @@ int OmniboxResultView::DrawRenderText(
     prefix_render_text->Draw(canvas);
   }
 
+  // No text should be drawn within the right margin.

[Peter Kasting, 2015/08/18 19:55:37]

This isn't the right way to do this.  You need to change the code in
PaintMatch() so that it computes the contents and description widths based on
the correct available width, as we're making eliding and layout decisions that
rely on knowing precisely how much space is available.  What this code does
instead is lay out as if we have the full width, but then truncate the last
portion.

[tdanderson, 2015/08/19 17:22:35]

OK, please take a look at what I've done in Patch Set 2, which looks fine with
and without the MD flag on a local build. Though I'm a bit suspicious of the
fact that I am fiddling with the call to mirroring_context_->remaining_width(x)
in PaintMatch() but not the one in DrawRenderText().

+  if (ui::MaterialDesignController::IsModeMaterial())
+    right_x = std::min(right_x, width() - RightOffset());
+
   // Set the display rect to trigger eliding.
   render_text->SetDisplayRect(
       gfx::Rect(mirroring_context_->mirrored_left_coord(x, right_x), y,
@@ -626,21 +630,22 @@ void OmniboxResultView::Layout() {
       ThemeProperties::PROPERTY_ICON_LABEL_VIEW_TRAILING_PADDING);
 
   icon_bounds_.SetRect(
-      horizontal_padding +
+      horizontal_padding + LeftOffset() +

[Peter Kasting, 2015/08/18 19:55:37]

Nit: If we create two more temps:

  const int start_x = LeftOffset() + horizontal_padding;
  const int end_x = width() - RightOffset() - horizontal_padding;

Then we can simplify the code here some:

  icon_bounds_.SetRect(
      start_x + ((icon.width() == default_icon_size_) ? 0 : trailing_padding),
      (GetContentLineHeight() - icon.height()) / 2,
      icon.width(), icon.height());

  const int text_x = start_x + default_icon_size_ + horizontal_padding_;
  int text_width = end_x - text_x;

  if (match_.associated_keyword) {
    const int max_kw_x = end_x - keyword_icon_->width();
    const int kw_x = animation_->CurrentValueBetween(max_kw_x, start_x);
    ...

[tdanderson, 2015/08/19 17:22:35]

Thanks, should've caught that. Done.

           ((icon.width() == default_icon_size_) ? 0 : trailing_padding),
       (GetContentLineHeight() - icon.height()) / 2, icon.width(),
       icon.height());
 
-  int text_x = (2 * horizontal_padding) + default_icon_size_;
-  int text_width = width() - text_x - horizontal_padding;
+  const int text_x =
+      LeftOffset() + (2 * horizontal_padding) + default_icon_size_;
+  int text_width = width() - text_x - horizontal_padding - RightOffset();
 
   if (match_.associated_keyword.get()) {
     const int kw_collapsed_size = keyword_icon_->width() + horizontal_padding;
-    const int max_kw_x = width() - kw_collapsed_size;
-    const int kw_x =
-        animation_->CurrentValueBetween(max_kw_x, horizontal_padding);
-    const int kw_text_x = kw_x + keyword_icon_->width() + horizontal_padding;
+    const int max_kw_x = width() - kw_collapsed_size - RightOffset();
+    const int kw_x = animation_->CurrentValueBetween(
+        max_kw_x, LeftOffset() + horizontal_padding);
 
+    const int kw_text_x = kw_x + keyword_icon_->width() + horizontal_padding;
     text_width = kw_x - text_x - horizontal_padding;
     keyword_text_bounds_.SetRect(
         kw_text_x, 0, std::max(width() - kw_text_x - horizontal_padding, 0),
@@ -653,7 +658,7 @@ void OmniboxResultView::Layout() {
 }
 
 void OmniboxResultView::OnBoundsChanged(const gfx::Rect& previous_bounds) {
-  animation_->SetSlideDuration(width() / 4);
+  animation_->SetSlideDuration((width() - LeftOffset() - RightOffset()) / 4);
 }
 
 void OmniboxResultView::OnPaint(gfx::Canvas* canvas) {
@@ -800,3 +805,15 @@ void OmniboxResultView::AppendAnswerTextHelper(gfx::RenderText* destination,
       GetNativeTheme()->GetSystemColor(text_style.colors[GetState()]), range);
   destination->ApplyBaselineStyle(text_style.baseline, range);
 }
+
+int OmniboxResultView::LeftOffset() const {
+  if (ui::MaterialDesignController::IsModeMaterial())

[Peter Kasting, 2015/08/18 19:55:37]

Nit: Shorter:

  return ui::MaterialDesignController::IsModeMaterial() ?
      model_->left_margin() : 0;

(2 places)

[tdanderson, 2015/08/19 17:22:35]

Done.

+    return model_->left_margin();
+  return 0;
+}
+
+int OmniboxResultView::RightOffset() const {
+  if (ui::MaterialDesignController::IsModeMaterial())
+    return model_->right_margin();
+  return 0;
+}