Cleanup: Remove dead code, use early returns to reduce indenting, shorten.

This makes one behavioral change: a mouse release with a successful HitTest and |canceled| true will set the state to HOT instead of NORMAL.  This seemed more correct to me.

BUG=none
TEST=none
Review URL: http://codereview.chromium.org/3175027

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

views/controls/button/custom_button.cc

@@ -19,24 +19,25 @@ CustomButton::~CustomButton() {
 }
 
 void CustomButton::SetState(ButtonState state) {
-  if (state != state_) {
-    if (animate_on_state_change_ || !hover_animation_->is_animating()) {
-      animate_on_state_change_ = true;
-      if (state_ == BS_NORMAL && state == BS_HOT) {
-        // Button is hovered from a normal state, start hover animation.
-        hover_animation_->Show();
-      } else if (state_ == BS_HOT && state == BS_NORMAL) {
-        // Button is returning to a normal state from hover, start hover
-        // fade animation.
-        hover_animation_->Hide();
-      } else {
-        hover_animation_->Stop();
-      }
-    }
+  if (state == state_)
+    return;
 
-    state_ = state;
-    SchedulePaint();
+  if (animate_on_state_change_ || !hover_animation_->is_animating()) {
+    animate_on_state_change_ = true;
+    if (state_ == BS_NORMAL && state == BS_HOT) {
+      // Button is hovered from a normal state, start hover animation.
+      hover_animation_->Show();
+    } else if (state_ == BS_HOT && state == BS_NORMAL) {
+      // Button is returning to a normal state from hover, start hover
+      // fade animation.
+      hover_animation_->Hide();
+    } else {
+      hover_animation_->Stop();
+    }
   }
+
+  state_ = state;
+  SchedulePaint();
 }
 
 void CustomButton::StartThrobbing(int cycles_til_stop) {
@@ -54,14 +55,13 @@ void CustomButton::SetAnimationDuration(int duration) {
 bool CustomButton::GetAccessibleState(AccessibilityTypes::State* state) {
   *state = 0;
   switch (state_) {
-    case BS_NORMAL:
-      *state = 0;
     case BS_HOT:
       *state = AccessibilityTypes::STATE_HOTTRACKED;
     case BS_PUSHED:
       *state = AccessibilityTypes::STATE_PRESSED;
     case BS_DISABLED:
       *state = AccessibilityTypes::STATE_UNAVAILABLE;
+    case BS_NORMAL:
     case BS_COUNT:
       // No additional accessibility state set for this button state.
       break;
@@ -71,11 +71,8 @@ bool CustomButton::GetAccessibleState(AccessibilityTypes::State* state) {
 }
 
 void CustomButton::SetEnabled(bool enabled) {
-  if (enabled && state_ == BS_DISABLED) {
-    SetState(BS_NORMAL);
-  } else if (!enabled && state_ != BS_DISABLED) {
-    SetState(BS_DISABLED);
-  }
+  if (enabled ? (state_ == BS_DISABLED) : (state_ != BS_DISABLED))
+    SetState(enabled ? BS_NORMAL : BS_DISABLED);
 }
 
 bool CustomButton::IsEnabled() const {
@@ -107,14 +104,14 @@ bool CustomButton::IsTriggerableEvent(const MouseEvent& e) {
 // CustomButton, View overrides (protected):
 
 bool CustomButton::AcceleratorPressed(const Accelerator& accelerator) {
-  if (enabled_) {
-    SetState(BS_NORMAL);
-    KeyEvent key_event(Event::ET_KEY_RELEASED, accelerator.GetKeyCode(),
-                       accelerator.modifiers(), 0, 0);
-    NotifyClick(key_event);
-    return true;
-  }
-  return false;
+  if (!enabled_)
+    return false;
+
+  SetState(BS_NORMAL);
+  KeyEvent key_event(Event::ET_KEY_RELEASED, accelerator.GetKeyCode(),
+                     accelerator.modifiers(), 0, 0);
+  NotifyClick(key_event);
+  return true;
 }
 
 bool CustomButton::OnMousePressed(const MouseEvent& e) {
@@ -129,34 +126,29 @@ bool CustomButton::OnMousePressed(const MouseEvent& e) {
 
 bool CustomButton::OnMouseDragged(const MouseEvent& e) {
   if (state_ != BS_DISABLED) {
-    if (!HitTest(e.location()))
-      SetState(BS_NORMAL);
-    else if (ShouldEnterPushedState(e))
-      SetState(BS_PUSHED);
+    if (HitTest(e.location()))
+      SetState(ShouldEnterPushedState(e) ? BS_PUSHED : BS_HOT);
     else
-      SetState(BS_HOT);
+      SetState(BS_NORMAL);
   }
   return true;
 }
 
 void CustomButton::OnMouseReleased(const MouseEvent& e, bool canceled) {
-  if (InDrag()) {
-    // Starting a drag results in a MouseReleased, we need to ignore it.
+  // Starting a drag results in a MouseReleased, we need to ignore it.
+  if ((state_ == BS_DISABLED) || InDrag())
+    return;
+
+  if (!HitTest(e.location())) {
+    SetState(BS_NORMAL);
     return;
   }
 
-  if (state_ != BS_DISABLED) {
-    if (canceled || !HitTest(e.location())) {
-      SetState(BS_NORMAL);
-    } else {
-      SetState(BS_HOT);
-      if (IsTriggerableEvent(e)) {
-        NotifyClick(e);
-        // We may be deleted at this point (by the listener's notification
-        // handler) so no more doing anything, just return.
-        return;
-      }
-    }
+  SetState(BS_HOT);
+  if (!canceled && IsTriggerableEvent(e)) {
+    NotifyClick(e);
+    // NOTE: We may be deleted at this point (by the listener's notification
+    // handler).
   }
 }
 
@@ -166,13 +158,8 @@ void CustomButton::OnMouseEntered(const MouseEvent& e) {
 }
 
 void CustomButton::OnMouseMoved(const MouseEvent& e) {
-  if (state_ != BS_DISABLED) {
-    if (HitTest(e.location())) {
-      SetState(BS_HOT);
-    } else {
-      SetState(BS_NORMAL);
-    }
-  }
+  if (state_ != BS_DISABLED)
+    SetState(HitTest(e.location()) ? BS_HOT : BS_NORMAL);
 }
 
 void CustomButton::OnMouseExited(const MouseEvent& e) {
@@ -182,31 +169,28 @@ void CustomButton::OnMouseExited(const MouseEvent& e) {
 }
 
 bool CustomButton::OnKeyPressed(const KeyEvent& e) {
-  if (state_ != BS_DISABLED) {
-    // Space sets button state to pushed. Enter clicks the button. This matches
-    // the Windows native behavior of buttons, where Space clicks the button
-    // on KeyRelease and Enter clicks the button on KeyPressed.
-    if (e.GetKeyCode() == base::VKEY_SPACE) {
-      SetState(BS_PUSHED);
-      return true;
-    } else if  (e.GetKeyCode() == base::VKEY_RETURN) {
-      SetState(BS_NORMAL);
-      NotifyClick(e);
-      return true;
-    }
+  if (state_ == BS_DISABLED)
+    return false;
+
+  // Space sets button state to pushed. Enter clicks the button. This matches
+  // the Windows native behavior of buttons, where Space clicks the button on
+  // KeyRelease and Enter clicks the button on KeyPressed.
+  if (e.GetKeyCode() == base::VKEY_SPACE) {
+    SetState(BS_PUSHED);
+  } else if (e.GetKeyCode() == base::VKEY_RETURN) {
+    SetState(BS_NORMAL);
+    NotifyClick(e);
   }
-  return false;
+  return true;
 }
 
 bool CustomButton::OnKeyReleased(const KeyEvent& e) {
-  if (state_ != BS_DISABLED) {
-    if (e.GetKeyCode() == base::VKEY_SPACE) {
-      SetState(BS_NORMAL);
-      NotifyClick(e);
-      return true;
-    }
-  }
-  return false;
+  if ((state_ == BS_DISABLED) || (e.GetKeyCode() != base::VKEY_SPACE))
+    return false;
+
+  SetState(BS_NORMAL);
+  NotifyClick(e);
+  return true;
 }
 
 void CustomButton::OnDragDone() {
@@ -214,14 +198,14 @@ void CustomButton::OnDragDone() {
 }
 
 void CustomButton::ShowContextMenu(const gfx::Point& p, bool is_mouse_gesture) {
-  if (GetContextMenuController()) {
-    // We're about to show the context menu. Showing the context menu likely
-    // means we won't get a mouse exited and reset state. Reset it now to be
-    // sure.
-    if (state_ != BS_DISABLED)
-      SetState(BS_NORMAL);
-    View::ShowContextMenu(p, is_mouse_gesture);
-  }
+  if (!GetContextMenuController())
+    return;
+
+  // We're about to show the context menu. Showing the context menu likely means
+  // we won't get a mouse exited and reset state. Reset it now to be sure.
+  if (state_ != BS_DISABLED)
+    SetState(BS_NORMAL);
+  View::ShowContextMenu(p, is_mouse_gesture);
 }
 
 void CustomButton::ViewHierarchyChanged(bool is_add, View *parent,
@@ -258,23 +242,4 @@ bool CustomButton::ShouldEnterPushedState(const MouseEvent& e) {
   return IsTriggerableEvent(e);
 }
 
-////////////////////////////////////////////////////////////////////////////////
-// CustomButton, private:
-
-void CustomButton::SetHighlighted(bool highlighted) {
-  if (highlighted && state_ != BS_DISABLED) {
-    SetState(BS_HOT);
-  } else if (!highlighted && state_ != BS_DISABLED) {
-    SetState(BS_NORMAL);
-  }
-}
-
-bool CustomButton::IsHighlighted() const {
-  return state_ == BS_HOT;
-}
-
-bool CustomButton::IsPushed() const {
-  return state_ == BS_PUSHED;
-}
-
 }  // namespace views

views/controls/button/custom_button.h

@@ -103,13 +103,6 @@ class CustomButton : public Button,
   scoped_ptr<ThrobAnimation> hover_animation_;
 
  private:
-  // Set / test whether the button is highlighted (in the hover state).
-  void SetHighlighted(bool highlighted);
-  bool IsHighlighted() const;
-
-  // Returns whether the button is pushed.
-  bool IsPushed() const;
-
   // Should we animate when the state changes? Defaults to true, but false while
   // throbbing.
   bool animate_on_state_change_;