Misc. cleanup:

* Scope constants/variables as narrowly as possible
* Don't override virtual methods unnecessarily
* Minor comment improvements
* Don't inline not-normally-inlined methods in non-test code
* Use {} where required
* Eliminate leftover forward-decl in MaximizeBubbleController (oops)
* Fix indenting & wrapping

BUG=none
TEST=none
R=sky@chromium.org

Review URL: https://codereview.chromium.org/80043002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@236628 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 99 insertions, 107 deletions

diff --git a/ash/system/tray/tray_popup_label_button_border.cc b/ash/system/tray/tray_popup_label_button_border.cc
index 275457d..d5be6bc 100644
--- a/ash/system/tray/tray_popup_label_button_border.cc
+++ b/ash/system/tray/tray_popup_label_button_border.cc
@@ -15,51 +15,50 @@
 namespace ash {
 namespace internal {
 
-namespace {
-
-const int kTrayPopupLabelButtonPaddingHorizontal = 16;
-const int kTrayPopupLabelButtonPaddingVertical = 8;
-
-const int kTrayPopupLabelButtonBorderImagesNormal[] = {
-    IDR_AURA_TRAY_POPUP_LABEL_BUTTON_BORDER,
-    IDR_AURA_TRAY_POPUP_LABEL_BUTTON_NORMAL_BACKGROUND,
-    IDR_AURA_TRAY_POPUP_LABEL_BUTTON_NORMAL_BACKGROUND,
-    IDR_AURA_TRAY_POPUP_LABEL_BUTTON_BORDER,
-    IDR_AURA_TRAY_POPUP_LABEL_BUTTON_NORMAL_BACKGROUND,
-    IDR_AURA_TRAY_POPUP_LABEL_BUTTON_NORMAL_BACKGROUND,
-    IDR_AURA_TRAY_POPUP_LABEL_BUTTON_BORDER,
-    IDR_AURA_TRAY_POPUP_LABEL_BUTTON_NORMAL_BACKGROUND,
-    IDR_AURA_TRAY_POPUP_LABEL_BUTTON_NORMAL_BACKGROUND,
-};
-
-const int kTrayPopupLabelButtonBorderImagesHovered[] = {
-    IDR_AURA_TRAY_POPUP_LABEL_BUTTON_BORDER,
-    IDR_AURA_TRAY_POPUP_LABEL_BUTTON_BORDER,
-    IDR_AURA_TRAY_POPUP_LABEL_BUTTON_BORDER,
-    IDR_AURA_TRAY_POPUP_LABEL_BUTTON_BORDER,
-    IDR_AURA_TRAY_POPUP_LABEL_BUTTON_HOVER_BACKGROUND,
-    IDR_AURA_TRAY_POPUP_LABEL_BUTTON_BORDER,
-    IDR_AURA_TRAY_POPUP_LABEL_BUTTON_BORDER,
-    IDR_AURA_TRAY_POPUP_LABEL_BUTTON_BORDER,
-    IDR_AURA_TRAY_POPUP_LABEL_BUTTON_BORDER,
-};
-
-}  // namespace
-
 TrayPopupLabelButtonBorder::TrayPopupLabelButtonBorder()
     : LabelButtonBorder(views::Button::STYLE_TEXTBUTTON) {
+  const int kTrayPopupLabelButtonBorderImagesNormal[] = {
+      IDR_AURA_TRAY_POPUP_LABEL_BUTTON_BORDER,
+      IDR_AURA_TRAY_POPUP_LABEL_BUTTON_NORMAL_BACKGROUND,
+      IDR_AURA_TRAY_POPUP_LABEL_BUTTON_NORMAL_BACKGROUND,
+      IDR_AURA_TRAY_POPUP_LABEL_BUTTON_BORDER,
+      IDR_AURA_TRAY_POPUP_LABEL_BUTTON_NORMAL_BACKGROUND,
+      IDR_AURA_TRAY_POPUP_LABEL_BUTTON_NORMAL_BACKGROUND,
+      IDR_AURA_TRAY_POPUP_LABEL_BUTTON_BORDER,
+      IDR_AURA_TRAY_POPUP_LABEL_BUTTON_NORMAL_BACKGROUND,
+      IDR_AURA_TRAY_POPUP_LABEL_BUTTON_NORMAL_BACKGROUND,
+  };
   SetPainter(false, views::Button::STATE_NORMAL,
              views::Painter::CreateImageGridPainter(
                  kTrayPopupLabelButtonBorderImagesNormal));
   SetPainter(false, views::Button::STATE_DISABLED,
              views::Painter::CreateImageGridPainter(
                  kTrayPopupLabelButtonBorderImagesNormal));
+
+  const int kTrayPopupLabelButtonBorderImagesHovered[] = {
+      IDR_AURA_TRAY_POPUP_LABEL_BUTTON_BORDER,
+      IDR_AURA_TRAY_POPUP_LABEL_BUTTON_BORDER,
+      IDR_AURA_TRAY_POPUP_LABEL_BUTTON_BORDER,
+      IDR_AURA_TRAY_POPUP_LABEL_BUTTON_BORDER,
+      IDR_AURA_TRAY_POPUP_LABEL_BUTTON_HOVER_BACKGROUND,
+      IDR_AURA_TRAY_POPUP_LABEL_BUTTON_BORDER,
+      IDR_AURA_TRAY_POPUP_LABEL_BUTTON_BORDER,
+      IDR_AURA_TRAY_POPUP_LABEL_BUTTON_BORDER,
+      IDR_AURA_TRAY_POPUP_LABEL_BUTTON_BORDER,
+  };
   SetPainter(false, views::Button::STATE_HOVERED,
              views::Painter::CreateImageGridPainter(
                  kTrayPopupLabelButtonBorderImagesHovered));
   SetPainter(false, views::Button::STATE_PRESSED,
              views::Painter::CreateImageGridPainter(
                  kTrayPopupLabelButtonBorderImagesHovered));
+
+  const int kTrayPopupLabelButtonPaddingHorizontal = 16;
+  const int kTrayPopupLabelButtonPaddingVertical = 8;
+  set_insets(gfx::Insets(kTrayPopupLabelButtonPaddingVertical,
+                         kTrayPopupLabelButtonPaddingHorizontal,
+                         kTrayPopupLabelButtonPaddingVertical,
+                         kTrayPopupLabelButtonPaddingHorizontal));
 }
 
 TrayPopupLabelButtonBorder::~TrayPopupLabelButtonBorder() {}
@@ -98,12 +97,5 @@ void TrayPopupLabelButtonBorder::Paint(const views::View& view,
   }
 }
 
-gfx::Insets TrayPopupLabelButtonBorder::GetInsets() const {
-  return gfx::Insets(kTrayPopupLabelButtonPaddingVertical,
-                     kTrayPopupLabelButtonPaddingHorizontal,
-                     kTrayPopupLabelButtonPaddingVertical,
-                     kTrayPopupLabelButtonPaddingHorizontal);
-}
-
 }  // namespace internal
 }  // namespace ash
