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 110 matching lines @@
   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();
+  size_t child_rv_count = child_count();
+
+  for (size_t i = 0; i < child_rv_count; i += 2) {
+    View* result = child_at(i);
+    if (!result->IsVisible())
+      continue;
+
+    AutocompleteResultView* keyword =
+        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()));
+
+      result->SetBounds(contents_rect.x(), top, kw_x,
+          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())

[Peter Kasting, 2011/07/27 20:18:25]

How come we didn't use to have this?  Was it an error?

[aaron.randolph, 2011/07/28 16:01:55]

The old loop adjusted all children, implicitly including the opt_in_view.  With
the new loop, I increment by two, resulting in it skipping the final
(opt_in_view) view.  So I made it explicit.  Looking at it again, it appears
child_rv_count is unnecessary so I'll remove that.

+    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();
+    child_at(static_cast<int>(line * 2 + 1))->SchedulePaint();
+
+    if (line == model_->selected_line() && model_->keyword_selected()) {

[Peter Kasting, 2011/07/27 20:18:25]

Nit: No need for {}

+      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));
+  const size_t result_size = model_->result().size();
+  CreateCachedViews(child_rv_count, result_size * 2);

[Peter Kasting, 2011/07/27 20:18:25]

Nit: It's always bugged me that we create child views on demand but don't
destroy them when they're no longer needed.  It seems like it would be simpler
to just create all the children up front in the constructor, and make them all
hidden.  We could create all the keyword animations at that point too.

[aaron.randolph, 2011/07/28 16:01:55]

I agree.  I'll make that change for the next update.

+  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);
+
+    // Create a view and animation for an associated keyword, even if
+    // the match doesn't have a keyword.
+    view = static_cast<AutocompleteResultView*>(child_at(i * 2 + 1));
+    if (match.associated_keyword.get()) {

[Peter Kasting, 2011/07/27 20:18:25]

Nit: Shorter:

    view->SetVisible(match.associated_keyword.get());
    if (view->IsVisible())
      view->SetMatch(*match.associated_keyword);

+      view->SetMatch(*match.associated_keyword);
+      view->SetVisible(true);
     } else {
-      result_view = static_cast<AutocompleteResultView*>(child_at(i));
-      result_view->SetVisible(true);
+      view->SetVisible(false);
     }
-    result_view->SetMatch(GetMatchAtIndex(i));
+
+    if (keyword_animations_.size() <= i) {
+      ui::SlideAnimation* anim = new ui::SlideAnimation(this);

[Peter Kasting, 2011/07/27 20:18:25]

Nit: Instead of using a ScopedVector and allocating these on the heap, could we
just use a ui::SlideAnimation[]? This way they'd all be stack-allocated and it'd
hopefully be a fraction simpler.

+      anim->SetSlideDuration(500);
+      keyword_animations_.push_back(anim);
+    }
+    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());
   }
 
+  LayoutChildren();

[Peter Kasting, 2011/07/27 20:18:25]

Is this call necessary?  It seems like it ought not to be, because if it were,
then we'd presumably also need to UpdateBlurRegion() like Layout() does.

[aaron.randolph, 2011/07/28 16:01:55]

It's necessary so the result views will resize to include keyword hints. 
Without it, the keyword hints don't display.   I'll change both these lines to
just a call to Layout().

   SchedulePaint();
 }
 
+void AutocompletePopupContentsView::CreateCachedViews(
+    size_t child_count,
+    size_t required_count) {
+  for (size_t i = child_count; i < required_count; ++i) {
+    AutocompleteResultView* result_view =
+        CreateResultView(this, i, result_font_, result_bold_font_);
+    AddChildViewAt(result_view, static_cast<int>(i));
+  }
+}
+
 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

[Peter Kasting, 2011/07/27 20:18:25]

Nit: Trailing period

+  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(

[Peter Kasting, 2011/07/27 20:18:25]

If I'm reading this right, it draws the line in the first column of pixels on
the keyword side.

I think it'd be better to actually have 1 px between the normal and keyword
portions of the row and draw the line inside this pixel.  That way the line
would be outside the selected area no matter which side is selected.

+        SkIntToScalar(bounds.x()),
+        SkIntToScalar(bounds.y() +
+            LocationBarView::kNormalHorizontalEdgeThickness),
+        SkIntToScalar(bounds.x()),
+        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();
 }