Refactor the avatar button/icon class

chrome/browser/ui/views/profiles/avatar_base_button.h

+// Copyright 2015 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.
+
+#ifndef CHROME_BROWSER_UI_VIEWS_PROFILES_AVATAR_BASE_BUTTON_H_
+#define CHROME_BROWSER_UI_VIEWS_PROFILES_AVATAR_BASE_BUTTON_H_
+
+#include "chrome/browser/profiles/profile_info_cache_observer.h"
+
+class Browser;
+
+// This view manages the button that sits in the top of the window frame and
+// displays the active profile's info when using multi-profiles.
+class AvatarBaseButton : public ProfileInfoCacheObserver {

[msw, 2015/04/01 01:30:16]

Two overarching questions here:
1) Don't we plan to deprecate/remove the old button soon anyway? (if so, why
bother with this? if not, why not?)
2) Will you consolidate more code here? as-is this refactoring doesn't offer
much compelling cleanup.

[yao, 2015/04/10 00:43:32]

I'm not sure, I did not know much of the history of this menu/bubble. I only
know about this bug: crbug/452524. Maybe @noms can provide more details here,
but she's moving recently, not sure if she'll see this.  

[noms (inactive), 2015/04/16 22:29:59]

1) Yes we do, sort of. The button will be gone, but it will still be used to
display the incognito badge (so the click listener code will go away, but
unfortunately the code around it won't).

2) I really hate the "avatar button" and "new avatar button" names of classes,
so i was hoping this could be a stepping stone towards what we have on the mac:
avatar base/avatar icon (the incognito badge) and avatar button (the actual
button). If they both inherit from the same base, then we would only need to
hold one avatar_button_ member in all the browser views, and it would just do
the right thing. What do you think?

+ public:
+  explicit AvatarBaseButton(Browser* browser);
+  ~AvatarBaseButton() override;
+
+ protected:
+  Browser* browser() const {return browser_;}

[msw, 2015/04/01 01:30:16]

nit: spaces inside curly braces.

[yao, 2015/04/10 00:43:32]

Done.

+
+ private:
+  // ProfileInfoCacheObserver:
+  void OnProfileAdded(const base::FilePath& profile_path) override;
+  void OnProfileWasRemoved(const base::FilePath& profile_path,
+                           const base::string16& profile_name) override;
+  void OnProfileNameChanged(const base::FilePath& profile_path,
+                            const base::string16& old_profile_name) override;
+  void OnProfileAvatarChanged(const base::FilePath& profile_path) override;
+  void OnProfileSupervisedUserIdChanged(
+      const base::FilePath& profile_path) override;
+
+  // Called when the profile info cache has changed, which means we might
+  // have to update the icon/text of the button.
+  virtual void Update();

[msw, 2015/04/01 01:30:16]

Shouldn't this be protected for subclasses overrides?

[yao, 2015/04/10 00:43:32]

Done.

+
+  Browser* browser_;
+
+  DISALLOW_COPY_AND_ASSIGN(AvatarBaseButton);
+};
+
+#endif  // CHROME_BROWSER_UI_VIEWS_PROFILES_AVATAR_BASE_BUTTON_H_