Update avatar button to MD

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

Index: chrome/browser/ui/views/profiles/new_avatar_button.h
diff --git a/chrome/browser/ui/views/profiles/new_avatar_button.h b/chrome/browser/ui/views/profiles/new_avatar_button.h
index 5cbcb617359b115af693793aa9c098f77ebba766..751ebbeafc617f0f8c884de1552750614675423f 100644
--- a/chrome/browser/ui/views/profiles/new_avatar_button.h
+++ b/chrome/browser/ui/views/profiles/new_avatar_button.h
@@ -9,28 +9,37 @@
 #include "chrome/browser/profiles/profile_attributes_storage.h"
 #include "chrome/browser/ui/avatar_button_error_controller.h"
 #include "chrome/browser/ui/avatar_button_error_controller_delegate.h"
+#include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/profiles/avatar_button_style.h"
-#include "ui/views/controls/button/label_button.h"
+#include "ui/views/animation/ink_drop.h"
+#include "ui/views/controls/button/menu_button.h"
 
 class AvatarButtonDelegate;
 class Profile;
 
-// Avatar button that displays the active profile's name in the caption area.
-class NewAvatarButton : public views::LabelButton,
-                        public AvatarButtonErrorControllerDelegate,
-                        public ProfileAttributesStorage::Observer {
+class AvatarButton : public views::MenuButton,
+                     public AvatarButtonErrorControllerDelegate,
+                     public ProfileAttributesStorage::Observer {
  public:
-  NewAvatarButton(AvatarButtonDelegate* delegate,
-                  AvatarButtonStyle button_style,
-                  Profile* profile);
-  ~NewAvatarButton() override;
+  AvatarButton(AvatarButtonDelegate* delegate, Profile* profile);
+  ~AvatarButton() override;
 
-  // Views::LabelButton
-  bool OnMousePressed(const ui::MouseEvent& event) override;
-  void OnMouseReleased(const ui::MouseEvent& event) override;
+  // Update the avatar button height previously calculated by GetPreferredSize,
+  // if needed, now that the button's position is known.
+  virtual void UpdateButtonHeightForPosition(const int button_x,
+                                             int* button_height) const {}
 
-  // Views
-  void OnGestureEvent(ui::GestureEvent* event) override;
+ protected:
+  // MenuButton
+  bool IsTriggerableEvent(const ui::Event& event) override;
+
+  // Called when the profile info cache or signin/sync error has changed, which
+  // means we might have to update the icon/text of the button.
+  void Update();
+
+  void set_generic_avatar(gfx::ImageSkia generic_avatar) {

[Evan Stade, 2017/04/27 01:30:55]

const ref

[emx, 2017/04/27 16:30:58]

Done.

+    generic_avatar_ = generic_avatar;
+  }
 
  private:
   friend class ProfileChooserViewExtensionsTest;
@@ -47,9 +56,9 @@ class NewAvatarButton : public views::LabelButton,
   void OnProfileSupervisedUserIdChanged(
       const base::FilePath& profile_path) override;
 
-  // Called when the profile info cache or signin/sync error has changed, which
-  // means we might have to update the icon/text of the button.
-  void Update();
+  void UpdateButton(bool use_generic_button,
+                    Profile* profile,

[Evan Stade, 2017/04/27 01:30:55]

why does this take a profile param instead of using profile_? ditto
error_controller_

[emx, 2017/04/27 16:30:58]

Mistakenly left over from an earlier implementation. Fixed.

+                    AvatarButtonErrorController* error_controller);
 
   AvatarButtonDelegate* delegate_;
   AvatarButtonErrorController error_controller_;
@@ -59,12 +68,54 @@ class NewAvatarButton : public views::LabelButton,
   // Different assets are used depending on the OS version.
   gfx::ImageSkia generic_avatar_;
 
-  // This is used to check if the bubble was showing during the mouse pressed
-  // event. If this is true then the mouse released event is ignored to prevent
-  // the bubble from reshowing.
-  bool suppress_mouse_released_action_;
+  DISALLOW_COPY_AND_ASSIGN(AvatarButton);
+};
+
+// Avatar button that displays the active profile's name in the caption area.
+class NewAvatarButton : public AvatarButton {

[Evan Stade, 2017/04/27 01:30:55]

This should be called something like ThemedAvatarButton, and I believe it
deserves its own file.

(Note that I am also making changes to this class, i.e. avatar buttons for
opaque browser frame, and both this and the class below will follow the general
principles of MD, so "new" and "md" are not really apt descriptors.)

[emx, 2017/04/27 16:30:58]

This isn't only used for themes, though, it's also used on platforms other than
Windows 10. What about "DefaultAvatarButton"? Any other ideas for a name?

[Evan Stade, 2017/04/27 17:03:30]

You're right, it's always used on Linux. But using "theme" in the name matches
the naming convention of AvatarButtonStyle. The only other name that makes sense
I guess is "opaque" something, because it's used exclusively by the
OpaqueBrowserFrame, except that is also kinda weird because it's not necessarily
actually opaque. So I'd stick with "themed" for lack of a better word, unless
you can come up with a better word and want to also rename the AvatarButtonStyle
values.

+ public:
+  NewAvatarButton(AvatarButtonDelegate* delegate,
+                  AvatarButtonStyle button_style,
+                  Profile* profile);
+  ~NewAvatarButton() override;
+
+ protected:
+  gfx::Size GetPreferredSize() const override;
+
+ private:
+  std::unique_ptr<views::Border> CreateBorder(
+      const int normal_image_set[],
+      const int hot_image_set[],
+      const int pushed_image_set[]) const;
+  // Sets generic_avatar_ to the image with the specified IDR.
+  void SetButtonAvatar(const int avatar_idr);

[Evan Stade, 2017/04/27 01:30:55]

nit: no const on POD parameters

[emx, 2017/04/27 16:30:59]

Done.

 
   DISALLOW_COPY_AND_ASSIGN(NewAvatarButton);
 };
 
+class MaterialDesignAvatarButton : public AvatarButton {

[Evan Stade, 2017/04/27 01:30:55]

This should be called something like Win10NativeAvatarButton rather than
MaterialDesignAvatarButton, and I think it deserves its own file.

[emx, 2017/04/27 16:30:59]

"Win10NativeAvatarButton" sounds good to me. I'll change this together with the
other class, once we agree on the name.

+ public:
+  MaterialDesignAvatarButton(AvatarButtonDelegate* delegate,
+                             Profile* profile,
+                             BrowserView* browser_view);
+  ~MaterialDesignAvatarButton() override;
+
+  void UpdateButtonHeightForPosition(const int button_x,
+                                     int* button_height) const override;
+
+ protected:
+  gfx::Size GetPreferredSize() const override;
+  std::unique_ptr<views::InkDrop> CreateInkDrop() override;
+  std::unique_ptr<views::InkDropRipple> CreateInkDropRipple() const override;
+  std::unique_ptr<views::InkDropHighlight> CreateInkDropHighlight()
+      const override;
+
+ private:
+  std::unique_ptr<views::Border> CreateBorder() const;
+
+  BrowserView* browser_view_;
+
+  DISALLOW_COPY_AND_ASSIGN(MaterialDesignAvatarButton);
+};
+
 #endif  // CHROME_BROWSER_UI_VIEWS_PROFILES_NEW_AVATAR_BUTTON_H_