Change width of rows in material design omnibox dropdown

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 313 matching lines @@
         l10n_util::GetStringUTF16(IDS_AUTOCOMPLETE_MATCH_DESCRIPTION_SEPARATOR);
     separator_rendertext_.reset(CreateRenderText(separator).release());
     separator_rendertext_->SetColor(GetColor(GetState(), DIMMED_TEXT));
     separator_width_ = separator_rendertext_->GetContentWidth();
   }
 
   contents->SetDisplayRect(gfx::Rect(gfx::Size(INT_MAX, 0)));
   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);
 
   int after_contents_x =
       DrawRenderText(match, contents, true, canvas, x, y, contents_max_width);
 
   if (description_max_width != 0) {
     if (match.answer) {
       y += GetContentLineHeight();
@@ skipping 269 matching lines @@
   ui::ThemeProvider* theme_provider = location_bar_view_->GetThemeProvider();
   // |theme_provider| can be null when animations are running during shutdown,
   // after OmniboxResultView has been removed from the tree of Views.
   if (!theme_provider)
     return;
   const int horizontal_padding = theme_provider->GetDisplayProperty(
       ThemeProperties::PROPERTY_LOCATION_BAR_HORIZONTAL_PADDING);
   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;
     keyword_text_bounds_.SetRect(
         kw_text_x, 0, std::max(width() - kw_text_x - horizontal_padding, 0),
         height());
     keyword_icon_->SetPosition(
         gfx::Point(kw_x, (height() - keyword_icon_->height()) / 2));
   }
 
   text_bounds_.SetRect(text_x, 0, std::max(text_width, 0), height());
 }
 
 void OmniboxResultView::OnBoundsChanged(const gfx::Rect& previous_bounds) {
-  animation_->SetSlideDuration(width() / 4);
+  animation_->SetSlideDuration((width() - LeftMargin() - RightMargin()) / 4);
 }
 
 void OmniboxResultView::OnPaint(gfx::Canvas* canvas) {
   const ResultViewState state = GetState();
   if (state != NORMAL)
     canvas->DrawColor(GetColor(state, BACKGROUND));
 
   // NOTE: While animating the keyword match, both matches may be visible.
 
   if (!ShowOnlyKeywordMatch()) {
@@ skipping 126 matching lines @@
   gfx::Range range(offset, offset + text.length());
   destination->AppendText(text);
   const TextStyle& text_style = GetTextStyle(text_type);
   // TODO(dschuyler): follow up on the problem of different font sizes within
   // one RenderText.  Maybe with destination->SetFontList(...).
   destination->ApplyStyle(gfx::BOLD, is_bold, range);
   destination->ApplyColor(
       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;
+}