Fix regression I introduced where Stop/Go button would toggle state instantly instead of having protections against accidental user actions while the mouse was hovering the button.

I elected to condense ChangeMode() and ScheduleChangeMode() into one function, which as a result became pretty simple.

BUG=9843
Review URL: http://codereview.chromium.org/67156

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

12 files changed, 33 insertions, 55 deletions

diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc
index 9609a0b..e8a3e4f 100644
--- a/chrome/browser/browser.cc
+++ b/chrome/browser/browser.cc
@@ -1580,7 +1580,7 @@ void Browser::TabSelectedAt(TabContents* old_contents,
   UpdateToolbar(true);
 
   // Update stop/go state.
-  UpdateStopGoState(new_contents->is_loading());
+  UpdateStopGoState(new_contents->is_loading(), true);
 
   // Update commands to reflect current state.
   UpdateCommandsForTabState();
@@ -1832,7 +1832,7 @@ void Browser::LoadingStateChanged(TabContents* source) {
   window_->UpdateTitleBar();
 
   if (source == GetSelectedTabContents()) {
-    UpdateStopGoState(source->is_loading());
+    UpdateStopGoState(source->is_loading(), false);
     if (GetStatusBubble())
       GetStatusBubble()->SetStatus(GetSelectedTabContents()->GetStatusText());
   }
@@ -2245,8 +2245,8 @@ void Browser::UpdateCommandsForFullscreenMode(bool is_fullscreen) {
   command_updater_.UpdateCommandEnabled(IDC_ABOUT, show_main_ui);
 }
 
-void Browser::UpdateStopGoState(bool is_loading) {
-  window_->UpdateStopGoState(is_loading);
+void Browser::UpdateStopGoState(bool is_loading, bool force) {
+  window_->UpdateStopGoState(is_loading, force);
   command_updater_.UpdateCommandEnabled(IDC_GO, !is_loading);
   command_updater_.UpdateCommandEnabled(IDC_STOP, is_loading);
 }
diff --git a/chrome/browser/browser.h b/chrome/browser/browser.h
index 65dba6e..7956413 100644
--- a/chrome/browser/browser.h
+++ b/chrome/browser/browser.h
@@ -480,9 +480,10 @@ class Browser : public TabStripModelDelegate,
   // mode.
   void UpdateCommandsForFullscreenMode(bool is_fullscreen);
 
-  // Set the correct stop/go icon and update the Go and Stop command states.
-  // |is_loading| is true if the current TabContents is loading.
-  void UpdateStopGoState(bool is_loading);
+  // Ask the Stop/Go button to change its icon, and update the Go and Stop
+  // command states.  |is_loading| is true if the current TabContents is
+  // loading.  |force| is true if the button should change its icon immediately.
+  void UpdateStopGoState(bool is_loading, bool force);
 
   // UI update coalescing and handling ////////////////////////////////////////
 
diff --git a/chrome/browser/browser_window.h b/chrome/browser/browser_window.h
index e7e8e4f..440c66d 100644
--- a/chrome/browser/browser_window.h
+++ b/chrome/browser/browser_window.h
@@ -106,7 +106,7 @@ class BrowserWindow {
 
   // Informs the view whether or not a load is in progress for the current tab.
   // The view can use this notification to update the go/stop button.
-  virtual void UpdateStopGoState(bool is_loading) = 0;
+  virtual void UpdateStopGoState(bool is_loading, bool force) = 0;
 
   // Updates the toolbar with the state for the specified |contents|.
   virtual void UpdateToolbar(TabContents* contents,
diff --git a/chrome/browser/cocoa/browser_window_cocoa.h b/chrome/browser/cocoa/browser_window_cocoa.h
index b1c5cc0..67a7cc6 100644
--- a/chrome/browser/cocoa/browser_window_cocoa.h
+++ b/chrome/browser/cocoa/browser_window_cocoa.h
@@ -44,7 +44,7 @@ class BrowserWindowCocoa : public BrowserWindow {
   virtual bool IsFullscreen() const;
   virtual LocationBar* GetLocationBar() const;
   virtual void SetFocusToLocationBar();
-  virtual void UpdateStopGoState(bool is_loading);
+  virtual void UpdateStopGoState(bool is_loading, bool force);
   virtual void UpdateToolbar(TabContents* contents,
                              bool should_restore_state);
   virtual void FocusToolbar();
diff --git a/chrome/browser/cocoa/browser_window_cocoa.mm b/chrome/browser/cocoa/browser_window_cocoa.mm
index a2b2a9c..b0023ac 100644
--- a/chrome/browser/cocoa/browser_window_cocoa.mm
+++ b/chrome/browser/cocoa/browser_window_cocoa.mm
@@ -118,7 +118,7 @@ void BrowserWindowCocoa::SetFocusToLocationBar() {
   [controller_ focusLocationBar];
 }
 
-void BrowserWindowCocoa::UpdateStopGoState(bool is_loading) {
+void BrowserWindowCocoa::UpdateStopGoState(bool is_loading, bool force) {
   [controller_ setIsLoading:is_loading ? YES : NO];
 }
 
diff --git a/chrome/browser/gtk/browser_window_gtk.cc b/chrome/browser/gtk/browser_window_gtk.cc
index a700d9d..78c87b0 100644
--- a/chrome/browser/gtk/browser_window_gtk.cc
+++ b/chrome/browser/gtk/browser_window_gtk.cc
@@ -434,9 +434,9 @@ void BrowserWindowGtk::SetFocusToLocationBar() {
   GetLocationBar()->FocusLocation();
 }
 
-void BrowserWindowGtk::UpdateStopGoState(bool is_loading) {
+void BrowserWindowGtk::UpdateStopGoState(bool is_loading, bool force) {
   toolbar_->GetGoButton()->ChangeMode(
-      is_loading ? GoButtonGtk::MODE_STOP : GoButtonGtk::MODE_GO);
+      is_loading ? GoButtonGtk::MODE_STOP : GoButtonGtk::MODE_GO, force);
 }
 
 void BrowserWindowGtk::UpdateToolbar(TabContents* contents,
diff --git a/chrome/browser/gtk/browser_window_gtk.h b/chrome/browser/gtk/browser_window_gtk.h
index e7d87cf..a304328 100644
--- a/chrome/browser/gtk/browser_window_gtk.h
+++ b/chrome/browser/gtk/browser_window_gtk.h
@@ -59,7 +59,7 @@ class BrowserWindowGtk : public BrowserWindow,
   virtual bool IsFullscreen() const;
   virtual LocationBar* GetLocationBar() const;
   virtual void SetFocusToLocationBar();
-  virtual void UpdateStopGoState(bool is_loading);
+  virtual void UpdateStopGoState(bool is_loading, bool force);
   virtual void UpdateToolbar(TabContents* contents,
                              bool should_restore_state);
   virtual void FocusToolbar();
diff --git a/chrome/browser/views/frame/browser_view.cc b/chrome/browser/views/frame/browser_view.cc
index 8c6573c..ed494d6 100644
--- a/chrome/browser/views/frame/browser_view.cc
+++ b/chrome/browser/views/frame/browser_view.cc
@@ -737,9 +737,9 @@ void BrowserView::SetFocusToLocationBar() {
   }
 }
 
-void BrowserView::UpdateStopGoState(bool is_loading) {
+void BrowserView::UpdateStopGoState(bool is_loading, bool force) {
   toolbar_->GetGoButton()->ChangeMode(
-      is_loading ? GoButton::MODE_STOP : GoButton::MODE_GO);
+      is_loading ? GoButton::MODE_STOP : GoButton::MODE_GO, force);
 }
 
 void BrowserView::UpdateToolbar(TabContents* contents,
diff --git a/chrome/browser/views/frame/browser_view.h b/chrome/browser/views/frame/browser_view.h
index 5479cdc..89aad2a 100644
--- a/chrome/browser/views/frame/browser_view.h
+++ b/chrome/browser/views/frame/browser_view.h
@@ -187,7 +187,7 @@ class BrowserView : public BrowserWindow,
   virtual bool IsFullscreen() const;
   virtual LocationBar* GetLocationBar() const;
   virtual void SetFocusToLocationBar();
-  virtual void UpdateStopGoState(bool is_loading);
+  virtual void UpdateStopGoState(bool is_loading, bool force);
   virtual void UpdateToolbar(TabContents* contents, bool should_restore_state);
   virtual void FocusToolbar();
   virtual void DestroyBrowser();
diff --git a/chrome/browser/views/go_button.cc b/chrome/browser/views/go_button.cc
index f276e92..4c6de94 100644
--- a/chrome/browser/views/go_button.cc
+++ b/chrome/browser/views/go_button.cc
@@ -32,33 +32,17 @@ GoButton::~GoButton() {
   stop_timer_.RevokeAll();
 }
 
-void GoButton::ChangeMode(Mode mode) {
-  stop_timer_.RevokeAll();
-
-  SetToggled(mode == MODE_STOP);
+void GoButton::ChangeMode(Mode mode, bool force) {
   intended_mode_ = mode;
-  visible_mode_ = mode;
-}
 
-void GoButton::ScheduleChangeMode(Mode mode) {
-  if (mode == MODE_STOP) {
-    // If we still have a timer running, we can't yet change to a stop sign,
-    // so we'll queue up the change for when the timer expires or for when
-    // the mouse exits the button.
-    if (!stop_timer_.empty() && state() == BS_HOT) {
-      intended_mode_ = MODE_STOP;
-    } else {
-      ChangeMode(MODE_STOP);
-    }
-  } else {
-    // If we want to change the button to a go button, but the user's mouse
-    // is hovering, don't change the mode just yet - this prevents the
-    // stop button changing to a go under the user's mouse cursor.
-    if (visible_mode_ == MODE_STOP && state() == BS_HOT) {
-      intended_mode_ = MODE_GO;
-    } else {
-      ChangeMode(MODE_GO);
-    }
+  // If the change is forced, or the user isn't hovering the icon, or it's safe
+  // to change it to the other image type, make the change immediately;
+  // otherwise we'll let it happen later.
+  if (force || (state() != BS_HOT) || ((mode == MODE_STOP) ?
+      stop_timer_.empty() : (visible_mode_ != MODE_STOP))) {
+    stop_timer_.RevokeAll();
+    SetToggled(mode == MODE_STOP);
+    visible_mode_ = mode;
   }
 }
 
@@ -71,7 +55,7 @@ void GoButton::ButtonPressed(views::Button* button) {
 
     // The user has clicked, so we can feel free to update the button,
     // even if the mouse is still hovering.
-    ChangeMode(MODE_GO);
+    ChangeMode(MODE_GO, true);
   } else if (visible_mode_ == MODE_GO && stop_timer_.empty()) {
     // If the go button is visible and not within the double click timer, go.
     browser_->Go(event_utils::DispositionFromEventFlags(mouse_event_flags()));
@@ -98,9 +82,7 @@ void GoButton::ButtonPressed(views::Button* button) {
 // GoButton, View overrides:
 
 void GoButton::OnMouseExited(const views::MouseEvent& e) {
-  if (visible_mode_ != intended_mode_)
-    ChangeMode(intended_mode_);
-
+  ChangeMode(intended_mode_, true);
   if (state() != BS_DISABLED)
     SetState(BS_NORMAL);
 }
@@ -140,8 +122,6 @@ bool GoButton::GetTooltipText(int x, int y, std::wstring* tooltip) {
 // GoButton, private:
 
 void GoButton::OnButtonTimer() {
-  if (intended_mode_ != visible_mode_)
-    ChangeMode(intended_mode_);
-
   stop_timer_.RevokeAll();
+  ChangeMode(intended_mode_, true);
 }
diff --git a/chrome/browser/views/go_button.h b/chrome/browser/views/go_button.h
index cdd5bea..61b528e 100644
--- a/chrome/browser/views/go_button.h
+++ b/chrome/browser/views/go_button.h
@@ -32,12 +32,9 @@ class GoButton : public views::ToggleImageButton,
   GoButton(LocationBarView* location_bar, Browser* Browser);
   virtual ~GoButton();
 
-  // Force the button state
-  void ChangeMode(Mode mode);
-
-  // Ask for a specified button state. This is commonly called by the Browser
-  // when page load state changes.
-  void ScheduleChangeMode(Mode mode);
+  // Ask for a specified button state.  If |force| is true this will be applied
+  // immediately.
+  void ChangeMode(Mode mode, bool force);
 
   // Overridden from views::ButtonListener:
   virtual void ButtonPressed(views::Button* button);
diff --git a/chrome/test/test_browser_window.h b/chrome/test/test_browser_window.h
index 7a8ad17..d715210 100644
--- a/chrome/test/test_browser_window.h
+++ b/chrome/test/test_browser_window.h
@@ -42,7 +42,7 @@ class TestBrowserWindow : public BrowserWindow {
     return const_cast<TestLocationBar*>(&location_bar_);
   }
   virtual void SetFocusToLocationBar() {}
-  virtual void UpdateStopGoState(bool is_loading) {}
+  virtual void UpdateStopGoState(bool is_loading, bool force) {}
   virtual void UpdateToolbar(TabContents* contents,
                              bool should_restore_state) {}
   virtual void FocusToolbar() {}