Enabled pressing TAB to cycle through the Omnibox results.

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

Index: chrome/browser/ui/views/autocomplete/autocomplete_popup_contents_view.cc
===================================================================
--- chrome/browser/ui/views/autocomplete/autocomplete_popup_contents_view.cc	(revision 95169)
+++ chrome/browser/ui/views/autocomplete/autocomplete_popup_contents_view.cc	(working copy)
@@ -24,6 +24,7 @@
 #include "ui/gfx/canvas_skia.h"
 #include "ui/gfx/insets.h"
 #include "ui/gfx/path.h"
+#include "ui/gfx/rect.h"
 #include "unicode/ubidi.h"
 #include "views/controls/button/text_button.h"
 #include "views/controls/label.h"
@@ -49,6 +50,12 @@
 
 const SkAlpha kGlassPopupAlpha = 240;
 const SkAlpha kOpaquePopupAlpha = 255;
+
+// An alpha value to apply to the AutocompleteResultView dim text color
+// that will help the resulting color (used for dividing result and keyword
+// views) match the popup border color.
+const SkAlpha kResultDividerAlpha = 170;
+
 // The size delta between the font used for the edit and the result rows. Passed
 // to gfx::Font::DeriveFont.
 #if defined(OS_CHROMEOS)
@@ -142,7 +149,7 @@
   virtual ~AutocompletePopupWidget() {}
 
  private:
-   DISALLOW_COPY_AND_ASSIGN(AutocompletePopupWidget);
+  DISALLOW_COPY_AND_ASSIGN(AutocompletePopupWidget);
 };
 
 class AutocompletePopupContentsView::InstantOptInView
@@ -246,6 +253,17 @@
   set_border(bubble_border);
   // The contents is owned by the LocationBarView.
   set_parent_owned(false);
