Enabled pressing TAB to cycle through the Omnibox results.

chrome/browser/ui/views/autocomplete/autocomplete_result_view.cc

Index: chrome/browser/ui/views/autocomplete/autocomplete_result_view.cc
===================================================================
--- chrome/browser/ui/views/autocomplete/autocomplete_result_view.cc	(revision 81359)
+++ chrome/browser/ui/views/autocomplete/autocomplete_result_view.cc	(working copy)
@@ -14,6 +14,9 @@
 #include <algorithm>  // NOLINT
 
 #include "base/i18n/bidi_line_iterator.h"
+#include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/search_engines/template_url.h"
+#include "chrome/browser/search_engines/template_url_model.h"
 #include "chrome/browser/ui/views/autocomplete/autocomplete_result_view_model.h"
 #include "chrome/browser/ui/views/location_bar/location_bar_view.h"
 #include "grit/generated_resources.h"
@@ -23,6 +26,8 @@
 #include "ui/base/text/text_elider.h"
 #include "ui/gfx/canvas_skia.h"
 #include "ui/gfx/color_utils.h"
+#include "views/controls/image_view.h"
+#include "views/controls/label.h"
 
 #if defined(OS_LINUX)
 #include "chrome/browser/ui/gtk/gtk_util.h"
@@ -106,6 +111,101 @@
   DISALLOW_COPY_AND_ASSIGN(MirroringContext);
 };
 
