[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/win/windows_version.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/profiles/profiles_state.h"
 #include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
 #include "chrome/browser/ui/browser.h"
 #include "components/signin/core/browser/profile_oauth2_token_service.h"
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/color_utils.h"
 #include "ui/gfx/font_list.h"
 #include "ui/gfx/text_elider.h"
 #include "ui/views/border.h"
 #include "ui/views/painter.h"
 
 namespace {
 
 // Text padding within the button border.
 const int kInset = 10;
+const int kBottomInset = 3;

[msw, 2014/05/28 19:34:58]

I'm not sure this change does the right thing. In the images you posted with the
CL description, the 'j' in "ninja" looks too low, especially on the normal
(restored) window's button (I sent you a pic in e-mail). Also, the glass shading
of the button doesn't match the other caption buttons (minimize, restore,
close). Also, the alignment of the content with that glass shading is
inconsistent between normal (restored) and maximized. Can you address these
issues (perhaps separately if they weren't changed at all by this CL?)

[noms (inactive), 2014/05/28 19:59:27]

The misalignment of the drop-down arrow and the shading seem to have been around
since the button's been around, and now i'm super sad I didn't catch them
earlier. I might as well fix them in this CL. 

On 2014/05/28 19:34:58, msw wrote:

[noms (inactive), 2014/05/30 17:43:34]

Actually, I'd rather fix them in a separate CL. That CL will be delayed by a
bit, and the auth error looks pretty terrible right now. :(

On 2014/05/28 19:59:27, Monica Dinculescu wrote:

 
 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::TextButtonDefaultBorder> border(
       new views::TextButtonDefaultBorder());
-
-  border->SetInsets(gfx::Insets(kInset, kInset, kInset, kInset));
+  int bottom_inset = is_themed_window ? kBottomInset : 0;
+  border->SetInsets(gfx::Insets(0, kInset, bottom_inset, kInset));
   border->set_normal_painter(
       views::Painter::CreateImageGridPainter(normal_image_set));
   border->set_hot_painter(
       views::Painter::CreateImageGridPainter(hot_image_set));
   border->set_pushed_painter(
       views::Painter::CreateImageGridPainter(pushed_image_set));
 
   return border.PassAs<views::Border>();
 }
 
@@ skipping 26 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.
   SigninErrorController* error =
       ProfileOAuth2TokenServiceFactory::GetForProfile(browser_->profile())->
           signin_error_controller();
   error->AddObserver(this);
   OnErrorChanged();
 
   SchedulePaint();
 }
 
 NewAvatarButton::~NewAvatarButton() {
   g_browser_process->profile_manager()->
       GetProfileInfoCache().RemoveObserver(this);
   SigninErrorController* error =
       ProfileOAuth2TokenServiceFactory::GetForProfile(browser_->profile())->
           signin_error_controller();
   error->RemoveObserver(this);
 }
 
 void NewAvatarButton::OnPaintText(gfx::Canvas* canvas, PaintButtonMode mode) {
   // Get text bounds, and then adjust for the top and RTL languages.
   gfx::Rect rect = GetTextBounds();
-  rect.Offset(0, -rect.y());
+
   if (rect.width() > 0)
     rect.set_x(GetMirroredXForRect(rect));
 
   canvas->DrawStringRectWithHalo(
       text(),
       gfx::FontList(),
       SK_ColorWHITE,
       SK_ColorDKGRAY,
       rect,
       gfx::Canvas::NO_SUBPIXEL_RENDERING);
@@ skipping 17 matching lines @@
 
 void NewAvatarButton::OnErrorChanged() {
   gfx::ImageSkia icon;
 
   // If there is an error, show an warning icon.
   SigninErrorController* error =
       ProfileOAuth2TokenServiceFactory::GetForProfile(browser_->profile())->
           signin_error_controller();
   if (error->HasError()) {
     ui::ResourceBundle* rb = &ui::ResourceBundle::GetSharedInstance();
-    icon = *rb->GetImageNamed(IDR_WARNING).ToImageSkia();
+    icon = *rb->GetImageNamed(
+        IDR_ICON_PROFILES_AVATAR_BUTTON_ERROR).ToImageSkia();

[msw, 2014/05/28 19:34:58]

nit: We have many similar warning icons:
update_failed.png
pageinfo_warning_major.png
infobar_warning.png
favicon_conflicts.png
avatar_button_auth_error.png
avatar_menu_auth_error.png
alert_small.png
about_conflicts.png

Other related warning images:
pageinfo_warning_minor.png
omnibox_https_warning.png
reset_warning.png

And other related info images:
pageinfo_info.png
notification_alert.png
input_alert_menu.png
input_alert.png
info_small.png

Any chance you could try consolidating some? (perhaps remove
avatar_button_auth_error.png and use a down-sized avatar_menu_auth_error.png).

[noms (inactive), 2014/05/28 19:59:27]

Oh my. So I was just given the new avatar_button_auth resources. This makes me
want to sit down with a designer and go through all of the resources in Chrome,
but I think that's a different issue.
However, I annotated your list with what the icons look like, to illustrate
there's at most one I can reuse.

I was originally going to resize the avatar_button_auth_error, but since I was
given the smaller resource, I thought it would be easier to just use that
(there's a Mac CL using it coming your way soon too). Would you prefer I just
resized it?

update_failed.png — yellow exclamation mark
pageinfo_warning_major.png - yellow exclamation mark
infobar_warning.png - grey warning
favicon_conflicts.png - grey warning
avatar_button_auth_error.png
avatar_menu_auth_error.png
alert_small.png - potentially good yellow warning sign, but looks less flat
about_conflicts.png - blue warning

Other related warning images:
pageinfo_warning_minor.png - lock with a warning icon
omnibox_https_warning.png - lock with a yellow warning
reset_warning.png - exclamation mark

And other related info images:
pageinfo_info.png - blue i icon
notification_alert.png - little exclamation mark in a giant yellow square
input_alert_menu.png - white exclamation mark in a yellow square
input_alert.png - same as above but a different size
info_small.png - blue i icon in a blue square
  


On 2014/05/28 19:34:58, msw wrote:

[msw, 2014/05/28 20:56:27]

If scaling down the larger icon looks okay (and isn't inefficiently scaled on
each call to Paint), then i'd do that. Yeah, the icons are a mess, I filed
crbug.com/378467: Chrome has too many distinct warning icons.

[noms (inactive), 2014/05/30 17:43:34]

Done.

   }
 
   SetIcon(icon);
   UpdateAvatarButtonAndRelayoutParent();
 }
 
 void NewAvatarButton::UpdateAvatarButtonAndRelayoutParent() {
   // We want the button to resize if the new text is shorter.
   SetText(GetElidedText(
       profiles::GetAvatarNameForProfile(browser_->profile())));
   ClearMaxTextSize();
 
   // 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();
 }