+
+  for (size_t i = 0; i < AutocompleteResult::kMaxMatches * 2; ++i) {
+    AutocompleteResultView* result_view =
+        CreateResultView(this, i, result_font_, result_bold_font_);
+    AddChildViewAt(result_view, static_cast<int>(i));
+    result_view->SetVisible(false);
+
+    if (i < AutocompleteResult::kMaxMatches) {

[sky, 2011/08/09 00:32:51]

Ick, how about two loops. But as I mentioned last time around I don't like
pre-creating the max number of views and animations. As far as the animations
go, could you use BoundsAnimator? It should work for this case.

+      keyword_animations_.push_back(new ui::SlideAnimation(this));

[sky, 2011/08/09 00:32:51]

Create one AnimationContainer and have all animations share it.

+    }
+  }
 }
 
 AutocompletePopupContentsView::~AutocompletePopupContentsView() {
@@ -273,14 +291,37 @@
 void AutocompletePopupContentsView::LayoutChildren() {
   gfx::Rect contents_rect = GetContentsBounds();
   int top = contents_rect.y();
-  for (int i = 0; i < child_count(); ++i) {
-    View* v = child_at(i);
-    if (v->IsVisible()) {
-      v->SetBounds(contents_rect.x(), top, contents_rect.width(),
-                   v->GetPreferredSize().height());
-      top = v->bounds().bottom();
+
+  for (size_t i = 0; i < AutocompleteResult::kMaxMatches * 2; i += 2) {
+    View* result = child_at(i);
+    if (!result->IsVisible())
+      continue;
+
+    AutocompleteResultView* keyword =
+        static_cast<AutocompleteResultView*>(child_at(i + 1));
+    if (keyword->IsVisible()) {
+      const int collapsed_width = keyword->GetCollapsedSize().width() +
+          LocationBarView::kEdgeItemPadding;
+      const int kw_collapsed_x = contents_rect.width() - collapsed_width;
+      const int kw_x = contents_rect.width() -
+          keyword_animations_[i / 2]->CurrentValueBetween(collapsed_width,
+              kw_collapsed_x);
+
+      result->SetBounds(contents_rect.x(), top,
+          kw_x - LocationBarView::kNormalHorizontalEdgeThickness,
+          result->GetPreferredSize().height());
+      keyword->SetBounds(contents_rect.x() + kw_x, top,
+          contents_rect.width() - kw_x, keyword->GetPreferredSize().height());
+    } else {
+      result->SetBounds(contents_rect.x(), top, contents_rect.width(),
+          result->GetPreferredSize().height());
     }
+    top = result->bounds().bottom();
   }
+
+  if (opt_in_view_ && opt_in_view_->IsVisible())
+    opt_in_view_->SetBounds(contents_rect.x(), top, contents_rect.width(),
+        opt_in_view_->GetPreferredSize().height());
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -291,7 +332,14 @@
 }
 
 void AutocompletePopupContentsView::InvalidateLine(size_t line) {
-  child_at(static_cast<int>(line))->SchedulePaint();
+  child_at(static_cast<int>(line * 2))->SchedulePaint();
+  child_at(static_cast<int>(line * 2 + 1))->SchedulePaint();
+
+  if (line == model_->selected_line() &&
+      model_->selected_match() == AutocompletePopupModel::KEYWORD)
+    keyword_animations_[line]->Show();
+  else
+    keyword_animations_[line]->Hide();
 }
 
 void AutocompletePopupContentsView::UpdatePopupAppearance() {
@@ -299,6 +347,11 @@
     // No matches, close any existing popup.
     if (popup_ != NULL) {
       size_animation_.Stop();
+
+      for (SlideAnimations::iterator i(keyword_animations_.begin());
+          i != keyword_animations_.end(); ++i)
+        (*i)->Stop();
+
       // NOTE: Do NOT use CloseNow() here, as we may be deep in a callstack
       // triggered by the popup receiving a message (e.g. LBUTTONUP), and
       // destroying the popup would cause us to read garbage when we unwind back
@@ -316,19 +369,22 @@
     DCHECK(child_rv_count > 0);
     child_rv_count--;
   }
-  for (size_t i = 0; i < model_->result().size(); ++i) {
-    AutocompleteResultView* result_view;
-    if (i >= child_rv_count) {
-      result_view =
-          CreateResultView(this, i, result_font_, result_bold_font_);
-      AddChildViewAt(result_view, static_cast<int>(i));
-    } else {
-      result_view = static_cast<AutocompleteResultView*>(child_at(i));
-      result_view->SetVisible(true);
-    }
-    result_view->SetMatch(GetMatchAtIndex(i));
+  const size_t result_size = model_->result().size();
+  for (size_t i = 0; i < result_size; ++i) {
+    const AutocompleteMatch& match = GetMatchAtIndex(i);
+    AutocompleteResultView* view = static_cast<AutocompleteResultView*>(
+        child_at(i * 2));
+    view->SetMatch(match);
+    view->SetVisible(true);
+
+    view = static_cast<AutocompleteResultView*>(child_at(i * 2 + 1));
+    view->SetVisible(match.associated_keyword.get() != NULL);
+    if (view->IsVisible())
+      view->SetMatch(*match.associated_keyword);
+
+    keyword_animations_[i]->Reset();
   }
-  for (size_t i = model_->result().size(); i < child_rv_count; ++i)
+  for (size_t i = result_size * 2; i < child_rv_count; ++i)
     child_at(i)->SetVisible(false);
 
   PromoCounter* counter = model_->profile()->GetInstantPromoCounter();
@@ -375,7 +431,7 @@
     popup_->SetBounds(GetPopupBounds());
   }
 
-  SchedulePaint();
+  Layout();
 }
 
 gfx::Rect AutocompletePopupContentsView::GetTargetBounds() {
@@ -394,11 +450,16 @@
 // AutocompletePopupContentsView, AutocompleteResultViewModel implementation:
 
 bool AutocompletePopupContentsView::IsSelectedIndex(size_t index) const {
-  return HasMatchAt(index) ? index == model_->selected_line() : false;
+  size_t selected_line = model_->selected_line() * 2;
+
+  if (model_->selected_match() == AutocompletePopupModel::KEYWORD)
+    selected_line++;
+
+  return index == selected_line;
 }
 
 bool AutocompletePopupContentsView::IsHoveredIndex(size_t index) const {
-  return HasMatchAt(index) ? index == model_->hovered_line() : false;
+  return index == model_->hovered_line() * 2;
 }
 
 const SkBitmap* AutocompletePopupContentsView::GetIconIfExtensionMatch(
@@ -416,6 +477,8 @@
   // We should only be running the animation when the popup is already visible.
   DCHECK(popup_ != NULL);
   popup_->SetBounds(GetPopupBounds());
+
+  Layout();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -509,6 +572,20 @@
   canvas->drawColor(AutocompleteResultView::GetColor(
       AutocompleteResultView::NORMAL, AutocompleteResultView::BACKGROUND));
   View::PaintChildren(canvas);
+
+  // Draw dividing lines between results and keyword views.
+  SkColor divider_color = SkColorSetA(AutocompleteResultView::GetColor(
+      AutocompleteResultView::NORMAL, AutocompleteResultView::DIMMED_TEXT),
+      kResultDividerAlpha);
+
+  for (size_t i = 0; i < model_->result().size(); ++i) {
+    if (model_->result().match_at(i).associated_keyword.get()) {
+      views::View* child = child_at(i * 2);
+      canvas->FillRectInt(divider_color, child->bounds().right(),
+          child->bounds().y(), LocationBarView::kNormalHorizontalEdgeThickness,
+          child->bounds().height());
+    }
+  }
 }
 
 int AutocompletePopupContentsView::CalculatePopupHeight() {
@@ -645,10 +722,8 @@
   // extension, |match| and its contents.  So copy the relevant match out to
   // make sure it stays alive until the call completes.
   AutocompleteMatch match = model_->result().match_at(index);
-  string16 keyword;
-  const bool is_keyword_hint = model_->GetKeywordForMatch(match, &keyword);
   omnibox_view_->OpenMatch(match, disposition, GURL(), index,
-                         is_keyword_hint ? string16() : keyword);
+      match.keyword);
 }
 
 size_t AutocompletePopupContentsView::GetIndexForPoint(
@@ -659,7 +734,7 @@
   int nb_match = model_->result().size();
   DCHECK(nb_match <= child_count());
   for (int i = 0; i < nb_match; ++i) {
-    views::View* child = child_at(i);
+    views::View* child = child_at(i * 2);
     gfx::Point point_in_child_coords(point);
     View::ConvertPointToView(this, child, &point_in_child_coords);
     if (child->HitTest(point_in_child_coords))