Enabled pressing TAB to cycle through the Omnibox results.

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

 // Copyright (c) 2011 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.
 
 #include "chrome/browser/ui/views/autocomplete/autocomplete_popup_contents_view.h"
 
 #include "base/compiler_specific.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/autocomplete/autocomplete_popup_model.h"
 #include "chrome/browser/instant/instant_confirm_dialog.h"
 #include "chrome/browser/instant/promo_counter.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/omnibox/omnibox_view.h"
 #include "chrome/browser/ui/views/autocomplete/autocomplete_result_view.h"
 #include "chrome/browser/ui/views/bubble/bubble_border.h"
 #include "chrome/browser/ui/views/location_bar/location_bar_view.h"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
 #include "third_party/skia/include/core/SkShader.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/base/theme_provider.h"
 #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"
 #include "views/layout/grid_layout.h"
 #include "views/layout/layout_constants.h"
 #include "views/painter.h"
 #include "views/widget/widget.h"
 
 #if defined(OS_WIN)
 #include <commctrl.h>
@@ skipping 98 matching lines @@
 }  // namespace
 
 class AutocompletePopupContentsView::AutocompletePopupWidget
     : public views::Widget,
       public base::SupportsWeakPtr<AutocompletePopupWidget> {
  public:
   AutocompletePopupWidget() {}
   virtual ~AutocompletePopupWidget() {}
 
  private:
-   DISALLOW_COPY_AND_ASSIGN(AutocompletePopupWidget);
+  DISALLOW_COPY_AND_ASSIGN(AutocompletePopupWidget);
 };
 
 class AutocompletePopupContentsView::InstantOptInView
     : public views::View,
       public views::ButtonListener {
  public:
   InstantOptInView(AutocompletePopupContentsView* contents_view,
                    const gfx::Font& label_font,
                    const gfx::Font& button_font)
       : contents_view_(contents_view),
@@ skipping 83 matching lines @@
       result_bold_font_(result_font_.DeriveFont(0, gfx::Font::BOLD)),
       ignore_mouse_drag_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(size_animation_(this)) {
   // The following little dance is required because set_border() requires a
   // pointer to a non-const object.
   BubbleBorder* bubble_border = new BubbleBorder(BubbleBorder::NONE);
   bubble_border_ = bubble_border;
   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) {

[sky, 2011/08/01 16:02:15]

I don't like always creating the max result views and slideanimations.

+    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) {
+      ui::SlideAnimation* anim = new ui::SlideAnimation(this);
+      anim->SetSlideDuration(500);

[sky, 2011/08/01 16:02:15]

500 is too long. Can you use the default?

+      keyword_animations_.push_back(anim);
+    }
+  }
 }
 
 AutocompletePopupContentsView::~AutocompletePopupContentsView() {
   // We don't need to do anything with |popup_| here.  The OS either has already
   // closed the window, in which case it's been deleted, or it will soon, in
   // which case there's nothing we need to do.
 }
 
 gfx::Rect AutocompletePopupContentsView::GetPopupBounds() const {
   if (!size_animation_.is_animating())
     return target_bounds_;
 
   gfx::Rect current_frame_bounds = start_bounds_;
   int total_height_delta = target_bounds_.height() - start_bounds_.height();
   // Round |current_height_delta| instead of truncating so we won't leave single
   // white pixels at the bottom of the popup as long when animating very small
   // height differences.
   int current_height_delta = static_cast<int>(
       size_animation_.GetCurrentValue() * total_height_delta - 0.5);
   current_frame_bounds.set_height(
       current_frame_bounds.height() + current_height_delta);
   return current_frame_bounds;
 }
 
 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 (int i = 0; i < child_count(); i += 2) {
+    View* result = child_at(i);
+    if (!result->IsVisible())
+      continue;
+
+    AutocompleteResultView* keyword =

[sky, 2011/08/01 16:02:15]

Casts like this are easy to get wrong. Use an id or classname to find the views
you care about.

[aaron.randolph, 2011/08/02 21:54:58]

Just to be clear, are you suggesting I keep a list of AutocompleteResultView* as
a member and iterate through that instead of using child_at?  Or did you have an
alternative idea in mind?

[Peter Kasting, 2011/08/04 20:07:53]

Nit: TBH, I don't think this suggestion of Scott's is a net win, especially if
we're just DCHECKing it (GetClassName() will never get called in release mode,
etc., meaning we add a lot of boilerplate for little gain).

