Refactor the avatar button/icon class

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

Index: chrome/browser/ui/views/profiles/avatar_menu_button.cc
diff --git a/chrome/browser/ui/views/profiles/avatar_menu_button.cc b/chrome/browser/ui/views/profiles/avatar_menu_button.cc
index 3100a621347f5472fc4cd93069aa26796d068af4..2feb3b4faa26122a6130b2b7b3375359496ee295 100644
--- a/chrome/browser/ui/views/profiles/avatar_menu_button.cc
+++ b/chrome/browser/ui/views/profiles/avatar_menu_button.cc
@@ -35,8 +35,8 @@ static inline int Round(double x) {
 const char AvatarMenuButton::kViewClassName[] = "AvatarMenuButton";
 
 AvatarMenuButton::AvatarMenuButton(Browser* browser, bool disabled)
-    : MenuButton(NULL, base::string16(), this, false),
-      browser_(browser),
+    : AvatarBaseButton(browser),
+      MenuButton(NULL, base::string16(), this, false),
       disabled_(disabled),
       is_rectangle_(false),
       old_height_(0),
@@ -46,6 +46,8 @@ AvatarMenuButton::AvatarMenuButton(Browser* browser, bool disabled)
 
   SetEventTargeter(
       scoped_ptr<views::ViewTargeter>(new views::ViewTargeter(this)));
+
+  Update();
 }
 
 AvatarMenuButton::~AvatarMenuButton() {
@@ -144,6 +146,30 @@ bool AvatarMenuButton::GetAvatarImages(Profile* profile,
   return true;
 }
 
+void AvatarMenuButton::Update() {
+  // The browser can be null in tests.
+  if (!browser())
+    return;
+
+  gfx::Image avatar;
+  gfx::Image taskbar_badge_avatar;
+  bool is_rectangle = false;
+
+  // Update the avatar button in the window frame and the taskbar overlay.
+  const bool should_show_avatar_menu = AvatarMenu::ShouldShowAvatarMenu();
+
+  if (!AvatarMenuButton::GetAvatarImages(
+          browser()->profile(), should_show_avatar_menu, &avatar,
+          &taskbar_badge_avatar, &is_rectangle)) {
+    return;

[msw, 2015/04/16 22:53:34]

I still think this shouldn't early return... If we can't load the image, then we
shouldn't call SetAvatarIcon, but regardless of that, we should call
SetEnabled(should_show_avatar_menu).

[yao, 2015/05/04 17:30:06]

Done.

+  }
+
+  // Disable the menu when we should not show the menu.

[msw, 2015/04/16 22:53:34]

Ping, I think this code should unconditionally call
SetEnabled(should_show_avatar_menu);

[yao, 2015/05/04 17:30:06]

Done.

+  if (!should_show_avatar_menu)
+    SetEnabled(false);
+  SetAvatarIcon(avatar, is_rectangle);
+}
+
 // views::ViewTargeterDelegate:
 bool AvatarMenuButton::DoesIntersectRect(const views::View* target,
                                          const gfx::Rect& rect) const {
@@ -156,5 +182,5 @@ bool AvatarMenuButton::DoesIntersectRect(const views::View* target,
 void AvatarMenuButton::OnMenuButtonClicked(views::View* source,
                                            const gfx::Point& point) {
   if (!disabled_)
-    chrome::ShowAvatarMenu(browser_);
+    chrome::ShowAvatarMenu(browser());
 }