Make access control match the superclass

This is cleanups to make access control of ToolbarButton's subclasses match
the superclass. in order to do this, reorder some functions and modify comments.
There's no intended behavior change.

Bug: 1048901
Change-Id: I2f956cb40042ab9385d1b7e9203988d7c9a377e6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2275165Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#784445}

chrome/browser/ui/views/extensions/extensions_toolbar_button.h

@@ -24,7 +24,6 @@ class ExtensionsToolbarButton : public ToolbarButton,
 
   void UpdateIcon();
 
- private:
   // ToolbarButton:
   gfx::Size CalculatePreferredSize() const override;
   gfx::Size GetMinimumSize() const override;
@@ -37,6 +36,7 @@ class ExtensionsToolbarButton : public ToolbarButton,
   // views::WidgetObserver:
   void OnWidgetDestroying(views::Widget* widget) override;
 
+ private:
   int GetIconSize() const;
 
   // A lock to keep the button pressed when a popup is visible.

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

@@ -185,18 +185,6 @@ const char* AvatarToolbarButton::GetClassName() const {
   return kAvatarToolbarButtonClassName;
 }
 
-void AvatarToolbarButton::NotifyClick(const ui::Event& event) {
-  Button::NotifyClick(event);
-  delegate_->NotifyClick();
-  // TODO(bsep): Other toolbar buttons have ToolbarView as a listener and let it
-  // call ExecuteCommandWithDisposition on their behalf. Unfortunately, it's not
-  // possible to plumb IsKeyEvent through, so this has to be a special case.
-  browser_->window()->ShowAvatarBubbleFromAvatarButton(
-      BrowserWindow::AVATAR_BUBBLE_MODE_DEFAULT,
-      signin_metrics::AccessPoint::ACCESS_POINT_AVATAR_BUBBLE_SIGN_IN,
-      event.IsKeyEvent());
-}
-
 void AvatarToolbarButton::OnMouseExited(const ui::MouseEvent& event) {
   delegate_->OnMouseExited();
   ToolbarButton::OnMouseExited(event);
@@ -218,6 +206,18 @@ void AvatarToolbarButton::OnHighlightChanged() {
   delegate_->OnHighlightChanged();
 }
 
+void AvatarToolbarButton::NotifyClick(const ui::Event& event) {
+  Button::NotifyClick(event);
+  delegate_->NotifyClick();
+  // TODO(bsep): Other toolbar buttons have ToolbarView as a listener and let it
+  // call ExecuteCommandWithDisposition on their behalf. Unfortunately, it's not
+  // possible to plumb IsKeyEvent through, so this has to be a special case.
+  browser_->window()->ShowAvatarBubbleFromAvatarButton(
+      BrowserWindow::AVATAR_BUBBLE_MODE_DEFAULT,
+      signin_metrics::AccessPoint::ACCESS_POINT_AVATAR_BUBBLE_SIGN_IN,
+      event.IsKeyEvent());
+}
+
 base::string16 AvatarToolbarButton::GetAvatarTooltipText() const {
   switch (delegate_->GetState()) {
     case State::kIncognitoProfile:

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

@@ -55,17 +55,8 @@ class AvatarToolbarButton : public ToolbarButton,
 
   void NotifyHighlightAnimationFinished();
 
-  // views::View:
-  const char* GetClassName() const override;
-
-  static const char kAvatarToolbarButtonClassName[];
-
- private:
-  FRIEND_TEST_ALL_PREFIXES(AvatarToolbarButtonTest,
-                           HighlightMeetsMinimumContrast);
-
   // ToolbarButton:
-  void NotifyClick(const ui::Event& event) override;
+  const char* GetClassName() const override;
   void OnMouseExited(const ui::MouseEvent& event) override;
   void OnBlur() override;
   void OnThemeChanged() override;
@@ -73,6 +64,16 @@ class AvatarToolbarButton : public ToolbarButton,
   // ToolbarIconContainerView::Observer:
   void OnHighlightChanged() override;
 
+  static const char kAvatarToolbarButtonClassName[];
+
+ protected:
+  // ToolbarButton:
+  void NotifyClick(const ui::Event& event) override;
+
+ private:
+  FRIEND_TEST_ALL_PREFIXES(AvatarToolbarButtonTest,
+                           HighlightMeetsMinimumContrast);
+
   base::string16 GetAvatarTooltipText() const;
   gfx::ImageSkia GetAvatarIcon(ButtonState state,
                                const gfx::Image& profile_identity_image) const;

chrome/browser/ui/views/toolbar/browser_app_menu_button.cc

@@ -251,11 +251,6 @@ void BrowserAppMenuButton::ShowMenu(int run_types) {
       browser, run_types, alert_reopen_tab_items);
 }
 
-void BrowserAppMenuButton::OnThemeChanged() {
-  AppMenuButton::OnThemeChanged();
-  UpdateIcon();
-}
-
 void BrowserAppMenuButton::UpdateIcon() {
   bool touch_ui = ui::TouchUiController::Get()->touch_ui();
   if (base::FeatureList::IsEnabled(features::kUseTextForUpdateButton)) {
@@ -275,6 +270,11 @@ void BrowserAppMenuButton::UpdateIcon() {
   }
 }
 
+void BrowserAppMenuButton::OnThemeChanged() {
+  AppMenuButton::OnThemeChanged();
+  UpdateIcon();
+}
+
 const char* BrowserAppMenuButton::GetClassName() const {
   return "BrowserAppMenuButton";
 }

chrome/browser/ui/views/toolbar/browser_app_menu_button.h

@@ -45,9 +45,6 @@ class BrowserAppMenuButton : public AppMenuButton {
   // noticeable color, and the menu item appearance may be affected.
   void SetPromoFeature(base::Optional<InProductHelpFeature> promo_feature);
 
-  // views::MenuButton:
-  void OnThemeChanged() override;
-
   // Updates the presentation according to |severity_| and the theme provider.
   void UpdateIcon();
 
@@ -55,14 +52,8 @@ class BrowserAppMenuButton : public AppMenuButton {
   // Used only in testing.
   static bool g_open_app_immediately_for_testing;
 
- protected:
-  // If the button is being used as an anchor for a promo, returns the best
-  // promo color given the current background color. Otherwise, returns the
-  // standard ToolbarButton foreground color for the given |state|.
-  SkColor GetForegroundColor(ButtonState state) const override;
-
- private:
   // AppMenuButton:
+  void OnThemeChanged() override;
   const char* GetClassName() const override;
   bool GetDropFormats(int* formats,
                       std::set<ui::ClipboardFormatType>* format_types) override;
@@ -78,6 +69,13 @@ class BrowserAppMenuButton : public AppMenuButton {
   SkColor GetInkDropBaseColor() const override;
   base::string16 GetTooltipText(const gfx::Point& p) const override;
 
+ protected:
+  // If the button is being used as an anchor for a promo, returns the best
+  // promo color given the current background color. Otherwise, returns the
+  // standard ToolbarButton foreground color for the given |state|.
+  SkColor GetForegroundColor(ButtonState state) const override;
+
+ private:
   void OnTouchUiChanged();
 
   AppMenuIconController::TypeAndSeverity type_and_severity_{

chrome/browser/ui/views/toolbar/home_button.h

@@ -17,7 +17,6 @@ class HomeButton : public ToolbarButton {
   HomeButton& operator=(const HomeButton&) = delete;
   ~HomeButton() override;
 
- private:
   // ToolbarButton:
   const char* GetClassName() const override;
   bool GetDropFormats(int* formats,
@@ -26,6 +25,7 @@ class HomeButton : public ToolbarButton {
   int OnDragUpdated(const ui::DropTargetEvent& event) override;
   int OnPerformDrop(const ui::DropTargetEvent& event) override;
 
+ private:
   Browser* const browser_;
 };