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 9714e3c17e304bd4319f045a765b344e1fc84bdb..00a46cdd35efe8fd89b61fdfc20ee93a52df0731 100644
--- a/chrome/browser/ui/views/location_bar/location_bar_view.cc
+++ b/chrome/browser/ui/views/location_bar/location_bar_view.cc
@@ -44,6 +44,7 @@
 #include "chrome/browser/ui/views/location_bar/page_action_image_view.h"
 #include "chrome/browser/ui/views/location_bar/page_action_with_badge_view.h"
 #include "chrome/browser/ui/views/location_bar/script_bubble_icon_view.h"
+#include "chrome/browser/ui/views/location_bar/search_token_view.h"
 #include "chrome/browser/ui/views/location_bar/selected_keyword_view.h"
 #include "chrome/browser/ui/views/location_bar/star_view.h"
 #include "chrome/browser/ui/views/location_bar/web_intents_button_view.h"
@@ -177,6 +178,7 @@ LocationBarView::LocationBarView(Browser* browser,
       selected_keyword_view_(NULL),
       suggested_text_view_(NULL),
       keyword_hint_view_(NULL),
+      search_token_view_(NULL),
       zoom_view_(NULL),
       open_pdf_in_reader_view_(NULL),
       script_bubble_icon_view_(NULL),
@@ -264,6 +266,11 @@ void LocationBarView::Init() {
   keyword_hint_view_->SetVisible(false);
   keyword_hint_view_->SetFont(font_);
 
+  search_token_view_ = new SearchTokenView(this);
+  search_token_view_->SetFont(font_);
+  AddChildView(search_token_view_);
+  search_token_view_->SetVisible(false);

[Peter Kasting, 2012/12/17 21:01:00]

Nit: For consistency and efficiency, can you make all the blocks in this
function have this order:

  foo = new Foo;
  foo->SetVisible(false);
  foo->SetFont(font_);
  AddChildView(foo);

This will do the minimum number of relayouts.

[kuan, 2012/12/20 00:26:14]

Done.

+
   for (int i = 0; i < CONTENT_SETTINGS_NUM_TYPES; ++i) {
     ContentSettingImageView* content_blocked_view =
         new ContentSettingImageView(static_cast<ContentSettingsType>(i),
@@ -424,6 +431,16 @@ void LocationBarView::Update(const WebContents* tab_for_state_restoring) {
   if (star_view_ && !extensions::FeatureSwitch::action_box()->IsEnabled())
     star_view_->SetVisible(star_enabled);
 
+  string16 search_provider;
+  if (model_->WouldReplaceSearchURLWithSearchTerms()) {
+    const TemplateURL* template_url =
+        TemplateURLServiceFactory::GetForProfile(profile_)->
+            GetDefaultSearchProvider();
+    if (template_url)
+      search_provider = template_url->short_name();
+  }
+  search_token_view_->SetSearchProvider(search_provider);
+
   // Don't Update in app launcher mode so that the location entry does not show
   // a URL or security background.
   if (mode_ != APP_LAUNCHER)
@@ -902,6 +919,27 @@ void LocationBarView::Layout() {
     }
   }
 
+  // If |search_token_view_| has search provider and there's enough space left
+  // after query string in the omnibox, lay it out on the immediate right of the
+  // query string.
+  if (search_token_view_->HasSearchProvider()) {
+    // |entry_width| has already been enlarged by |kEditInternalSpace|, but
+    // |search_token_view_| is a trailing view, and doesn't need it to the
+    // right.  As for the padding to the left, the view itself manages it.

[Peter Kasting, 2012/12/17 21:01:00]

This isn't right, see comments in search_token_view.cc.

[kuan, 2012/12/20 00:26:14]

Done.

+    location_bounds.Inset(0, 0, kEditInternalSpace, 0);
+    int pref_width = search_token_view_->GetPreferredSize().width();
+    search_token_view_->SetVisible(pref_width < AvailableWidth(max_edit_width));

[Peter Kasting, 2012/12/17 21:01:00]

Use LayoutView() for calculating visibility and setting bounds, as we do above.

[kuan, 2012/12/20 00:26:14]

i initially tried that, and discovered that LayoutView() only sets view
invisible if view's preferred/min width is less than location_bounds.width,
whereas i need to set view invisible if view's width is less than
AvailableWidth(max_edit_width).
e.g. when i manually resized the chrome window till the narrowest possible, the
left tab-to-search blue bubble always showed up, and instead the omnibox text
got shrunk.
the right-aligned search token, on the other hand, needs to disappear in this
scenario.

[Peter Kasting, 2012/12/20 01:45:39]

I suggest modifying LayoutView() to do the right thing.  I would probably make
it take one more argument that, when the view doesn't fit in the available
width, toggles between "resize to min width" and "hide altogether".

Another possibility is that instead of modifying LayoutView() you could maybe
make the view's GetMinimumSize() function return zero width.  In this scenario,
though, LayoutView() would still reserve the requested amount of padding, which
might goof things up.

[beaudoin, 2012/12/20 03:05:01]

Before investing too much time in this, check out my CL that I'm readying for
review over here:
https://codereview.chromium.org/11648024/

LayoutView is gone, but I think it makes adding new decorations (or shifting
them around) much easier. It may not be a bad idea to wait for this to land.

[kuan, 2012/12/20 16:51:22]

should i wait for philippe's refactoring cl to land first?

in any case, LayoutView() or philippe's new design expects only 1 padding for
the view, either on the left or the right - SearchTokenView has both, one on
left and one on right.  i can't help but feel that the right padding between end
of label and separator should be handled by SearchTokenView itself?  of course,
we can pass the padding size into its constructor so theoretically, it's still
determined in LocationBarView.

+    if (search_token_view_->visible()) {
+      search_token_view_->SetBounds(
+          location_bounds.right() - pref_width,
+          location_y, pref_width, location_height);
+      location_bounds.set_width(location_bounds.width() - pref_width);
+    } else {
+      // Put back the enlargement that we undid above.
+      location_bounds.Inset(0, 0, -kEditInternalSpace, 0);
+    }
+  }
+
   // Layout out the suggested text view right aligned to the location
   // entry. Only show the suggested text if we can fit the text from one
   // character before the end of the selection to the end of the text and the