alternate ntp: implement right-aligned search token

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

 // Copyright (c) 2012 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/location_bar/location_bar_view.h"
 
 #include <algorithm>
 #include <map>
 
 #include "base/command_line.h"
@@ skipping 11 matching lines @@
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search_engines/template_url.h"
 #include "chrome/browser/search_engines/template_url_service.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
 #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"
 #include "chrome/browser/ui/views/browser_dialogs.h"
 #include "chrome/browser/ui/views/extensions/extension_popup.h"
 #include "chrome/browser/ui/views/location_bar/action_box_button_view.h"
 #include "chrome/browser/ui/views/location_bar/content_setting_image_view.h"
 #include "chrome/browser/ui/views/location_bar/ev_bubble_view.h"
 #include "chrome/browser/ui/views/location_bar/keyword_hint_view.h"
 #include "chrome/browser/ui/views/location_bar/location_icon_view.h"
@@ skipping 619 matching lines @@
   const int kBubbleVerticalPadding = (mode_ == POPUP) ?
       -1 : kBubbleHorizontalPadding;
 
   // Start by reserving the padding at the right edge.
   int entry_width = width() - kEdgeThickness;
   // No need for edge item padding with action box as it fills
   // all the area on the right.
   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);
   ev_bubble_view_->SetVisible(false);
 
   const string16 keyword(location_entry_->model()->keyword());
   const bool is_keyword_hint(location_entry_->model()->is_keyword_hint());
   const bool show_selected_keyword = !keyword.empty() && !is_keyword_hint;
   if (show_selected_keyword) {
     // Assume the keyword might be hidden.
     entry_width -= (kEdgeThickness + kEdgeEditPadding);
   } else if (model_->GetSecurityLevel() == ToolbarModel::EV_SECURE) {
     ev_bubble_view_->SetVisible(true);
     ev_bubble_view_->SetLabel(model_->GetEVCertName());
     ev_bubble_width = ev_bubble_view_->GetPreferredSize().width();
     // We'll adjust this width and take it out of |entry_width| below.
   } else {
     location_icon_view_->SetVisible(true);
     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()) {
     entry_width -= open_pdf_in_reader_view_->GetPreferredSize().width() +
         GetItemPadding();
   }
   for (PageActionViews::const_iterator i(page_action_views_.begin());
        i != page_action_views_.end(); ++i) {
     if ((*i)->visible())
       entry_width -= ((*i)->GetPreferredSize().width() + GetItemPadding());
   }
   if (zoom_view_->visible())
     entry_width -= zoom_view_->GetPreferredSize().width() + GetItemPadding();
   for (ContentSettingViews::const_iterator i(content_setting_views_.begin());
        i != content_setting_views_.end(); ++i) {
     if ((*i)->visible())
       entry_width -= ((*i)->GetPreferredSize().width() + GetItemPadding());
   }
   if (web_intents_button_view_->visible()) {
     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;
 
   // Size the EV bubble after taking star/page actions/content settings out of
   // |entry_width| so we won't take too much space.
   if (ev_bubble_width) {
     // Try to elide the bubble to be no larger than half the total available
     // 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);
   }
 
   const int max_edit_width = location_entry_->GetMaxEditWidth(entry_width);
   if (max_edit_width < 0)
     return;
 
   const bool show_keyword_hint = !keyword.empty() && is_keyword_hint;
@@ skipping 17 matching lines @@
         selected_keyword_view_->set_is_extension_icon(false);
       }
     }
   } else if (show_keyword_hint) {
     if (keyword_hint_view_->keyword() != keyword)
       keyword_hint_view_->SetKeyword(keyword);
   }
 
   // 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(
         gfx::Point(offset,
                    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();
     offset -= star_width;
     star_view_->SetBounds(offset, location_y, star_width, location_height);
     offset -= GetItemPadding() - star_view_->GetBuiltInHorizontalPadding();
   }
 
   if (open_pdf_in_reader_view_ && open_pdf_in_reader_view_->visible()) {
     offset += open_pdf_in_reader_view_->GetBuiltInHorizontalPadding();
@@ skipping 726 matching lines @@
 int LocationBarView::GetInternalHeight(bool use_preferred_size) {
   int total_height =
       use_preferred_size ? GetPreferredSize().height() : height();
   return std::max(total_height - (kVerticalEdgeThickness * 2), 0);
 }
 
 bool LocationBarView::HasValidSuggestText() const {
   return suggested_text_view_ && !suggested_text_view_->size().IsEmpty() &&
       !suggested_text_view_->text().empty();
 }