First stab at touch optimized omnibox auto-complete per sgabriel's mocks.

chrome/browser/ui/views/autocomplete/autocomplete_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 15 matching lines @@
 #include "ui/gfx/color_utils.h"
 #include "ui/gfx/native_theme.h"
 
 namespace {
 
 const char16 kEllipsis[] = { 0x2026, 0x0 };
 
 // The minimum distance between the top and bottom of the {icon|text} and the
 // top or bottom of the row.
 const int kMinimumIconVerticalPadding = 2;
-const int kMinimumTextVerticalPadding = 3;
+const int kMinimumTextVerticalPadding = 3;  // Default value, may be overridden.

[Peter Kasting, 2012/05/07 21:03:12]

Nit: Comment unnecessary given your comments on the members.

[Jói, 2012/05/10 18:33:07]

Done.

 
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
 // AutocompleteResultView, public:
 
 // Precalculated data used to draw the portion of a match classification that
 // fits entirely within one run.
 struct AutocompleteResultView::ClassificationData {
   string16 text;
@@ skipping 49 matching lines @@
   int right_;
 
   DISALLOW_COPY_AND_ASSIGN(MirroringContext);
 };
 
 AutocompleteResultView::AutocompleteResultView(
     AutocompleteResultViewModel* model,
     int model_index,
     const gfx::Font& font,
     const gfx::Font& bold_font)
-    : model_(model),
+    : edge_item_padding_(LocationBarView::kEdgeItemPadding),
+      item_padding_(LocationBarView::kItemPadding),
+      minimum_text_vertical_padding_(kMinimumTextVerticalPadding),
+      model_(model),
       model_index_(model_index),
       normal_font_(font),
       bold_font_(bold_font),
       ellipsis_width_(font.GetStringWidth(string16(kEllipsis))),
       mirroring_context_(new MirroringContext()),
       keyword_icon_(new views::ImageView()),
       ALLOW_THIS_IN_INITIALIZER_LIST(
           animation_(new ui::SlideAnimation(this))) {
   CHECK_GE(model_index, 0);
   if (default_icon_size_ == 0) {
@@ skipping 84 matching lines @@
 }
 
 void AutocompleteResultView::Invalidate() {
   keyword_icon_->SetImage(GetKeywordIcon());
   SchedulePaint();
 }
 
 gfx::Size AutocompleteResultView::GetPreferredSize() {
   return gfx::Size(0, std::max(
       default_icon_size_ + (kMinimumIconVerticalPadding * 2),
-      GetTextHeight() + (kMinimumTextVerticalPadding * 2)));
+      GetTextHeight() + (minimum_text_vertical_padding_ * 2)));
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // AutocompleteResultView, protected:
 
+AutocompleteResultView::ResultViewState
+    AutocompleteResultView::GetState() const {
+  if (model_->IsSelectedIndex(model_index_))
+    return SELECTED;
+  return model_->IsHoveredIndex(model_index_) ? HOVERED : NORMAL;
+}
+
 void AutocompleteResultView::PaintMatch(gfx::Canvas* canvas,
                                         const AutocompleteMatch& match,
                                         int x) {
   x = DrawString(canvas, match.contents, match.contents_class, false, x,
                  text_bounds_.y());
 
   // Paint the description.
   // TODO(pkasting): Because we paint in multiple separate pieces, we can wind
   // up with no space even for an ellipsis for one or both of these pieces.
   // Instead, we should paint the entire match as a single long string.  This
@@ skipping 10 matching lines @@
 
     DrawString(canvas, match.description, match.description_class, true, x,
                text_bounds_.y());
   }
 }
 
 int AutocompleteResultView::GetTextHeight() const {
   return std::max(normal_font_.GetHeight(), bold_font_.GetHeight());
 }
 
+void AutocompleteResultView::AdjustIconBounds(const AutocompleteMatch& match,
+                                              gfx::Rect* bounds) {
+}
+
 // static
 bool AutocompleteResultView::SortRunsLogically(const RunData& lhs,
                                                const RunData& rhs) {
   return lhs.run_start < rhs.run_start;
 }
 
 // static
 bool AutocompleteResultView::SortRunsVisually(const RunData& lhs,
                                               const RunData& rhs) {
   return lhs.visual_order < rhs.visual_order;
 }
 
 // static
 int AutocompleteResultView::default_icon_size_ = 0;
 
