3 files changed, 147 insertions, 152 deletions

diff --git a/chrome/browser/gtk/reload_button_gtk.cc b/chrome/browser/gtk/reload_button_gtk.cc
index e5e8857..4dd278b 100644
--- a/chrome/browser/gtk/reload_button_gtk.cc
+++ b/chrome/browser/gtk/reload_button_gtk.cc
@@ -25,15 +25,15 @@ ReloadButtonGtk::ReloadButtonGtk(LocationBarViewGtk* location_bar,
                                  Browser* browser)
     : location_bar_(location_bar),
       browser_(browser),
-      button_delay_(0),
-      pretend_timer_is_running_for_unittest_(false),
       intended_mode_(MODE_RELOAD),
       visible_mode_(MODE_RELOAD),
       theme_provider_(browser ?
                       GtkThemeProvider::GetFrom(browser->profile()) : NULL),
       reload_(theme_provider_, IDR_RELOAD, IDR_RELOAD_P, IDR_RELOAD_H, 0),
       stop_(theme_provider_, IDR_STOP, IDR_STOP_P, IDR_STOP_H, IDR_STOP_D),
-      widget_(gtk_chrome_button_new()) {
+      widget_(gtk_chrome_button_new()),
+      testing_mouse_hovered_(false),
+      testing_reload_count_(0) {
   gtk_widget_set_size_request(widget(), reload_.Width(), reload_.Height());
 
   gtk_widget_set_app_paintable(widget(), TRUE);
@@ -57,6 +57,13 @@ ReloadButtonGtk::ReloadButtonGtk(LocationBarViewGtk* location_bar,
                    NotificationType::BROWSER_THEME_CHANGED,
                    Source<GtkThemeProvider>(theme_provider_));
   }
+
+  // Set the default timer delay to the system double-click time.
+  int timer_delay_ms;
+  GtkSettings* settings = gtk_settings_get_default();
+  g_object_get(G_OBJECT(settings), "gtk-double-click-time", &timer_delay_ms,
+               NULL);
+  timer_delay_ = base::TimeDelta::FromMilliseconds(timer_delay_ms);
 }
 
 ReloadButtonGtk::~ReloadButtonGtk() {
@@ -69,9 +76,9 @@ void ReloadButtonGtk::ChangeMode(Mode mode, bool force) {
   // 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 || GTK_WIDGET_STATE(widget()) == GTK_STATE_NORMAL ||
-      ((mode == MODE_STOP) ?
-        !timer_running() : (visible_mode_ != MODE_STOP))) {
+  if (force || ((GTK_WIDGET_STATE(widget()) == GTK_STATE_NORMAL) &&
+      !testing_mouse_hovered_) || ((mode == MODE_STOP) ?
+          !timer_.IsRunning() : (visible_mode_ != MODE_STOP))) {
     timer_.Stop();
     visible_mode_ = mode;
 
@@ -104,7 +111,7 @@ void ReloadButtonGtk::ChangeMode(Mode mode, bool force) {
 
 void ReloadButtonGtk::Observe(NotificationType type,
                               const NotificationSource& source,
-                              const NotificationDetails& details) {
+                              const NotificationDetails& /* details */) {
   DCHECK(NotificationType::BROWSER_THEME_CHANGED == type);
 
   GtkThemeProvider* provider = static_cast<GtkThemeProvider*>(
@@ -118,10 +125,10 @@ void ReloadButtonGtk::OnButtonTimer() {
   ChangeMode(intended_mode_, false);
 }
 
-void ReloadButtonGtk::OnClicked(GtkWidget* sender) {
+void ReloadButtonGtk::OnClicked(GtkWidget* /* sender */) {
   if (visible_mode_ == MODE_STOP) {
-    // The stop button is disabled because the user hovered over the button
-    // until the stop action is no longer selectable.
+    // Do nothing if Stop was disabled due to an attempt to change back to
+    // RELOAD mode while hovered.
     if (stop_.paint_override() == GTK_STATE_INSENSITIVE)
       return;
 
@@ -131,7 +138,7 @@ void ReloadButtonGtk::OnClicked(GtkWidget* sender) {
     // The user has clicked, so we can feel free to update the button,
     // even if the mouse is still hovering.
     ChangeMode(MODE_RELOAD, true);
-  } else if (!timer_running()) {
+  } else if (!timer_.IsRunning()) {
     // Shift-clicking or Ctrl-clicking the reload button means we should ignore
     // any cached content.
     int command;
@@ -148,31 +155,24 @@ void ReloadButtonGtk::OnClicked(GtkWidget* sender) {
 
     WindowOpenDisposition disposition =
         event_utils::DispositionFromEventFlags(modifier_state_uint);
-    if (disposition == CURRENT_TAB) {
+    if ((disposition == CURRENT_TAB) && location_bar_) {
       // Forcibly reset the location bar, since otherwise it won't discard any
       // ongoing user edits, since it doesn't realize this is a user-initiated
       // action.
       location_bar_->Revert();
     }
 
-    // Figure out the system double-click time.
-    if (button_delay_ == 0) {
-      GtkSettings* settings = gtk_settings_get_default();
-      g_object_get(G_OBJECT(settings), "gtk-double-click-time", &button_delay_,
-                   NULL);
-    }
-
     // Start a timer - while this timer is running, the reload button cannot be
     // changed to a stop button.  We do not set |intended_mode_| to MODE_STOP
     // here as the browser will do that when it actually starts loading (which
     // may happen synchronously, thus the need to do this before telling the
     // browser to execute the reload command).
     timer_.Stop();
-    timer_.Start(base::TimeDelta::FromMilliseconds(button_delay_), this,
-                 &ReloadButtonGtk::OnButtonTimer);
+    timer_.Start(timer_delay_, this, &ReloadButtonGtk::OnButtonTimer);
 
     if (browser_)
       browser_->ExecuteCommandWithDisposition(command, disposition);
+    ++testing_reload_count_;
   }
 }
 
@@ -184,16 +184,16 @@ gboolean ReloadButtonGtk::OnExpose(GtkWidget* widget,
       widget, e, hover_controller_.GetCurrentValue());
 }
 
-gboolean ReloadButtonGtk::OnLeaveNotify(GtkWidget* widget,
-                                        GdkEventCrossing* event) {
+gboolean ReloadButtonGtk::OnLeaveNotify(GtkWidget* /* widget */,
+                                        GdkEventCrossing* /* event */) {
   ChangeMode(intended_mode_, true);
   return FALSE;
 }
 
-gboolean ReloadButtonGtk::OnQueryTooltip(GtkWidget* sender,
-                                         gint x,
-                                         gint y,
-                                         gboolean keyboard_mode,
+gboolean ReloadButtonGtk::OnQueryTooltip(GtkWidget* /* sender */,
+                                         gint /* x */,
+                                         gint /* y */,
+                                         gboolean /* keyboard_mode */,
                                          GtkTooltip* tooltip) {
   // |location_bar_| can be NULL in tests.
   if (!location_bar_)
diff --git a/chrome/browser/gtk/reload_button_gtk.h b/chrome/browser/gtk/reload_button_gtk.h
index 2a152a4..b8818dc 100644
--- a/chrome/browser/gtk/reload_button_gtk.h
+++ b/chrome/browser/gtk/reload_button_gtk.h
@@ -37,48 +37,48 @@ class ReloadButtonGtk : public NotificationObserver {
   // Provide NotificationObserver implementation.
   virtual void Observe(NotificationType type,
                        const NotificationSource& source,
-                       const NotificationDetails& details);
+                       const NotificationDetails& /* details */);
 
  private:
-  friend class ReloadButtonGtkPeer;
+  friend class ReloadButtonGtkTest;
 
   CHROMEGTK_CALLBACK_0(ReloadButtonGtk, void, OnClicked);
   CHROMEGTK_CALLBACK_1(ReloadButtonGtk, gboolean, OnExpose, GdkEventExpose*);
-  CHROMEGTK_CALLBACK_1(ReloadButtonGtk, gboolean, OnLeaveNotify,
+  CHROMEGTK_CALLBACK_1(ReloadButtonGtk,
+                       gboolean,
+                       OnLeaveNotify,
                        GdkEventCrossing*);
-  CHROMEGTK_CALLBACK_4(ReloadButtonGtk, gboolean, OnQueryTooltip, gint, gint,
-                       gboolean, GtkTooltip*);
-
-  void SetToggled();
-
-  bool timer_running() const {
-    return timer_.IsRunning() || pretend_timer_is_running_for_unittest_;
-  }
+  CHROMEGTK_CALLBACK_4(ReloadButtonGtk,
+                       gboolean,
+                       OnQueryTooltip,
+                       gint,
+                       gint,
+                       gboolean,
+                       GtkTooltip*);
 
   void OnButtonTimer();
 
   void UpdateThemeButtons();
 
-  // Used to listen for theme change notifications.
-  NotificationRegistrar registrar_;
+  base::OneShotTimer<ReloadButtonGtk> timer_;
 
+  // These may be NULL when testing.
   LocationBarViewGtk* const location_bar_;
-
-  // Keep a pointer to the Browser object to execute commands on it.
   Browser* const browser_;
 
-  // Delay time to wait before allowing a mode change.  This is to prevent a
-  // mode switch while the user is double clicking.
-  int button_delay_;
-  base::OneShotTimer<ReloadButtonGtk> timer_;
-  bool pretend_timer_is_running_for_unittest_;
+  // The delay time for the double-click timer.  This is a member so that tests
+  // can modify it.
+  base::TimeDelta timer_delay_;
 
-  // The mode we should be in.
+  // The mode we should be in assuming no timers are running.
   Mode intended_mode_;
 
-  // The currently-visible mode - this may different from the intended mode.
+  // The currently-visible mode - this may differ from the intended mode.
   Mode visible_mode_;
 
+  // Used to listen for theme change notifications.
+  NotificationRegistrar registrar_;
+
   GtkThemeProvider* theme_provider_;
 
   CustomDrawButtonBase reload_;
@@ -87,6 +87,13 @@ class ReloadButtonGtk : public NotificationObserver {
 
   OwnedWidgetGtk widget_;
 
+  // TESTING ONLY
+  // True if we should pretend the button is hovered.
+  bool testing_mouse_hovered_;
+  // Increments when we would tell the browser to "reload", so
+  // test code can tell whether we did so (as there may be no |browser_|).
+  int testing_reload_count_;
+
   DISALLOW_IMPLICIT_CONSTRUCTORS(ReloadButtonGtk);
 };
 
diff --git a/chrome/browser/gtk/reload_button_gtk_unittest.cc b/chrome/browser/gtk/reload_button_gtk_unittest.cc
index 12384b6..04e01d0 100644
--- a/chrome/browser/gtk/reload_button_gtk_unittest.cc
+++ b/chrome/browser/gtk/reload_button_gtk_unittest.cc
@@ -2,137 +2,125 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/message_loop.h"
 #include "chrome/browser/gtk/reload_button_gtk.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-class ReloadButtonGtkPeer {
+class ReloadButtonGtkTest : public testing::Test {
  public:
-  explicit ReloadButtonGtkPeer(ReloadButtonGtk* reload) : reload_(reload) { }
+  ReloadButtonGtkTest();
 
-  // const accessors for internal state
-  ReloadButtonGtk::Mode intended_mode() const {
-    return reload_->intended_mode_;
-  }
-  ReloadButtonGtk::Mode visible_mode() const { return reload_->visible_mode_; }
+  void CheckState(bool enabled,
+                  ReloadButtonGtk::Mode intended_mode,
+                  ReloadButtonGtk::Mode visible_mode,
+                  bool timer_running);
 
-  // mutators for internal state
-  void SetState(GtkStateType state) {
-    gtk_widget_set_state(reload_->widget(), state);
-  }
-  void set_intended_mode(ReloadButtonGtk::Mode mode) {
-    reload_->intended_mode_ = mode;
-  }
-  void set_visible_mode(ReloadButtonGtk::Mode mode) {
-    reload_->visible_mode_ = mode;
+  // These accessors eliminate the need to declare each testcase as a friend.
+  void set_mouse_hovered(bool hovered) {
+    reload_.testing_mouse_hovered_ = hovered;
   }
-  void set_timer_running() {
-    reload_->pretend_timer_is_running_for_unittest_ = true;
-  }
-  void set_timer_stopped() {
-    reload_->pretend_timer_is_running_for_unittest_ = false;
-  }
-
-  // forwarders to private methods
-  gboolean OnLeave() {
-    return reload_->OnLeaveNotify(reload_->widget(), NULL);
-  }
-
-  void OnClicked() {
-    reload_->OnClicked(reload_->widget());
-  }
-
- private:
-  ReloadButtonGtk* const reload_;
-};
-
-namespace {
+  int reload_count() { return reload_.testing_reload_count_; }
+  void fake_mouse_leave() { reload_.OnLeaveNotify(reload_.widget(), NULL); }
 
-class ReloadButtonGtkTest : public testing::Test {
  protected:
-  ReloadButtonGtkTest() : reload_(NULL, NULL), peer_(&reload_) { }
+  // We need a message loop for the timers to post events.
+  MessageLoop loop_;
 
- protected:
   ReloadButtonGtk reload_;
-  ReloadButtonGtkPeer peer_;
 };
 
-TEST_F(ReloadButtonGtkTest, ChangeModeReload) {
-  reload_.ChangeMode(ReloadButtonGtk::MODE_RELOAD, true);
-  EXPECT_EQ(ReloadButtonGtk::MODE_RELOAD, peer_.intended_mode());
-  EXPECT_EQ(ReloadButtonGtk::MODE_RELOAD, peer_.visible_mode());
+ReloadButtonGtkTest::ReloadButtonGtkTest() : reload_(NULL, NULL) {
+  // Set the timer delay to 0 so that timers will fire as soon as we tell the
+  // message loop to run pending tasks.
+  reload_.timer_delay_ = base::TimeDelta();
 }
 
-TEST_F(ReloadButtonGtkTest, ChangeModeStop) {
-  reload_.ChangeMode(ReloadButtonGtk::MODE_STOP, true);
-  EXPECT_EQ(ReloadButtonGtk::MODE_STOP, peer_.intended_mode());
-  EXPECT_EQ(ReloadButtonGtk::MODE_STOP, peer_.visible_mode());
+void ReloadButtonGtkTest::CheckState(bool enabled,
+                                     ReloadButtonGtk::Mode intended_mode,
+                                     ReloadButtonGtk::Mode visible_mode,
+                                     bool timer_running) {
+  EXPECT_NE(enabled, reload_.stop_.paint_override() == GTK_STATE_INSENSITIVE);
+  EXPECT_EQ(intended_mode, reload_.intended_mode_);
+  EXPECT_EQ(visible_mode, reload_.visible_mode_);
+  EXPECT_EQ(timer_running, reload_.timer_.IsRunning());
 }
 
-TEST_F(ReloadButtonGtkTest, ScheduleChangeModeNormalReload) {
-  peer_.set_visible_mode(ReloadButtonGtk::MODE_STOP);
-  peer_.SetState(GTK_STATE_NORMAL);
-  reload_.ChangeMode(ReloadButtonGtk::MODE_RELOAD, false);
-  EXPECT_EQ(ReloadButtonGtk::MODE_RELOAD, peer_.intended_mode());
-  EXPECT_EQ(ReloadButtonGtk::MODE_RELOAD, peer_.visible_mode());
-}
+TEST_F(ReloadButtonGtkTest, Basic) {
+  // The stop/reload button starts in the "enabled reload" state with no timer
+  // running.
+  CheckState(true, ReloadButtonGtk::MODE_RELOAD, ReloadButtonGtk::MODE_RELOAD,
+             false);
 
-TEST_F(ReloadButtonGtkTest, ScheduleChangeModeHotReload) {
-  peer_.set_visible_mode(ReloadButtonGtk::MODE_STOP);
-  peer_.SetState(GTK_STATE_PRELIGHT);
-  reload_.ChangeMode(ReloadButtonGtk::MODE_RELOAD, false);
-  EXPECT_EQ(ReloadButtonGtk::MODE_RELOAD, peer_.intended_mode());
-  EXPECT_EQ(ReloadButtonGtk::MODE_STOP, peer_.visible_mode());
-}
+  // Press the button.  This should start the double-click timer.
+  gtk_button_clicked(GTK_BUTTON(reload_.widget()));
+  CheckState(true, ReloadButtonGtk::MODE_RELOAD, ReloadButtonGtk::MODE_RELOAD,
+             true);
 
-TEST_F(ReloadButtonGtkTest, ScheduleChangeModeNormalStop) {
-  peer_.set_visible_mode(ReloadButtonGtk::MODE_RELOAD);
-  peer_.SetState(GTK_STATE_NORMAL);
+  // Now change the mode (as if the browser had started loading the page).  This
+  // should cancel the timer since the button is not hovered.
   reload_.ChangeMode(ReloadButtonGtk::MODE_STOP, false);
-  EXPECT_EQ(ReloadButtonGtk::MODE_STOP, peer_.intended_mode());
-  EXPECT_EQ(ReloadButtonGtk::MODE_STOP, peer_.visible_mode());
+  CheckState(true, ReloadButtonGtk::MODE_STOP, ReloadButtonGtk::MODE_STOP,
+             false);
+
+  // Press the button again.  This should change back to reload.
+  gtk_button_clicked(GTK_BUTTON(reload_.widget()));
+  CheckState(true, ReloadButtonGtk::MODE_RELOAD, ReloadButtonGtk::MODE_RELOAD,
+             false);
 }
 
-TEST_F(ReloadButtonGtkTest, ScheduleChangeModeHotStop) {
-  peer_.set_visible_mode(ReloadButtonGtk::MODE_RELOAD);
-  peer_.SetState(GTK_STATE_PRELIGHT);
+TEST_F(ReloadButtonGtkTest, DoubleClickTimer) {
+  // Start by pressing the button.
+  gtk_button_clicked(GTK_BUTTON(reload_.widget()));
+
+  // Try to press the button again.  This should do nothing because the timer is
+  // running.
+  int original_reload_count = reload_count();
+  gtk_button_clicked(GTK_BUTTON(reload_.widget()));
+  CheckState(true, ReloadButtonGtk::MODE_RELOAD, ReloadButtonGtk::MODE_RELOAD,
+             true);
+  EXPECT_EQ(original_reload_count, reload_count());
+
+  // Hover the button, and change mode.  The visible mode should not change,
+  // again because the timer is running.
+  set_mouse_hovered(true);
   reload_.ChangeMode(ReloadButtonGtk::MODE_STOP, false);
-  EXPECT_EQ(ReloadButtonGtk::MODE_STOP, peer_.intended_mode());
-  EXPECT_EQ(ReloadButtonGtk::MODE_STOP, peer_.visible_mode());
+  CheckState(true, ReloadButtonGtk::MODE_STOP, ReloadButtonGtk::MODE_RELOAD,
+             true);
+
+  // Now fire the timer.  This should complete the mode change.
+  loop_.RunAllPending();
+  CheckState(true, ReloadButtonGtk::MODE_STOP, ReloadButtonGtk::MODE_STOP,
+             false);
 }
 
-TEST_F(ReloadButtonGtkTest, ScheduleChangeModeTimerHotStop) {
-  peer_.set_visible_mode(ReloadButtonGtk::MODE_RELOAD);
-  peer_.SetState(GTK_STATE_PRELIGHT);
-  peer_.set_timer_running();
+TEST_F(ReloadButtonGtkTest, DisableOnHover) {
+  // Start by pressing the button and proceeding with the mode change.
+  gtk_button_clicked(GTK_BUTTON(reload_.widget()));
   reload_.ChangeMode(ReloadButtonGtk::MODE_STOP, false);
-  peer_.set_timer_stopped();
-  EXPECT_EQ(ReloadButtonGtk::MODE_STOP, peer_.intended_mode());
-  EXPECT_EQ(ReloadButtonGtk::MODE_RELOAD, peer_.visible_mode());
-}
 
-TEST_F(ReloadButtonGtkTest, OnLeaveIntendedStop) {
-  peer_.SetState(GTK_STATE_PRELIGHT);
-  peer_.set_visible_mode(ReloadButtonGtk::MODE_RELOAD);
-  peer_.set_intended_mode(ReloadButtonGtk::MODE_STOP);
-  peer_.OnLeave();
-  EXPECT_EQ(ReloadButtonGtk::MODE_STOP, peer_.visible_mode());
-  EXPECT_EQ(ReloadButtonGtk::MODE_STOP, peer_.intended_mode());
+  // Now hover the button and change back.  This should result in a disabled
+  // stop button.
+  set_mouse_hovered(true);
+  reload_.ChangeMode(ReloadButtonGtk::MODE_RELOAD, false);
+  CheckState(false, ReloadButtonGtk::MODE_RELOAD, ReloadButtonGtk::MODE_STOP,
+             false);
+
+  // Un-hover the button, which should allow it to reset.
+  set_mouse_hovered(false);
+  fake_mouse_leave();
+  CheckState(true, ReloadButtonGtk::MODE_RELOAD, ReloadButtonGtk::MODE_RELOAD,
+             false);
 }
 
-TEST_F(ReloadButtonGtkTest, OnLeaveIntendedReload) {
-  peer_.SetState(GTK_STATE_PRELIGHT);
-  peer_.set_visible_mode(ReloadButtonGtk::MODE_STOP);
-  peer_.set_intended_mode(ReloadButtonGtk::MODE_RELOAD);
-  peer_.OnLeave();
-  EXPECT_EQ(ReloadButtonGtk::MODE_RELOAD, peer_.visible_mode());
-  EXPECT_EQ(ReloadButtonGtk::MODE_RELOAD, peer_.intended_mode());
-}
+TEST_F(ReloadButtonGtkTest, ResetOnClick) {
+  // Start by pressing the button and proceeding with the mode change.
+  gtk_button_clicked(GTK_BUTTON(reload_.widget()));
+  reload_.ChangeMode(ReloadButtonGtk::MODE_STOP, false);
 
-TEST_F(ReloadButtonGtkTest, OnClickedStop) {
-  peer_.set_visible_mode(ReloadButtonGtk::MODE_STOP);
-  peer_.OnClicked();
-  EXPECT_EQ(ReloadButtonGtk::MODE_RELOAD, peer_.visible_mode());
-  EXPECT_EQ(ReloadButtonGtk::MODE_RELOAD, peer_.intended_mode());
+  // Hover the button and click it.  This should change back to reload despite
+  // the hover, because it's a direct user action.
+  set_mouse_hovered(true);
+  gtk_button_clicked(GTK_BUTTON(reload_.widget()));
+  CheckState(true, ReloadButtonGtk::MODE_RELOAD, ReloadButtonGtk::MODE_RELOAD,
+             false);
 }
-
-}  // namespace