[Win] UI fixes for the new avatar button/bubble auth errors.

chrome/browser/ui/views/profiles/new_avatar_button.cc

 // Copyright 2014 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/profiles/new_avatar_button.h"
 
 #include "base/strings/utf_string_conversions.h"
 #include "base/win/windows_version.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile_manager.h"
@@ skipping 12 matching lines @@
 #include "ui/gfx/text_elider.h"
 #include "ui/views/border.h"
 #include "ui/views/controls/button/label_button_border.h"
 #include "ui/views/painter.h"
 
 namespace {
 
 // Text padding within the button border.
 const int kLeftRightInset = 7;
 const int kTopBottomInset = 2;
+// The button in themed windows is slightly smaller, so it needs an extra
+// bottom inset so that the text doesn't get rendered outside the borders.
+const int kThemedWindowExtraBottomInset = 2;
+// Target size of the warning icon.

[msw, 2014/06/19 04:10:18]

Remove these unused consts.

[noms (inactive), 2014/06/19 15:10:55]

Done.

+const int kAuthErrorIconWidth = 13;
+const int kAuthErrorIconHeight = 11;
 
 scoped_ptr<views::Border> CreateBorder(const int normal_image_set[],
                                        const int hot_image_set[],
-                                       const int pushed_image_set[]) {
+                                       const int pushed_image_set[],
+                                       bool is_themed_window) {
   scoped_ptr<views::LabelButtonBorder> border(
       new views::LabelButtonBorder(views::Button::STYLE_TEXTBUTTON));
-
   border->SetPainter(false, views::Button::STATE_NORMAL,
       views::Painter::CreateImageGridPainter(normal_image_set));
   border->SetPainter(false, views::Button::STATE_HOVERED,
       views::Painter::CreateImageGridPainter(hot_image_set));
   border->SetPainter(false, views::Button::STATE_PRESSED,
       views::Painter::CreateImageGridPainter(pushed_image_set));
-
-  border->set_insets(gfx::Insets(kTopBottomInset, kLeftRightInset,
-                                 kTopBottomInset, kLeftRightInset));
+  int bottom_inset = is_themed_window ?
+      kTopBottomInset + kThemedWindowExtraBottomInset : kTopBottomInset;
+  int top_inset = is_themed_window ?
+      kTopBottomInset - kThemedWindowExtraBottomInset : kTopBottomInset;
+  border->set_insets(gfx::Insets(top_inset, kLeftRightInset,

[msw, 2014/06/19 04:10:17]

Ah, I'm sorry to put you out, but can you see if this works well with my WIP CL.
I have other pressing tasks, and your help here would be much appreciated.
Please try this with https://codereview.chromium.org/343853002

[noms (inactive), 2014/06/19 15:10:55]

Patched in your CL, and your insets look good. I've added a bit more top inset,
and left the changes here, so that you have an easier merge. Also because this
pixel math is way easier than my pixel math :)

On 2014/06/19 04:10:17, msw wrote:

+                                 bottom_inset, kLeftRightInset));
 
   return border.PassAs<views::Border>();
 }
 
 base::string16 GetElidedText(const base::string16& original_text) {
   // Maximum characters the button can be before the text will get elided.
   const int kMaxCharactersToDisplay = 15;
   const gfx::FontList font_list;
   return gfx::ElideText(original_text, font_list,
                         font_list.GetExpectedTextWidth(kMaxCharactersToDisplay),
@@ skipping 29 matching lines @@
   bool is_win8 = false;
 #if defined(OS_WIN)
   is_win8 = base::win::GetVersion() >= base::win::VERSION_WIN8;
 #endif
 
   if (button_style == THEMED_BUTTON) {
     const int kNormalImageSet[] = IMAGE_GRID(IDR_AVATAR_THEMED_BUTTON_NORMAL);
     const int kHotImageSet[] = IMAGE_GRID(IDR_AVATAR_THEMED_BUTTON_HOVER);
     const int kPushedImageSet[] = IMAGE_GRID(IDR_AVATAR_THEMED_BUTTON_PRESSED);
 
-    SetBorder(CreateBorder(kNormalImageSet, kHotImageSet, kPushedImageSet));
+    SetBorder(
+        CreateBorder(kNormalImageSet, kHotImageSet, kPushedImageSet, true));
     set_menu_marker(
         rb->GetImageNamed(IDR_AVATAR_THEMED_BUTTON_DROPARROW).ToImageSkia());
   } else if (is_win8) {
     const int kNormalImageSet[] = IMAGE_GRID(IDR_AVATAR_METRO_BUTTON_NORMAL);
     const int kHotImageSet[] = IMAGE_GRID(IDR_AVATAR_METRO_BUTTON_HOVER);
     const int kPushedImageSet[] = IMAGE_GRID(IDR_AVATAR_METRO_BUTTON_PRESSED);
 
-    SetBorder(CreateBorder(kNormalImageSet, kHotImageSet, kPushedImageSet));
+    SetBorder(
+        CreateBorder(kNormalImageSet, kHotImageSet, kPushedImageSet, false));
     set_menu_marker(
         rb->GetImageNamed(IDR_AVATAR_METRO_BUTTON_DROPARROW).ToImageSkia());
   } else {
     const int kNormalImageSet[] = IMAGE_GRID(IDR_AVATAR_GLASS_BUTTON_NORMAL);
     const int kHotImageSet[] = IMAGE_GRID(IDR_AVATAR_GLASS_BUTTON_HOVER);
     const int kPushedImageSet[] = IMAGE_GRID(IDR_AVATAR_GLASS_BUTTON_PRESSED);
 
-    SetBorder(CreateBorder(kNormalImageSet, kHotImageSet, kPushedImageSet));
+    SetBorder(
+        CreateBorder(kNormalImageSet, kHotImageSet, kPushedImageSet, false));
     set_menu_marker(
         rb->GetImageNamed(IDR_AVATAR_GLASS_BUTTON_DROPARROW).ToImageSkia());
   }
 
   g_browser_process->profile_manager()->GetProfileInfoCache().AddObserver(this);
 
   // Subscribe to authentication error changes so that the avatar button
   // can update itself.  Note that guest mode profiles won't have a token
   // service.
   SigninErrorController* error =
@@ skipping 31 matching lines @@
   UpdateAvatarButtonAndRelayoutParent();
 }
 
 void NewAvatarButton::OnErrorChanged() {
   gfx::ImageSkia icon;
 
   // If there is an error, show an warning icon.
   const SigninErrorController* error =
       profiles::GetSigninErrorController(browser_->profile());
   if (error && error->HasError()) {
-    ui::ResourceBundle* rb = &ui::ResourceBundle::GetSharedInstance();
-    icon = *rb->GetImageNamed(IDR_WARNING).ToImageSkia();
+    icon = *ui::ResourceBundle::GetSharedInstance().GetImageNamed(
+        IDR_ICON_PROFILES_AVATAR_BUTTON_ERROR).ToImageSkia();

[msw, 2014/06/19 04:10:18]

Did you mean to leave this as IDR_WARNING?

[noms (inactive), 2014/06/19 15:10:55]

Nope! IDR_WARNING was a placeholder while we were waiting on the real resources
(which is the one I'm using, and was the original reason for this CL)
On 2014/06/19 04:10:18, msw wrote:

   }
 
   SetImage(views::Button::STATE_NORMAL, icon);
   UpdateAvatarButtonAndRelayoutParent();
 }
 
 void NewAvatarButton::UpdateAvatarButtonAndRelayoutParent() {
   // We want the button to resize if the new text is shorter.
   SetText(GetButtonText(browser_->profile()));
   set_min_size(gfx::Size());
   InvalidateLayout();
 
   // Because the width of the button might have changed, the parent browser
   // frame needs to recalculate the button bounds and redraw it.
   if (parent())
     parent()->Layout();
 }