Maybe if instead of using "child_count()" above you use
"AutocompleteResult::kMaxMatches * 2", this will be safer against the addition
of more children later, and then you can go back to just casting directly.  That
will also make this code a bit less confusing w.r.t. apparently sizing all
children in this loop but not actually doing so for the |opt_in_view_|.

+        static_cast<AutocompleteResultView*>(child_at(i + 1));
+    if (keyword->IsVisible()) {
+      const float collapsed_width = static_cast<float>(
+          keyword->GetCollapsedSize().width() + LocationBarView::kItemPadding);
+      const float kw_collapsed_x = static_cast<float>(contents_rect.width() -
+          collapsed_width);
+      const int kw_x = static_cast<int>(kw_collapsed_x - ((kw_collapsed_x -
+          collapsed_width) * keyword_animations_[i / 2]->GetCurrentValue()));

[sky, 2011/08/01 16:02:15]

To make lifetime easier can be push the animation into the resultsview? And use
CurrentValueBetween.

+
+      result->SetBounds(contents_rect.x(), top,
+          kw_x - LocationBarView::kItemPadding,

[Peter Kasting, 2011/07/29 21:08:44]

Don't we only want to subtract 1?

+          result->GetPreferredSize().height());
+      keyword->SetBounds(kw_x + LocationBarView::kItemPadding, top,
+          contents_rect.width() - (kw_x - LocationBarView::kItemPadding),
+          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());
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // AutocompletePopupContentsView, AutocompletePopupView overrides:
 
 bool AutocompletePopupContentsView::IsOpen() const {
   return (popup_ != NULL);
 }
 
 void AutocompletePopupContentsView::InvalidateLine(size_t line) {
-  child_at(static_cast<int>(line))->SchedulePaint();
+    child_at(static_cast<int>(line * 2))->SchedulePaint();

[sky, 2011/08/01 16:02:15]

spacing if off.

+    child_at(static_cast<int>(line * 2 + 1))->SchedulePaint();
+
+    if (line == model_->selected_line() && model_->keyword_selected())
+      keyword_animations_[line]->Show();
+    else
+      keyword_animations_[line]->Hide();
 }
 
 void AutocompletePopupContentsView::UpdatePopupAppearance() {
   if (model_->result().empty()) {
     // 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
       // to that level.
       popup_->Close();  // This will eventually delete the popup.
       popup_.reset();
     }
     return;
   }
 
   // Update the match cached by each row, in the process of doing so make sure
   // we have enough row views.
   size_t child_rv_count = child_count();
   if (opt_in_view_) {
     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();
   if (!opt_in_view_ && counter && counter->ShouldShow(base::Time::Now())) {
     opt_in_view_ = new InstantOptInView(this, result_bold_font_, result_font_);
     AddChildView(opt_in_view_);
   } else if (opt_in_view_ && (!counter ||
                               !counter->ShouldShow(base::Time::Now()))) {
     delete opt_in_view_;
     opt_in_view_ = NULL;
@@ skipping 26 matching lines @@
     // Animate the popup shrinking, but don't animate growing larger since that
     // would make the popup feel less responsive.
     start_bounds_ = GetWidget()->GetWindowScreenBounds();
     if (target_bounds_.height() < start_bounds_.height())
       size_animation_.Show();
     else
       start_bounds_ = target_bounds_;
     popup_->SetBounds(GetPopupBounds());
   }
 
-  SchedulePaint();
+  Layout();
 }
 
 gfx::Rect AutocompletePopupContentsView::GetTargetBounds() {
   return target_bounds_;
 }
 
 void AutocompletePopupContentsView::PaintUpdatesNow() {
   // TODO(beng): remove this from the interface.
 }
 
 void AutocompletePopupContentsView::OnDragCanceled() {
   ignore_mouse_drag_ = true;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // 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_->keyword_selected())
+    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(
     size_t index) const {
   if (!HasMatchAt(index))
     return NULL;
   return model_->GetIconIfExtensionMatch(GetMatchAtIndex(index));
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // AutocompletePopupContentsView, AnimationDelegate implementation:
 
 void AutocompletePopupContentsView::AnimationProgressed(
     const ui::Animation* animation) {
   // We should only be running the animation when the popup is already visible.
   DCHECK(popup_ != NULL);
   popup_->SetBounds(GetPopupBounds());
+
+  Layout();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // AutocompletePopupContentsView, views::View overrides:
 
 void AutocompletePopupContentsView::Layout() {
   UpdateBlurRegion();
 
   // Size our children to the available content area.
   LayoutChildren();
@@ skipping 73 matching lines @@
   model_->SetHoveredLine(AutocompletePopupModel::kNoMatch);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // AutocompletePopupContentsView, protected:
 
 void AutocompletePopupContentsView::PaintResultViews(gfx::CanvasSkia* canvas) {
   canvas->drawColor(AutocompleteResultView::GetColor(
       AutocompleteResultView::NORMAL, AutocompleteResultView::BACKGROUND));
   View::PaintChildren(canvas);
+
+  // Draw dividing lines between results and keyword views.
+  SkPaint paint;
+  paint.setColor(AutocompleteResultView::GetColor(
+    AutocompleteResultView::NORMAL, AutocompleteResultView::DIMMED_TEXT));
+  paint.setStrokeWidth(SkIntToScalar(1));
+  paint.setAntiAlias(true);
+
+  for (size_t i = 0; i < model_->result().size(); ++i) {
+    views::View* kw_child = child_at(i * 2 + 1);
+    if (kw_child->IsVisible()) {
+      gfx::Rect bounds = kw_child->bounds();
+
+      canvas->drawLine(
+        SkIntToScalar(bounds.x() -
+            LocationBarView::kNormalHorizontalEdgeThickness),

[Peter Kasting, 2011/07/29 21:08:44]

Nit: Simpler: Use the even children and draw at (bounds.right()) instead of
using the odd children and subtracting a constant.

[aaron.randolph, 2011/07/29 21:57:10]

I would still need to add the constant to the right edge in order to get the
line centered between the two views.  Otherwise, the line draws on the edge of
the selection, and I was under the belief you wanted a gap from the selection
border?

[Peter Kasting, 2011/07/30 00:00:29]

I was imagining the left and right sides having exactly one pixel -- the border
pixel -- between them.  So e.g. left side is from [0, 10), border pixel at 10,
right side at [11, 21) or similar.  That would mean that the border pixel could
be drawn at the left side's ".right()" since the left side would draw "up to but
not including" that pixel.

If you implement this and it looks dumb we can add padding, but I figured this
would be more symmetrical with how we draw the selection right up to (but not
including) the outside edge of the popup.

[aaron.randolph, 2011/07/30 00:34:17]

I'm with you now.  I was thinking right() was inclusive, not exclusive.  I've
updated it for the next CL.

+        SkIntToScalar(bounds.y() +
+            LocationBarView::kNormalHorizontalEdgeThickness),
+        SkIntToScalar(bounds.x() -
+            LocationBarView::kNormalHorizontalEdgeThickness),
+        SkIntToScalar(bounds.y() + bounds.height() -
+            LocationBarView::kNormalHorizontalEdgeThickness * 2),
+        paint);
+    }
+  }
 }
 
 int AutocompletePopupContentsView::CalculatePopupHeight() {
   DCHECK_GE(static_cast<size_t>(child_count()), model_->result().size());
   int popup_height = 0;
   for (size_t i = 0; i < model_->result().size(); ++i)
     popup_height += child_at(i)->GetPreferredSize().height();
   return popup_height +
       (opt_in_view_ ? opt_in_view_->GetPreferredSize().height() : 0);
 }
@@ skipping 116 matching lines @@
 void AutocompletePopupContentsView::OpenIndex(
     size_t index,
     WindowOpenDisposition disposition) {
   if (!HasMatchAt(index))
     return;
 
   // OpenMatch() may close the popup, which will clear the result set and, by
   // 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(
     const gfx::Point& point) {
   if (!HitTest(point))
     return AutocompletePopupModel::kNoMatch;
 
   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))
       return i;
   }
   return AutocompletePopupModel::kNoMatch;
 }
 
 gfx::Rect AutocompletePopupContentsView::CalculateTargetBounds(int h) {
   gfx::Rect location_bar_bounds(location_bar_->GetContentsBounds());
@@ skipping 23 matching lines @@
   opt_in_view_ = NULL;
   PromoCounter* counter = model_->profile()->GetInstantPromoCounter();
   DCHECK(counter);
   counter->Hide();
   if (opt_in) {
     browser::ShowInstantConfirmDialogIfNecessary(
         location_bar_->GetWidget()->GetNativeWindow(), model_->profile());
   }
   UpdatePopupAppearance();
 }