+class AutocompleteResultView::IconLabelView : public views::View {
+ public:
+  explicit IconLabelView(gfx::Font font);
+
+  virtual void Layout();
+  virtual gfx::Size GetPreferredSize();
+  virtual void OnPaint(gfx::Canvas* canvas);
+
+  void SetLabelColor(SkColor color);
+  void SetLabelText(const string16& text);
+  void SetSelected(bool flag);

[Peter Kasting, 2011/04/13 21:06:59]

Nit: Name the arg |selected|.

+
+ private:
+  views::ImageView* image_;
+  views::Label* label_;
+  bool selected_;
+
+  DISALLOW_COPY_AND_ASSIGN(IconLabelView);
+};
+
+AutocompleteResultView::IconLabelView::IconLabelView(gfx::Font font)
+    : selected_(false) {
+  image_ = new views::ImageView();
+  AddChildView(image_);

[Peter Kasting, 2011/04/13 21:06:59]

Nit: I think our typical practice is to AddChildView() after the view's other
attributes have been set rather than before, in case updating one would
otherwise trigger any kind of re-layout/paint.

+  image_->SetImage(
+      ResourceBundle::GetSharedInstance().GetBitmapNamed(IDR_OMNIBOX_SEARCH));
+  label_ = new views::Label();
+  AddChildView(label_);
+  label_->SetFont(font);
+  label_->SetHorizontalAlignment(views::Label::ALIGN_LEFT);
+}
+
+void AutocompleteResultView::IconLabelView::Layout() {
+  const int icon_width = image_->GetPreferredSize().width();
+  image_->SetBounds(LocationBarView::kItemPadding * 2,

[Peter Kasting, 2011/04/13 21:06:59]

The use of padding constants in these next two functions seems strange.  I don't
know why the IconLabelView is given the padding on both sides of the divider
instead of just on its side, I don't understand why you're using
kExtensionItemPadding below, and I don't see any use of kEdgeItemPadding which
I'd expect to see for the padding on either side of the divider and after the
end of the text.

+      (height() - icon_width) / 2, icon_width, icon_width);
+  const int label_padding = icon_width + LocationBarView::kItemPadding * 3;
+  const int label_height = label_->GetPreferredSize().height();
+  label_->SetBounds(label_padding, (height() - label_height) / 2, width() -
+      label_padding, label_height);
+}
+
+gfx::Size AutocompleteResultView::IconLabelView::GetPreferredSize() {
+  gfx::Size size(image_->GetPreferredSize());
+  size.Enlarge(label_->GetPreferredSize().width() +
+      LocationBarView::kExtensionItemPadding +
+      LocationBarView::kItemPadding * 3, 0);
+  return size;
+}
+
+void AutocompleteResultView::IconLabelView::OnPaint(gfx::Canvas* canvas) {
+  if (selected_) {
+    canvas->Save();
+    canvas->ClipRectInt(LocationBarView::kItemPadding, 0, width() -
+        LocationBarView::kItemPadding, height());
+    canvas->AsCanvasSkia()->drawColor(GetColor(SELECTED, BACKGROUND));
+    canvas->Restore();
+  } else {
+    canvas->AsCanvasSkia()->drawColor(GetColor(NORMAL, BACKGROUND));
+  }
+
+  SkPaint paint;
+  paint.setColor(GetColor(NORMAL, DIMMED_TEXT));
+  paint.setStrokeWidth(SkIntToScalar(1));
+  paint.setAntiAlias(true);
+
+  canvas->AsCanvasSkia()->drawLine(
+      SkIntToScalar(LocationBarView::kVerticalEdgeThickness),
+      SkIntToScalar(LocationBarView::kNormalHorizontalEdgeThickness),
+      SkIntToScalar(LocationBarView::kVerticalEdgeThickness),
+      SkIntToScalar(height() -
+          LocationBarView::kNormalHorizontalEdgeThickness * 2), paint);
+}
+
+void AutocompleteResultView::IconLabelView::SetLabelColor(SkColor color) {
+  label_->SetColor(color);
+}
+
+void AutocompleteResultView::IconLabelView::SetLabelText(const string16& text) {
+  label_->SetText(UTF16ToWide(text));
+}
+
+void AutocompleteResultView::IconLabelView::SetSelected(bool flag) {
+  selected_ = flag;
+
+  if (selected_)

[Peter Kasting, 2011/04/13 21:06:59]

Nit: Collapse this to one call using ?:

+    image_->SetImage(ResourceBundle::GetSharedInstance().
+        GetBitmapNamed(IDR_OMNIBOX_SEARCH_SELECTED));
+  else
+    image_->SetImage(ResourceBundle::GetSharedInstance().
+        GetBitmapNamed(IDR_OMNIBOX_SEARCH));
+
+  SchedulePaint();
+}
+
 AutocompleteResultView::AutocompleteResultView(
     AutocompleteResultViewModel* model,
     int model_index,
@@ -119,13 +219,19 @@
       bold_font_(bold_font),
       ellipsis_width_(font.GetStringWidth(string16(kEllipsis))),
       mirroring_context_(new MirroringContext()),
-      match_(NULL, 0, false, AutocompleteMatch::URL_WHAT_YOU_TYPED) {
+      match_(NULL, 0, false, AutocompleteMatch::URL_WHAT_YOU_TYPED),
+      search_view_(new IconLabelView(font)),
+      ALLOW_THIS_IN_INITIALIZER_LIST(animation_(new ui::SlideAnimation(this))) {
   CHECK_GE(model_index, 0);
   if (icon_size_ == 0) {
     icon_size_ = ResourceBundle::GetSharedInstance().GetBitmapNamed(
         AutocompleteMatch::TypeToIcon(AutocompleteMatch::URL_WHAT_YOU_TYPED))->
         width();
   }
+
+  animation_->SetSlideDuration(500);
+
+  AddChildView(search_view_.get());
 }
 
 AutocompleteResultView::~AutocompleteResultView() {
@@ -133,7 +239,8 @@
 
 void AutocompleteResultView::OnPaint(gfx::Canvas* canvas) {
   const ResultViewState state = GetState();
-  if (state != NORMAL)
+  if (state != NORMAL &&
+      match_.keyword_state != AutocompleteMatch::DUAL_SHOWING_KEYWORD)
     canvas->AsCanvasSkia()->drawColor(GetColor(state, BACKGROUND));
 
   // Paint the icon.
@@ -149,11 +256,29 @@
 void AutocompleteResultView::Layout() {
   icon_bounds_.SetRect(LocationBarView::kEdgeItemPadding,
                        (height() - icon_size_) / 2, icon_size_, icon_size_);
-  int text_x = icon_bounds_.right() + LocationBarView::kItemPadding;
-  int font_height = std::max(normal_font_.GetHeight(), bold_font_.GetHeight());
-  text_bounds_.SetRect(text_x, std::max(0, (height() - font_height) / 2),
-      std::max(bounds().width() - text_x - LocationBarView::kEdgeItemPadding,
-      0), font_height);
+  const int text_x = icon_bounds_.right() + LocationBarView::kItemPadding;
+  const int font_height = std::max(normal_font_.GetHeight(),
+      bold_font_.GetHeight());
+  const int text_y = std::max(0, (height() - font_height) / 2);
+
+  if (match_.has_keyword_hint()) {
+    gfx::Size search_size = search_view_->GetPreferredSize();
+    const float expanded_x = static_cast<float>(text_x +
+        normal_font_.GetStringWidth(UTF8ToUTF16(ui::kEllipsis)) +
+        LocationBarView::kItemPadding);
+    const float collapsed_x = static_cast<float>(bounds().width() -
+        search_size.width());
+    const int search_x = static_cast<int>(collapsed_x - ((collapsed_x -
+        expanded_x) * animation_->GetCurrentValue()));
+
+    text_bounds_.SetRect(text_x, text_y, std::max(search_x - text_x, 0),
+        font_height);
+    search_view_->SetBounds(search_x, 0, std::max(bounds().width() -
+        search_x, 0), height());
+  } else {
+    text_bounds_.SetRect(text_x, text_y,
+        std::max(bounds().width() - text_x, 0), font_height);
+  }
 }
 
 gfx::Size AutocompleteResultView::GetPreferredSize() {
@@ -269,7 +394,8 @@
 
   int icon = match_.starred ?
       IDR_OMNIBOX_STAR : AutocompleteMatch::TypeToIcon(match_.type);
-  if (model_->IsSelectedIndex(model_index_)) {
+  if (model_->IsSelectedIndex(model_index_) &&
+      match_.keyword_state != AutocompleteMatch::DUAL_SHOWING_KEYWORD) {
     switch (icon) {
       case IDR_OMNIBOX_HTTP:    icon = IDR_OMNIBOX_HTTP_SELECTED; break;
       case IDR_OMNIBOX_HISTORY: icon = IDR_OMNIBOX_HISTORY_SELECTED; break;
@@ -354,7 +480,9 @@
       const int style = classifications[i].style;
       const bool use_bold_font = !!(style & ACMatchClassification::MATCH);
       current_data->font = &(use_bold_font ? bold_font_ : normal_font_);
-      const ResultViewState state = GetState();
+      const ResultViewState state =
+          (match_.keyword_state == AutocompleteMatch::DUAL_SHOWING_KEYWORD) ?
+          NORMAL : GetState();
       if (style & ACMatchClassification::URL)
         current_data->color = GetColor(state, URL);
       else if (style & ACMatchClassification::DIM)
@@ -510,3 +638,51 @@
   // We couldn't draw anything.
   runs->clear();
 }
+
+void AutocompleteResultView::SetMatch(const AutocompleteMatch& match) {
+  const bool same_match = match.destination_url == match_.destination_url;
+  const bool is_keyword_mode =
+      match_.keyword_state == AutocompleteMatch::DUAL_SHOWING_KEYWORD;
+  const bool wants_keyword_mode =
+      match.keyword_state == AutocompleteMatch::DUAL_SHOWING_KEYWORD;
+
+  match_ = match;
+
+  if (wants_keyword_mode && !is_keyword_mode)

[Peter Kasting, 2011/04/13 21:06:59]

Do you need to check |same_match| here?

Nit: All arms need {} if one does.

+    animation_->Show();
+  else if (same_match && !wants_keyword_mode && is_keyword_mode)
+    animation_->Hide();
+  else if ((!same_match) || (same_match && !animation_->is_animating())) {

[Peter Kasting, 2011/04/13 21:06:59]

Nit: You can eliminate "same_match &&" and all the internal parens.

+    animation_->Stop();
+    animation_->Reset();
+  }
+
+  search_view_->SetSelected(wants_keyword_mode);
+  search_view_->SetVisible(match_.has_keyword_hint());

[Peter Kasting, 2011/04/13 21:06:59]

Here you really want to check if the match is a DUAL_xxx type.

+
+  if (wants_keyword_mode) {
+    int message_id = match_.keyword_url->IsExtensionKeyword() ?
+        IDS_EXTENSION_KEYWORD_COMMAND : IDS_KEYWORD_SEARCH;
+    string16 text = l10n_util::GetStringFUTF16(message_id,
+        match_.keyword, l10n_util::GetStringUTF16(IDS_EMPTY_KEYWORD_VALUE));

[Peter Kasting, 2011/04/13 21:06:59]

You should be using match_.keyword_url->AdjustedShortNameForLocaleDirection()
instead of match_.keyword here.

+    text.append(
+        l10n_util::GetStringUTF16(
+        IDS_AUTOCOMPLETE_MATCH_DESCRIPTION_SEPARATOR));
+    text.append(
+        l10n_util::GetStringFUTF16(IDS_AUTOCOMPLETE_KEYWORD_DESCRIPTION,
+        match_.keyword));
+    search_view_->SetLabelText(text);
+    search_view_->SetLabelColor(GetColor(SELECTED, DIMMED_TEXT));
+  } else {
+    search_view_->SetLabelText(UTF8ToUTF16(ui::kEllipsis));
+    search_view_->SetLabelColor(GetColor(NORMAL, DIMMED_TEXT));
+  }
+  Layout();
+}
+
+void AutocompleteResultView::AnimationProgressed(
+     const ui::Animation* animation) {
+  Layout();
+  SchedulePaint();
+}
+