Update avatar button to MD (part 1)

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

 // 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.
 
-#ifndef CHROME_BROWSER_UI_VIEWS_PROFILES_NEW_AVATAR_BUTTON_H_
-#define CHROME_BROWSER_UI_VIEWS_PROFILES_NEW_AVATAR_BUTTON_H_
+#ifndef CHROME_BROWSER_UI_VIEWS_PROFILES_AVATAR_BUTTON_H_
+#define CHROME_BROWSER_UI_VIEWS_PROFILES_AVATAR_BUTTON_H_
 
 #include "base/macros.h"
 #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/profiles/avatar_button_style.h"
 #include "ui/views/controls/button/label_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 {
+// Base class for avatar buttons that display the active profile's name in the

[Peter Kasting, 2017/05/06 02:23:09]

Nit: If these buttons are primarily about profile names, maybe we should move
away from the term "avatar" in class/filenames entirely and just say
ProfileSwitcherButton or the like.

[emx, 2017/05/09 16:26:52]

I don't know the answer to that, but the strings "AvatarButton" and "avatar
button" are used in many places throughout the code. I think changing that would
be well outside the scope of this CL.

[Peter Kasting, 2017/05/09 17:40:29]

That's fair.  Consider whether it makes sense as a separate task.

+// caption area.
+class AvatarButton : public views::LabelButton,
+                     public AvatarButtonErrorControllerDelegate,
+                     public ProfileAttributesStorage::Observer {
  public:
-  NewAvatarButton(AvatarButtonDelegate* delegate,
-                  AvatarButtonStyle button_style,
-                  Profile* profile);
-  ~NewAvatarButton() override;
-
-  // Views::LabelButton
-  bool OnMousePressed(const ui::MouseEvent& event) override;
-  void OnMouseReleased(const ui::MouseEvent& event) override;
+  AvatarButton(views::ButtonListener* listener,
+               AvatarButtonStyle button_style,
+               Profile* profile);
+  ~AvatarButton() override;
 
   // Views

[Peter Kasting, 2017/05/06 02:23:09]

Nit: While here: Fix this to say "views::LabelButton:"

[emx, 2017/05/09 16:26:52]

Done.

   void OnGestureEvent(ui::GestureEvent* event) override;
 
+ protected:

[Peter Kasting, 2017/05/06 02:23:09]

Nit: Make all these public that were public in the base class.

[emx, 2017/05/09 16:26:53]

Done.

+  // View:

[Peter Kasting, 2017/05/06 02:23:09]

Nit: You don't subclass View directly.  Name this "views::LabelButton:"

[emx, 2017/05/09 16:26:52]

Done.

+  gfx::Size GetMinimumSize() const override;
+  gfx::Size GetPreferredSize() const override;
+  std::unique_ptr<views::InkDropHighlight> CreateInkDropHighlight()
+      const override;
+  bool ShouldUseFloodFillInkDrop() const override;
+
  private:
   friend class ProfileChooserViewExtensionsTest;
 
   // AvatarButtonErrorControllerDelegate:
   void OnAvatarErrorChanged() override;
 
   // ProfileAttributesStorage::Observer:
   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 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);

[Peter Kasting, 2017/05/06 02:23:09]

Nit: Function needs a comment, including an explanation of this param

[emx, 2017/05/09 16:26:52]

I've inlined the function instead. It was only called in one place from
Update().

 
-  AvatarButtonDelegate* delegate_;
+  // Sets generic_avatar_ to the image with the specified IDR.
+  void SetButtonAvatar(int avatar_idr);
+
   AvatarButtonErrorController error_controller_;
   Profile* profile_;
 
   // The icon displayed instead of the profile name in the local profile case.
   // Different assets are used depending on the OS version.
   gfx::ImageSkia generic_avatar_;
+  bool use_win10_native_button_;

[Peter Kasting, 2017/05/06 02:23:09]

Nit: Should be separated from the variable above by a blank line, since the
comment above doesn't apply to this member.

Add a comment here describing what this means.

[emx, 2017/05/09 16:26:53]

Done.

 
-  // 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(NewAvatarButton);
+  DISALLOW_COPY_AND_ASSIGN(AvatarButton);
 };
 
-#endif  // CHROME_BROWSER_UI_VIEWS_PROFILES_NEW_AVATAR_BUTTON_H_
+#endif  // CHROME_BROWSER_UI_VIEWS_PROFILES_AVATAR_BUTTON_H_