[SiteChip] Add the basic painting of the site chip button

chrome/browser/ui/views/toolbar/toolbar_view.cc

 // Copyright 2013 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/toolbar/toolbar_view.h"
 
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
 #include "base/i18n/number_formatting.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/command_updater.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/search/search.h"
 #include "chrome/browser/themes/theme_service.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_command_controller.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_content_setting_bubble_model_delegate.h"
 #include "chrome/browser/ui/browser_instant_controller.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/global_error/global_error_service.h"
 #include "chrome/browser/ui/global_error/global_error_service_factory.h"
 #include "chrome/browser/ui/omnibox/omnibox_view.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/toolbar/wrench_menu_model.h"
 #include "chrome/browser/ui/view_ids.h"
 #include "chrome/browser/ui/views/extensions/suspicious_extension_bubble_view.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/location_bar/page_action_image_view.h"
 #include "chrome/browser/ui/views/location_bar/star_view.h"
 #include "chrome/browser/ui/views/location_bar/translate_icon_view.h"
 #include "chrome/browser/ui/views/outdated_upgrade_bubble_view.h"
 #include "chrome/browser/ui/views/toolbar/back_button.h"
 #include "chrome/browser/ui/views/toolbar/browser_actions_container.h"
 #include "chrome/browser/ui/views/toolbar/home_button.h"
 #include "chrome/browser/ui/views/toolbar/reload_button.h"
+#include "chrome/browser/ui/views/toolbar/site_chip_view.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_button.h"
 #include "chrome/browser/ui/views/toolbar/wrench_menu.h"
 #include "chrome/browser/ui/views/toolbar/wrench_toolbar_button.h"
 #include "chrome/browser/upgrade_detector.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "content/public/browser/browser_accessibility_state.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/user_metrics.h"
@@ skipping 64 matching lines @@
 
 ////////////////////////////////////////////////////////////////////////////////
 // ToolbarView, public:
 
 ToolbarView::ToolbarView(Browser* browser)
     : back_(NULL),
       forward_(NULL),
       reload_(NULL),
       home_(NULL),
       location_bar_(NULL),
