alternate ntp: implement right-aligned search token

chrome/browser/ui/views/location_bar/location_bar_view.cc

Index: chrome/browser/ui/views/location_bar/location_bar_view.cc
diff --git a/chrome/browser/ui/views/location_bar/location_bar_view.cc b/chrome/browser/ui/views/location_bar/location_bar_view.cc
index 3a3c56242912321467630f46248f638eaa92c0f5..ba6bbda282133d7bfc49b8b9adfbc3004d91fe3c 100644
--- a/chrome/browser/ui/views/location_bar/location_bar_view.cc
+++ b/chrome/browser/ui/views/location_bar/location_bar_view.cc
@@ -29,6 +29,7 @@
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/omnibox/location_bar_util.h"
 #include "chrome/browser/ui/omnibox/omnibox_popup_model.h"
+#include "chrome/browser/ui/search/search.h"
 #include "chrome/browser/ui/tab_contents/tab_contents.h"
 #include "chrome/browser/ui/view_ids.h"
 #include "chrome/browser/ui/views/bookmarks/bookmark_prompt_view.h"
@@ -668,8 +669,10 @@ void LocationBarView::Layout() {
   if (!action_box_button_view_)
     entry_width -= GetEdgeItemPadding();
 
-  // |location_icon_view_| is visible except when |ev_bubble_view_| or
-  // |selected_keyword_view_| are visible.
+  // |location_icon_view_| is visible except when |ev_bubble_view_| on the left
+  // of edit or |selected_keyword_view_| are visible; if |ev_bubble_view_| is
+  // visible on the right of edit, |location_icon_view_| could be visible too
+  // (see comments below for instant extended API).
   int location_icon_width = 0;
   int ev_bubble_width = 0;
   location_icon_view_->SetVisible(false);
@@ -691,12 +694,36 @@ void LocationBarView::Layout() {
     location_icon_width = location_icon_view_->GetPreferredSize().width();
     entry_width -= (kEdgeThickness + GetEdgeItemPadding() +
                     location_icon_width + kItemEditPadding);
+
+    // For instant extended API, omnibox could be secure and replacing the url
+    // with query terms, in which case, show the ev bubble view with the search
+    // provider name on the right of the edit box.
+    if (chrome::search::IsInstantExtendedAPIEnabled(profile_) &&
+        model_->GetSecurityLevel() == ToolbarModel::SECURE &&
+        model_->WouldReplaceSearchURLWithSearchTerms()) {
+      const TemplateURL* template_url =
+          TemplateURLServiceFactory::GetForProfile(profile_)->
+              GetDefaultSearchProvider();
+      if (template_url) {
+        ev_bubble_view_->SetVisible(true);
+        ev_bubble_view_->SetLabel(l10n_util::GetStringFUTF16(
+            IDS_OMNIBOX_EV_SEARCH_TEXT, template_url->short_name()));
+        // We'll adjust this width and take it out of |entry_width| below.
+        ev_bubble_width = ev_bubble_view_->GetPreferredSize().width();

[beaudoin, 2012/11/29 15:07:50]

Could we go with just a bool or an enum here and keep the size calculations for
later?
(Then, we could remove the cut-and-paste for calculating ev_bubble_width.)

[kuan, 2012/11/29 16:48:27]

Done.

+      }
+    }
   }
 
   if (action_box_button_view_) {
     action_box_button_view_->SetVisible(true);
     entry_width -= action_box_button_view_->width() + GetItemPadding();
   }
+
+  // Remember |entry_width| before considering the decorations between right of
+  // edit box and action box, so as to know if there are decorations to the left
+  // of the EV bubble that's showing on the right of edit box.
+  int entry_width_for_left_of_action_box = entry_width;

[beaudoin, 2012/11/29 15:07:50]

Comment and variable name here are hard to understand. I would personally prefer
a bool that you set to false in any of the "ifs" below, but the approach has
drawbacks too, so you can keep this approach. Just make sure you clean-up the
comment to mention that the only goal of this "int" is to determine whether any
other decoration is added.

[kuan, 2012/11/29 16:48:27]

Done.  i tried your bool approach before, but felt it was too repetitive, so i
changed to using an int.

[beaudoin, 2012/11/30 01:24:30]

Ack.

+
   if (star_view_ && star_view_->visible())
     entry_width -= star_view_->GetPreferredSize().width() + GetItemPadding();
   if (open_pdf_in_reader_view_ && open_pdf_in_reader_view_->visible()) {
@@ -719,6 +746,10 @@ void LocationBarView::Layout() {
     entry_width -= web_intents_button_view_->GetPreferredSize().width() +
         GetItemPadding();
   }
+
+  bool has_icons_left_of_action_box =
+      entry_width_for_left_of_action_box != entry_width;
+
   // The gap between the edit and whatever is to its right is shortened.
   entry_width += kEditInternalSpace;
 
@@ -729,8 +760,20 @@ void LocationBarView::Layout() {
     // space, but never elide it any smaller than 150 px.
     static const int kMinElidedBubbleWidth = 150;
     static const double kMaxBubbleFraction = 0.5;
-    const int total_padding =
-        kEdgeThickness + kBubbleHorizontalPadding + kItemEditPadding;
+    int total_padding = 0;
+    if (location_icon_width == 0) {  // EV bubble is on the left of edit box.

[beaudoin, 2012/11/29 15:07:50]

Again, I think a bool/enum would make this much easier to understand.

[kuan, 2012/11/29 16:48:27]

Done.

+      total_padding = kEdgeThickness + kBubbleHorizontalPadding +
+                      kItemEditPadding;
+    } else {  // EV bubble is on the right of edit box.
+      if (!action_box_button_view_)  // EV bubble is first from the right.
+        total_padding = kEdgeThickness + kBubbleHorizontalPadding;
+      else  // EV bubble is left of action box.
+        total_padding = GetItemPadding();
+      if (has_icons_left_of_action_box)  // Has icons on left of EV bubble.
+        total_padding += GetItemPadding();
+      else  // EV bubble is beside edit box.
+        total_padding += kItemEditPadding;
+    }
     ev_bubble_width = std::min(ev_bubble_width, std::max(kMinElidedBubbleWidth,
         static_cast<int>((entry_width - total_padding) * kMaxBubbleFraction)));
     entry_width -= (total_padding + ev_bubble_width);
@@ -768,6 +811,7 @@ void LocationBarView::Layout() {
 
   // Lay out items to the right of the edit field.
   int offset = width() - kEdgeThickness;
+  bool ev_bubble_on_right = ev_bubble_width > 0 && location_icon_width > 0;

[beaudoin, 2012/11/29 15:07:50]

I would set that bool much earlier, as soon as you know it's on the right.
Probably start it as false and turn it true inside the "if" at line 707.

Better yet: a 3-way enum:
NOT_VISIBLE, VISIBLE_ON_LEFT, VISIBLE_ON_RIGHT
that could be used instead of ev_bubble_width as an indicator. It would make the
entire code clearer IMHO.

[kuan, 2012/11/29 16:48:27]

Done.

   if (action_box_button_view_) {
     offset -= action_box_button_view_->width();
     action_box_button_view_->SetPosition(
@@ -775,10 +819,20 @@ void LocationBarView::Layout() {
                    kVerticalEdgeThickness -
                        ActionBoxButtonView::kBorderOverlap));
     offset -= GetItemPadding();
+  } else if (ev_bubble_on_right) {
+    offset -= kBubbleHorizontalPadding;
   } else {
     offset -= GetEdgeItemPadding();
   }
 
+  if (ev_bubble_on_right) {
+    offset -= ev_bubble_width;
+    ev_bubble_view_->SetBounds(offset, location_y + kBubbleVerticalPadding,
+                               ev_bubble_width,
+                               ev_bubble_view_->GetPreferredSize().height());
+    offset -= GetItemPadding();
+  }
+
   if (star_view_ && star_view_->visible()) {
     offset += star_view_->GetBuiltInHorizontalPadding();
     int star_width = star_view_->GetPreferredSize().width();