-AutocompleteResultView::ResultViewState
-    AutocompleteResultView::GetState() const {
-  if (model_->IsSelectedIndex(model_index_))
-    return SELECTED;
-  return model_->IsHoveredIndex(model_index_) ? HOVERED : NORMAL;
-}
-
 const SkBitmap* AutocompleteResultView::GetIcon() const {
   const SkBitmap* bitmap = model_->GetIconIfExtensionMatch(model_index_);
   if (bitmap)
     return bitmap;
 
   int icon = match_.starred ?
       IDR_OMNIBOX_STAR : AutocompleteMatch::TypeToIcon(match_.type);
   if (GetState() == SELECTED) {
     switch (icon) {
       case IDR_OMNIBOX_EXTENSION_APP:
@@ skipping 252 matching lines @@
     }
   }
 
   // We couldn't draw anything.
   runs->clear();
 }
 
 void AutocompleteResultView::Layout() {
   const SkBitmap* icon = GetIcon();
 
-  icon_bounds_.SetRect(LocationBarView::kEdgeItemPadding +
-      ((icon->width() == default_icon_size_) ?
-          0 : LocationBarView::kIconInternalPadding),
+  icon_bounds_.SetRect(
+      edge_item_padding_ + ((icon->width() == default_icon_size_) ? 0 :

[Peter Kasting, 2012/05/07 21:03:12]

Nit: Wrap these lines as they were before (maintains linebreak after ? instead
of :, as well as more clarity that the "edge_item_padding_ +" portion is not
included in the ?: statement)

[Jói, 2012/05/10 18:33:07]

Went one step further and separated it out as a temporary, it was very hard to
read regardless of indentation:


  int icon_padding = (icon->width() == default_icon_size_) ?
                      0 : LocationBarView::kIconInternalPadding;
  icon_bounds_.SetRect(edge_item_padding_ + icon_padding,
                       (height() - icon->height()) / 2,
                       icon->width(),
                       icon->height());

+                            LocationBarView::kIconInternalPadding),
       (height() - icon->height()) / 2, icon->width(), icon->height());
 
-  int text_x = LocationBarView::kEdgeItemPadding + default_icon_size_ +
-      LocationBarView::kItemPadding;
+  int text_x = edge_item_padding_ + default_icon_size_ + item_padding_;
   int text_height = GetTextHeight();
   int text_width;
 
   if (match_.associated_keyword.get()) {
     const int kw_collapsed_size = keyword_icon_->width() +
-        LocationBarView::kEdgeItemPadding;
+        edge_item_padding_;
     const int max_kw_x = width() - kw_collapsed_size;
     const int kw_x = animation_->CurrentValueBetween(max_kw_x,
-        LocationBarView::kEdgeItemPadding);
-    const int kw_text_x = kw_x + keyword_icon_->width() +
-        LocationBarView::kItemPadding;
+        edge_item_padding_);
+    const int kw_text_x = kw_x + keyword_icon_->width() + item_padding_;
 
-    text_width = kw_x - text_x - LocationBarView::kItemPadding;
+    text_width = kw_x - text_x - item_padding_;
     keyword_text_bounds_.SetRect(kw_text_x, 0, std::max(
-        width() - kw_text_x - LocationBarView::kEdgeItemPadding, 0),
+        width() - kw_text_x - edge_item_padding_, 0),
         text_height);
     keyword_icon_->SetPosition(gfx::Point(kw_x,
         (height() - keyword_icon_->height()) / 2));
   } else {
-    text_width = width() - text_x - LocationBarView::kEdgeItemPadding;
+    text_width = width() - text_x - edge_item_padding_;
   }
 
   text_bounds_.SetRect(text_x, std::max(0, (height() - text_height) / 2),
       std::max(text_width, 0), text_height);
 }
 
 void AutocompleteResultView::OnBoundsChanged(
     const gfx::Rect& previous_bounds) {
   animation_->SetSlideDuration(width() / 4);
 }
 
 void AutocompleteResultView::OnPaint(gfx::Canvas* canvas) {
   const ResultViewState state = GetState();
   if (state != NORMAL)
     canvas->DrawColor(GetColor(state, BACKGROUND));
 
   if (!match_.associated_keyword.get() ||
       keyword_icon_->x() > icon_bounds_.right()) {
     // Paint the icon.
-    canvas->DrawBitmapInt(*GetIcon(), GetMirroredXForRect(icon_bounds_),
-        icon_bounds_.y());
+    AdjustIconBounds(match_, &icon_bounds_);
+    canvas->DrawBitmapInt(

[Peter Kasting, 2012/05/07 21:03:12]

Nit: Better than either old or new wrapping would be:

    canvas->DrawBitmapInt(*GetIcon(), GetMirroredXForRect(icon_bounds_),
                          icon_bounds_.y());

[Jói, 2012/05/10 18:33:07]

Done.

+        *GetIcon(), GetMirroredXForRect(icon_bounds_), icon_bounds_.y());
 
     // Paint the text.
     int x = GetMirroredXForRect(text_bounds_);
     mirroring_context_->Initialize(x, text_bounds_.width());
     PaintMatch(canvas, match_, x);
   }
 
   if (match_.associated_keyword.get()) {
     // Paint the keyword text.
     int x = GetMirroredXForRect(keyword_text_bounds_);
     mirroring_context_->Initialize(x, keyword_text_bounds_.width());
     PaintMatch(canvas, *match_.associated_keyword.get(), x);
   }
 }
 
 void AutocompleteResultView::AnimationProgressed(
     const ui::Animation* animation) {
   Layout();
   SchedulePaint();
 }