+      site_chip_view_(NULL),
       browser_actions_(NULL),
       app_menu_(NULL),
       browser_(browser) {
   set_id(VIEW_ID_TOOLBAR);
 
   chrome::AddCommandObserver(browser_, IDC_BACK, this);
   chrome::AddCommandObserver(browser_, IDC_FORWARD, this);
   chrome::AddCommandObserver(browser_, IDC_RELOAD, this);
   chrome::AddCommandObserver(browser_, IDC_HOME, this);
   chrome::AddCommandObserver(browser_, IDC_LOAD_NEW_TAB_PAGE, this);
@@ skipping 86 matching lines @@
   app_menu_->SetAccessibleName(l10n_util::GetStringUTF16(IDS_ACCNAME_APP));
   app_menu_->SetTooltipText(l10n_util::GetStringUTF16(IDS_APPMENU_TOOLTIP));
   app_menu_->set_id(VIEW_ID_APP_MENU);
 
   // Always add children in order from left to right, for accessibility.
   AddChildView(back_);
   AddChildView(forward_);
   AddChildView(reload_);
   AddChildView(home_);
   AddChildView(location_bar_);
+  site_chip_view_ = new SiteChipView(this);
+  AddChildView(site_chip_view_);
   AddChildView(browser_actions_);
   AddChildView(app_menu_);
 
   LoadImages();
 
   // Add any necessary badges to the menu item based on the system state.
   // Do this after |app_menu_| has been added as a bubble may be shown that
   // needs the widget (widget found by way of app_menu_->GetWidget()).
   UpdateAppMenuState();
 
   location_bar_->Init();
+  site_chip_view_->Init();
   show_home_button_.Init(prefs::kShowHomeButton,
                          browser_->profile()->GetPrefs(),
                          base::Bind(&ToolbarView::OnShowHomeButtonChanged,
                                     base::Unretained(this)));
 
   browser_actions_->Init();
 
   // Accessibility specific tooltip text.
   if (content::BrowserAccessibilityState::GetInstance()->
           IsAccessibleBrowser()) {
     back_->SetTooltipText(
         l10n_util::GetStringUTF16(IDS_ACCNAME_TOOLTIP_BACK));
     forward_->SetTooltipText(
         l10n_util::GetStringUTF16(IDS_ACCNAME_TOOLTIP_FORWARD));
   }
 }
 
 void ToolbarView::OnWidgetVisibilityChanged(views::Widget* widget,
                                             bool visible) {
   if (visible) {
     extensions::SuspiciousExtensionBubbleView::MaybeShow(browser_, app_menu_);
     GetWidget()->RemoveObserver(this);
   }
 }
 
 void ToolbarView::Update(WebContents* tab) {
   if (location_bar_)
     location_bar_->Update(tab);
+  if (site_chip_view_)
+    site_chip_view_->Update(tab);
 
   if (browser_actions_)
     browser_actions_->RefreshBrowserActionViews();
 
   if (reload_)
     reload_->set_menu_enabled(chrome::IsDebuggerAttachedToCurrentTab(browser_));
 }
 
 void ToolbarView::SetPaneFocusAndFocusAppMenu() {
   SetPaneFocus(app_menu_);
@@ skipping 194 matching lines @@
     ui::Accelerator* accelerator) {
   return GetWidget()->GetAccelerator(command_id, accelerator);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // ToolbarView, views::View overrides:
 
 gfx::Size ToolbarView::GetPreferredSize() {
   if (is_display_mode_normal()) {
     int button_spacing = GetButtonSpacing();
+    int site_chip_spacing = kStandardSpacing - 1;

[Peter Kasting, 2013/12/03 00:58:10]

Nit: Don't calculate this value in two different places.

Why is the -1 necessary?  What about the site chip imagery makes the spacing
need to be different?

[Greg Billock, 2013/12/03 17:35:08]

This is just an artifact of how the images are produced -- the distinct visual
edge of the border is set in 2 pixels. And the spacing specified by UI is 1
pixel more than the kStandardSpacing distance.

     int min_width = kLeftEdgeSpacing +
         back_->GetPreferredSize().width() + button_spacing +
         forward_->GetPreferredSize().width() + button_spacing +
         reload_->GetPreferredSize().width() + kStandardSpacing +
         (show_home_button_.GetValue() ?
             (home_->GetPreferredSize().width() + button_spacing) : 0) +
         location_bar_->GetPreferredSize().width() +
+        (site_chip_view_->ShouldShow() ?
+            site_chip_view_->GetPreferredSize().width() +
+            2*site_chip_spacing + 2*button_spacing : 0) +

[Peter Kasting, 2013/12/03 00:58:10]

Nit: Spaces around operators

Indent this line 4 more, then wrap after ':', so it's obvious how the expression
is constructed.  I'd place parens around the entire subexpression as well (i.e.
"x ? (1 + 2 + 3) : 0").

[Greg Billock, 2013/12/03 17:35:08]

Done.

         browser_actions_->GetPreferredSize().width() +
         app_menu_->GetPreferredSize().width() + kRightEdgeSpacing;
     gfx::ImageSkia* normal_background =
         GetThemeProvider()->GetImageSkiaNamed(IDR_CONTENT_TOP_CENTER);
     return gfx::Size(min_width,
                      normal_background->height() - content_shadow_height());
   }
 
   const int kPopupBottomSpacingGlass = 1;
   const int kPopupBottomSpacingNonGlass = 2;
@@ skipping 49 matching lines @@
   if (show_home_button_.GetValue()) {
     home_->SetVisible(true);
     home_->SetBounds(reload_->x() + reload_->width() + button_spacing,
                      child_y, home_->GetPreferredSize().width(), child_height);
   } else {
     home_->SetVisible(false);
     home_->SetBounds(reload_->x() + reload_->width(), child_y, 0, child_height);
   }
 
   int browser_actions_width = browser_actions_->GetPreferredSize().width();
+
+  // Note: spacing from location bar to site chip is 1 pixel less than
+  // kStandardSpacing given the edge thickness of the chip.
+  int site_chip_spacing = kStandardSpacing - 1;
+  int site_chip_width =
+      (site_chip_view_->ShouldShow() ?
+          site_chip_view_->GetPreferredSize().width() +
+          2 * site_chip_spacing : 0);
   int app_menu_width = app_menu_->GetPreferredSize().width();
   int location_x = home_->x() + home_->width() + kStandardSpacing;
   int available_width = std::max(0, width() - kRightEdgeSpacing -
       app_menu_width - browser_actions_width - location_x);
 
+  // Cap site chip width at 1/2 the size available to the location bar.
+  site_chip_width = std::min(site_chip_width, available_width / 2);
+  available_width -= site_chip_width;
+
   int location_height = location_bar_->GetPreferredSize().height();
   int location_y = (height() - location_height + 1) / 2;
   location_bar_->SetBounds(location_x, location_y, std::max(available_width, 0),
                            location_height);
 
-  browser_actions_->SetBounds(location_bar_->x() + location_bar_->width(), 0,
+  int browser_actions_x = location_bar_->x() + location_bar_->width();

[Peter Kasting, 2013/12/03 00:58:10]

So we put the browser actions flush against the location bar, but not flush
against the site chip?  That seems a bit odd.

[Greg Billock, 2013/12/03 17:35:08]

The browser actions are borderless, so this is giving us room to draw the site
chip border around the edges of the internal controls.

+
+  if (site_chip_view_->ShouldShow()) {
+    site_chip_view_->SetBounds(location_bar_->x() + location_bar_->width() +

[Peter Kasting, 2013/12/03 00:58:10]

Nit: Replace first two terms with "browser_actions_x"

[Greg Billock, 2013/12/03 17:35:08]

Done.

+                                   site_chip_spacing,
+                               child_y,
+                               site_chip_view_->GetPreferredSize().width(),
+                               child_height);
+    browser_actions_x += site_chip_view_->GetPreferredSize().width() +

[Peter Kasting, 2013/12/03 00:58:10]

Nit: Break after "+=" instead to avoid wrapping the calculation

[Greg Billock, 2013/12/03 17:35:08]

Done.

+                         site_chip_spacing;
+  }
+
+  browser_actions_->SetBounds(browser_actions_x, 0,
                               browser_actions_width, height());
+
   // The browser actions need to do a layout explicitly, because when an
   // extension is loaded/unloaded/changed, BrowserActionContainer removes and
   // re-adds everything, regardless of whether it has a page action. For a
   // page action, browser action bounds do not change, as a result of which
   // SetBounds does not do a layout at all.
   // TODO(sidchat): Rework the above behavior so that explicit layout is not
   //                required.
   browser_actions_->Layout();
 
   // Extend the app menu to the screen's right edge in maximized mode just like
@@ skipping 222 matching lines @@
 
 void ToolbarView::OnShowHomeButtonChanged() {
   Layout();
   SchedulePaint();
 }
 
 int ToolbarView::content_shadow_height() const {
   return browser_->host_desktop_type() == chrome::HOST_DESKTOP_TYPE_ASH ?
       kContentShadowHeightAsh : kContentShadowHeight;
 }