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 26 matching lines @@
 #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"
 #include "chrome/browser/ui/views/location_bar/open_pdf_in_reader_view.h"
 #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"
 #include "chrome/browser/ui/views/location_bar/zoom_bubble_view.h"
 #include "chrome/browser/ui/views/location_bar/zoom_view.h"
 #include "chrome/browser/ui/views/omnibox/omnibox_view_views.h"
 #include "chrome/browser/ui/views/omnibox/omnibox_views.h"
 #include "chrome/browser/ui/zoom/zoom_controller.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/extensions/feature_switch.h"
@@ skipping 113 matching lines @@
       disposition_(CURRENT_TAB),
       transition_(content::PageTransitionFromInt(
           content::PAGE_TRANSITION_TYPED |
           content::PAGE_TRANSITION_FROM_ADDRESS_BAR)),
       location_icon_view_(NULL),
       ev_bubble_view_(NULL),
       location_entry_view_(NULL),
       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),
       star_view_(NULL),
       web_intents_button_view_(NULL),
       action_box_button_view_(NULL),
       mode_(mode),
       show_focus_rect_(false),
       template_url_service_(NULL),
       animation_offset_(0) {
@@ skipping 67 matching lines @@
       profile_);
   AddChildView(selected_keyword_view_);
   selected_keyword_view_->SetFont(font_);
   selected_keyword_view_->SetVisible(false);
 
   keyword_hint_view_ = new KeywordHintView(profile_, this);
   AddChildView(keyword_hint_view_);
   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),
                                     kCSBubbleBackgroundImages, this,
                                     font_,
                                     GetColor(ToolbarModel::NONE, TEXT));
     content_setting_views_.push_back(content_blocked_view);
     AddChildView(content_blocked_view);
     content_blocked_view->SetVisible(false);
   }
@@ skipping 140 matching lines @@
 
   bool star_enabled = star_view_ && !model_->GetInputInProgress() &&
                       edit_bookmarks_enabled_.GetValue();
 
   command_updater_->UpdateCommandEnabled(IDC_BOOKMARK_PAGE, star_enabled);
   command_updater_->UpdateCommandEnabled(IDC_BOOKMARK_PAGE_FROM_STAR,
                                          star_enabled);
   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)
     location_entry_->Update(tab_for_state_restoring);
   OnChanged();
 }
 
 void LocationBarView::UpdateContentSettingsIcons() {
   RefreshContentSettingViews();
 
@@ skipping 458 matching lines @@
     // the added view.
     location_bounds.Inset(0, 0, kEditInternalSpace, 0);
     LayoutView(keyword_hint_view_, kItemEditPadding,
                AvailableWidth(max_edit_width), false, &location_bounds);
     if (!keyword_hint_view_->visible()) {
       // Put back the enlargement that we undid above.
       location_bounds.Inset(0, 0, -kEditInternalSpace, 0);
     }
   }
 
+  // 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
   // suggested text. If we can't it means either the suggested text is too big,
   // or the user has scrolled.
 
   // TODO(sky): We could potentially combine this with the previous step to
   // force using minimum size if necessary, but currently the chance of showing
   // keyword hints and suggested text is minimal and we're not confident this
   // is the right approach for suggested text.
@@ skipping 660 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();
 }