diff --git a/ash/system/tray/tray_popup_label_button_border.h b/ash/system/tray/tray_popup_label_button_border.h
index abfe793..04e5c73 100644
--- a/ash/system/tray/tray_popup_label_button_border.h
+++ b/ash/system/tray/tray_popup_label_button_border.h
@@ -19,9 +19,8 @@ class TrayPopupLabelButtonBorder : public views::LabelButtonBorder {
   TrayPopupLabelButtonBorder();
   virtual ~TrayPopupLabelButtonBorder();
 
-  // Overridden from views::LabelButtonBorder.
+  // views::LabelButtonBorder:
   virtual void Paint(const views::View& view, gfx::Canvas* canvas) OVERRIDE;
-  virtual gfx::Insets GetInsets() const OVERRIDE;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(TrayPopupLabelButtonBorder);
diff --git a/ash/wm/caption_buttons/bubble_contents_button_row.cc b/ash/wm/caption_buttons/bubble_contents_button_row.cc
index 2211d03..6eb9062 100644
--- a/ash/wm/caption_buttons/bubble_contents_button_row.cc
+++ b/ash/wm/caption_buttons/bubble_contents_button_row.cc
@@ -17,17 +17,17 @@ namespace ash {
 // BubbleDialogButton ---------------------------------------------------------
 
 // The image button gets overridden to be able to capture mouse hover events.
-// The constructor also assigns all button states and
+// The constructor also assigns all button states and adds |this| as a child of
+// |button_row|.
 class BubbleDialogButton : public views::ImageButton {
  public:
-  explicit BubbleDialogButton(
-      BubbleContentsButtonRow* button_row_listener,
-      int normal_image,
-      int hovered_image,
-      int pressed_image);
-  virtual ~BubbleDialogButton() {}
-
-  // CustomButton overrides:
+  explicit BubbleDialogButton(BubbleContentsButtonRow* button_row,
+                              int normal_image,
+                              int hovered_image,
+                              int pressed_image);
+  virtual ~BubbleDialogButton();
+
+  // views::ImageButton:
   virtual void OnMouseCaptureLost() OVERRIDE;
   virtual void OnMouseEntered(const ui::MouseEvent& event) OVERRIDE;
   virtual void OnMouseExited(const ui::MouseEvent& event) OVERRIDE;
@@ -57,6 +57,9 @@ BubbleDialogButton::BubbleDialogButton(
   button_row->AddChildView(this);
 }
 
+BubbleDialogButton::~BubbleDialogButton() {
+}
+
 void BubbleDialogButton::OnMouseCaptureLost() {
   button_row_->ButtonHovered(NULL);
   views::ImageButton::OnMouseCaptureLost();
@@ -114,43 +117,45 @@ BubbleContentsButtonRow::BubbleContentsButtonRow(
   }
 }
 
-// Overridden from ButtonListener.
+BubbleContentsButtonRow::~BubbleContentsButtonRow() {
+}
+
 void BubbleContentsButtonRow::ButtonPressed(views::Button* sender,
                                             const ui::Event& event) {
   // While shutting down, the connection to the owner might already be broken.
   if (!bubble_->controller())
     return;
-  if (sender == left_button_)
+  if (sender == left_button_) {
     bubble_->controller()->OnButtonClicked(
-        bubble_->controller()->maximize_type() == FRAME_STATE_SNAP_LEFT ?
+        (bubble_->controller()->maximize_type() == FRAME_STATE_SNAP_LEFT) ?
             SNAP_RESTORE : SNAP_LEFT);
-  else if (sender == minimize_button_)
+  } else if (sender == minimize_button_) {
     bubble_->controller()->OnButtonClicked(SNAP_MINIMIZE);
-  else if (sender == right_button_)
+  } else {
+    DCHECK(sender == right_button_);
     bubble_->controller()->OnButtonClicked(
-        bubble_->controller()->maximize_type() == FRAME_STATE_SNAP_RIGHT ?
+        (bubble_->controller()->maximize_type() == FRAME_STATE_SNAP_RIGHT) ?
             SNAP_RESTORE : SNAP_RIGHT);
-  else
-    NOTREACHED() << "Unknown button pressed.";
+  }
 }
 
-// Called from BubbleDialogButton.
 void BubbleContentsButtonRow::ButtonHovered(BubbleDialogButton* sender) {
   // While shutting down, the connection to the owner might already be broken.
   if (!bubble_->controller())
     return;
-  if (sender == left_button_)
+  if (sender == left_button_) {
     bubble_->controller()->OnButtonHover(
-        bubble_->controller()->maximize_type() == FRAME_STATE_SNAP_LEFT ?
+        (bubble_->controller()->maximize_type() == FRAME_STATE_SNAP_LEFT) ?
             SNAP_RESTORE : SNAP_LEFT);
-  else if (sender == minimize_button_)
+  } else if (sender == minimize_button_) {
     bubble_->controller()->OnButtonHover(SNAP_MINIMIZE);
-  else if (sender == right_button_)
+  } else if (sender == right_button_) {
     bubble_->controller()->OnButtonHover(
-        bubble_->controller()->maximize_type() == FRAME_STATE_SNAP_RIGHT ?
+        (bubble_->controller()->maximize_type() == FRAME_STATE_SNAP_RIGHT) ?
             SNAP_RESTORE : SNAP_RIGHT);
-  else
+  } else {
     bubble_->controller()->OnButtonHover(SNAP_NONE);
+  }
 }
 
 views::CustomButton* BubbleContentsButtonRow::GetButtonForUnitTest(
diff --git a/ash/wm/caption_buttons/bubble_contents_button_row.h b/ash/wm/caption_buttons/bubble_contents_button_row.h
index 5fb00ab..5f6e8d1 100644
--- a/ash/wm/caption_buttons/bubble_contents_button_row.h
+++ b/ash/wm/caption_buttons/bubble_contents_button_row.h
@@ -17,17 +17,17 @@ namespace ash {
 class BubbleDialogButton;
 class MaximizeBubbleControllerBubble;
 
-// A class that creates all buttons and put them into a view.
+// A class that creates all buttons and puts them into a view.
 class BubbleContentsButtonRow : public views::View,
                                 public views::ButtonListener {
  public:
   explicit BubbleContentsButtonRow(MaximizeBubbleControllerBubble* bubble);
+  virtual ~BubbleContentsButtonRow();
 
-  virtual ~BubbleContentsButtonRow() {}
-
-  // Overridden from ButtonListener.
+  // views::ButtonListener:
   virtual void ButtonPressed(views::Button* sender,
                              const ui::Event& event) OVERRIDE;
+
   // Called from BubbleDialogButton.
   void ButtonHovered(BubbleDialogButton* sender);
 
diff --git a/ash/wm/caption_buttons/maximize_bubble_controller.h b/ash/wm/caption_buttons/maximize_bubble_controller.h
index 8fda05e..9a842e5 100644
--- a/ash/wm/caption_buttons/maximize_bubble_controller.h
+++ b/ash/wm/caption_buttons/maximize_bubble_controller.h
@@ -30,8 +30,6 @@ class MaximizeBubbleControllerBubble;
 // A class which shows a helper UI for the maximize button after a delay.
 class ASH_EXPORT MaximizeBubbleController {
  public:
-  class Bubble;
-
   MaximizeBubbleController(FrameMaximizeButton* frame_maximize_button,
                            MaximizeBubbleFrameState maximize_type,
                            int appearance_delay_ms);
diff --git a/ash/wm/caption_buttons/maximize_bubble_controller_bubble.cc b/ash/wm/caption_buttons/maximize_bubble_controller_bubble.cc
index c598434..205c21a 100644
--- a/ash/wm/caption_buttons/maximize_bubble_controller_bubble.cc
+++ b/ash/wm/caption_buttons/maximize_bubble_controller_bubble.cc
@@ -20,27 +20,6 @@
 #include "ui/views/mouse_watcher.h"
 
 
-namespace {
-
-// The text color within the bubble.
-const SkColor kBubbleTextColor = SK_ColorWHITE;
-
-// The line width of the bubble.
-const int kLineWidth = 1;
-
-// The spacing for the top and bottom of the info label.
-const int kLabelSpacing = 4;
-
-// The pixel dimensions of the arrow.
-const int kArrowHeight = 10;
-const int kArrowWidth = 20;
-
-// The animation offset in y for the bubble when appearing.
-const int kBubbleAnimationOffsetY = 5;
-
-}  // namespace
-
-
 namespace ash {
 
 // BubbleContentsView ---------------------------------------------------------
@@ -50,8 +29,7 @@ class BubbleContentsView : public views::View {
  public:
   BubbleContentsView(MaximizeBubbleControllerBubble* bubble,
                      SnapType initial_snap_type);
-
-  virtual ~BubbleContentsView() {}
+  virtual ~BubbleContentsView();
 
   // Set the label content to reflect the currently selected |snap_type|.
   // This function can be executed through the frame maximize button as well as
@@ -60,9 +38,7 @@ class BubbleContentsView : public views::View {
 
   // Added for unit test: Retrieve the button for an action.
   // |state| can be either SNAP_LEFT, SNAP_RIGHT or SNAP_MINIMIZE.
-  views::CustomButton* GetButtonForUnitTest(SnapType state) {
-    return buttons_view_->GetButtonForUnitTest(state);
-  }
+  views::CustomButton* GetButtonForUnitTest(SnapType state);
 
  private:
   // The owning class.
@@ -96,12 +72,17 @@ BubbleContentsView::BubbleContentsView(
   SetSnapType(initial_snap_type);
   label_view_->SetBackgroundColor(
       MaximizeBubbleControllerBubble::kBubbleBackgroundColor);
+  const SkColor kBubbleTextColor = SK_ColorWHITE;
   label_view_->SetEnabledColor(kBubbleTextColor);
+  const int kLabelSpacing = 4;
   label_view_->set_border(views::Border::CreateEmptyBorder(
       kLabelSpacing, 0, kLabelSpacing, 0));
   AddChildView(label_view_);
 }
 
+BubbleContentsView::~BubbleContentsView() {
+}
+
 // Set the label content to reflect the currently selected |snap_type|.
 // This function can be executed through the frame maximize button as well as
 // through hover operations.
@@ -138,9 +119,21 @@ void BubbleContentsView::SetSnapType(SnapType snap_type) {
   label_view_->SetText(rb.GetLocalizedString(id));
 }
 
+views::CustomButton* BubbleContentsView::GetButtonForUnitTest(SnapType state) {
+  return buttons_view_->GetButtonForUnitTest(state);
+}
+
 
 // MaximizeBubbleBorder -------------------------------------------------------
 
+namespace {
+
+const int kLineWidth = 1;
+const int kArrowHeight = 10;
+const int kArrowWidth = 20;
+
+}  // namespace
+
 class MaximizeBubbleBorder : public views::BubbleBorder {
  public:
   MaximizeBubbleBorder(views::View* content_view, views::View* anchor);
@@ -150,11 +143,9 @@ class MaximizeBubbleBorder : public views::BubbleBorder {
   // Get the mouse active area of the window.
   void GetMask(gfx::Path* mask);
 
-  // Overridden from views::BubbleBorder to match the design specs.
+  // views::BubbleBorder:
   virtual gfx::Rect GetBounds(const gfx::Rect& position_relative_to,
                               const gfx::Size& contents_size) const OVERRIDE;
-
-  // Overridden from views::Border.
   virtual void Paint(const views::View& view, gfx::Canvas* canvas) OVERRIDE;
 
  private:
@@ -205,7 +196,7 @@ gfx::Rect MaximizeBubbleBorder::GetBounds(
   border_size.Enlarge(insets.width(), insets.height());
 
   // Position the bubble to center the box on the anchor.
-  int x = (-border_size.width() + anchor_size_.width()) / 2;
+  int x = (anchor_size_.width() - border_size.width()) / 2;
   // Position the bubble under the anchor, overlapping the arrow with it.
   int y = anchor_size_.height() - insets.top();
 
@@ -272,11 +263,10 @@ void MaximizeBubbleBorder::Paint(const views::View& view, gfx::Canvas* canvas) {
 // Note: This object gets destroyed when the MouseWatcher gets destroyed.
 class BubbleMouseWatcherHost: public views::MouseWatcherHost {
  public:
-  explicit BubbleMouseWatcherHost(MaximizeBubbleControllerBubble* bubble)
-      : bubble_(bubble) {}
-  virtual ~BubbleMouseWatcherHost() {}
+  explicit BubbleMouseWatcherHost(MaximizeBubbleControllerBubble* bubble);
+  virtual ~BubbleMouseWatcherHost();
 
-  // Implementation of MouseWatcherHost.
+  // views::MouseWatcherHost:
   virtual bool Contains(const gfx::Point& screen_point,
                         views::MouseWatcherHost::MouseEventType type) OVERRIDE;
  private:
@@ -285,6 +275,14 @@ class BubbleMouseWatcherHost: public views::MouseWatcherHost {
   DISALLOW_COPY_AND_ASSIGN(BubbleMouseWatcherHost);
 };
 
+BubbleMouseWatcherHost::BubbleMouseWatcherHost(
+    MaximizeBubbleControllerBubble* bubble)
+    : bubble_(bubble) {
+}
+
+BubbleMouseWatcherHost::~BubbleMouseWatcherHost() {
+}
+
 bool BubbleMouseWatcherHost::Contains(
     const gfx::Point& screen_point,
     views::MouseWatcherHost::MouseEventType type) {
@@ -387,6 +385,7 @@ void MaximizeBubbleControllerBubble::AnimationProgressed(
   if (!shutting_down_)
     return;
   // Upon fade out an additional shift is required.
+  const int kBubbleAnimationOffsetY = 5;
   int shift = animation->CurrentValueBetween(kBubbleAnimationOffsetY, 0);
   gfx::Rect rect = initial_position_;
 
@@ -418,7 +417,7 @@ void MaximizeBubbleControllerBubble::MouseMovedOutOfHost() {
     gfx::Point screen_location = Shell::GetScreen()->GetCursorScreenPoint();
     if (!owner_->frame_maximize_button()->GetBoundsInScreen().Contains(
         screen_location)) {
-        owner_->RequestDestructionThroughOwner();
+      owner_->RequestDestructionThroughOwner();
     }
   }
 }
@@ -448,8 +447,7 @@ gfx::Size MaximizeBubbleControllerBubble::GetPreferredSize() {
   return contents_view_->GetPreferredSize();
 }
 
-void MaximizeBubbleControllerBubble::OnWidgetDestroying(
-    views::Widget* widget) {
+void MaximizeBubbleControllerBubble::OnWidgetDestroying(views::Widget* widget) {
   if (bubble_widget_ == widget) {
     mouse_watcher_->Stop();