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..06ad8b50eeeeec9b70f792f60f74965c61096367 100644
--- a/chrome/browser/ui/views/omnibox/omnibox_result_view.cc
+++ b/chrome/browser/ui/views/omnibox/omnibox_result_view.cc
@@ -331,11 +331,18 @@ void OmniboxResultView::PaintMatch(const AutocompleteMatch& match,
   if (description)
     description->SetDisplayRect(gfx::Rect(gfx::Size(INT_MAX, 0)));
   int contents_max_width, description_max_width;
+
+  int remaining_width = mirroring_context_->remaining_width(x);
+  if (ui::MaterialDesignController::IsModeMaterial()) {

[Peter Kasting, 2015/08/19 20:37:34]

On more reflection: why is this block needed at all?

The mirroring context tells you how much space you have based on the values
given to it in an Initialize() call.  That call happens in OnPaint() and will
use the [keyword_]text_bounds_ calculated in Layout().  Since you've changed the
code there to reflect the margins you want, the mirroring context should be
correctly clamping to the space you want.  I think you ought to be able to
entirely revert the changes to PaintMatch(), and still get the behavior you
want.

If not, then I'll need to have a better understanding of what's going wrong to
know how to fix.

[tdanderson, 2015/08/19 22:32:57]

Without this block, the "Search <site> for <query>" text shows up in the right
margin when normally it would be hidden until the user actually presses tab.
I'll attach a screenshot to crbug.com/519514.

[Peter Kasting, 2015/08/21 22:13:06]

This is because the change to Layout() is incomplete.

You need to change line 652 to:

    keyword_text_bounds_.SetRect(kw_text_x, 0, std::max(end_x - kw_text_x, 0),
                                 height());

+    const int max_x = width() - RightMargin();
+    remaining_width = std::min(remaining_width, std::max(0, max_x - x));
+  }
+
   OmniboxPopupModel::ComputeMatchMaxWidths(
       contents->GetContentWidth(),
       separator_width_,
       description ? description->GetContentWidth() : 0,
-      mirroring_context_->remaining_width(x),
+      remaining_width,
       !AutocompleteMatch::IsSearchType(match.type),
       &contents_max_width,
       &description_max_width);
@@ -625,20 +632,20 @@ void OmniboxResultView::Layout() {
   const int trailing_padding = theme_provider->GetDisplayProperty(
       ThemeProperties::PROPERTY_ICON_LABEL_VIEW_TRAILING_PADDING);
 
+  const int start_x = LeftMargin() + horizontal_padding;
+  const int end_x = width() - RightMargin() - horizontal_padding;
+
   icon_bounds_.SetRect(
-      horizontal_padding +
-          ((icon.width() == default_icon_size_) ? 0 : trailing_padding),
-      (GetContentLineHeight() - icon.height()) / 2, icon.width(),
-      icon.height());
+      start_x + ((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 = start_x + horizontal_padding + default_icon_size_;

[Peter Kasting, 2015/08/19 20:37:34]

Nit: Put |default_icon_size_| before |horizontal_padding| so it's clear that's
the padding after the icon, not an erroneous duplication of the padding before
it.

[tdanderson, 2015/08/19 22:32:57]

Done.

+  int text_width = end_x - text_x;
 
   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 max_kw_x = end_x - keyword_icon_->width();
+    const int kw_x = animation_->CurrentValueBetween(max_kw_x, start_x);
     const int kw_text_x = kw_x + keyword_icon_->width() + horizontal_padding;
 
     text_width = kw_x - text_x - horizontal_padding;
@@ -653,7 +660,7 @@ void OmniboxResultView::Layout() {
 }
 
 void OmniboxResultView::OnBoundsChanged(const gfx::Rect& previous_bounds) {
-  animation_->SetSlideDuration(width() / 4);
+  animation_->SetSlideDuration((width() - LeftMargin() - RightMargin()) / 4);
 }
 
 void OmniboxResultView::OnPaint(gfx::Canvas* canvas) {
@@ -800,3 +807,13 @@ void OmniboxResultView::AppendAnswerTextHelper(gfx::RenderText* destination,
       GetNativeTheme()->GetSystemColor(text_style.colors[GetState()]), range);
   destination->ApplyBaselineStyle(text_style.baseline, range);
 }
+
+int OmniboxResultView::LeftMargin() const {
+  return ui::MaterialDesignController::IsModeMaterial() ?
+      model_->left_margin() : 0;
+}
+
+int OmniboxResultView::RightMargin() const {
+  return ui::MaterialDesignController::IsModeMaterial() ?
+      model_->right_margin() : 0;
+}