diff options
author | jar@chromium.org <jar@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-06-05 01:34:25 +0000 |
---|---|---|
committer | jar@chromium.org <jar@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-06-05 01:34:25 +0000 |
commit | af86029b6974eeecb13f1703f5b59f5152810549 (patch) | |
tree | f5fa92c98e6ab3a8b395fbf2937bcb649edd6d7d | |
parent | a20bc0936892ff263c48f3afebcac93af64c2348 (diff) | |
download | chromium_src-af86029b6974eeecb13f1703f5b59f5152810549.zip chromium_src-af86029b6974eeecb13f1703f5b59f5152810549.tar.gz chromium_src-af86029b6974eeecb13f1703f5b59f5152810549.tar.bz2 |
Break up unit test to avoid internal timing interactions between parts.
Hopefully this will cure teh flaky problem under Valgrind.
I also had to re-read my own code, and so I put in to minor changes to
hopefully make it incrementally easier ot read. There should be no semantic
changes caused by my coding changes, but the level of nested braces is
reduced, which makes it easier ot see what is (supposed to be) going on.
r=dank,phajdan
Review URL: http://codereview.chromium.org/42266
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@17698 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | base/watchdog.cc | 50 | ||||
-rw-r--r-- | base/watchdog_unittest.cc | 12 |
2 files changed, 33 insertions, 29 deletions
diff --git a/base/watchdog.cc b/base/watchdog.cc index 76662b2..b254577 100644 --- a/base/watchdog.cc +++ b/base/watchdog.cc @@ -90,33 +90,33 @@ void Watchdog::ThreadDelegate::ThreadMain() { if (remaining_duration.InMilliseconds() > 0) { // Spurios wake? Timer drifts? Go back to sleep for remaining time. watchdog_->condition_variable_.TimedWait(remaining_duration); - } else { - // We overslept, so this seems like a real alarm. - // Watch out for a user that stopped the debugger on a different alarm! - { - AutoLock static_lock(static_lock_); - if (last_debugged_alarm_time_ > watchdog_->start_time_) { - // False alarm: we started our clock before the debugger break (last - // alarm time). - watchdog_->start_time_ += last_debugged_alarm_delay_; - if (last_debugged_alarm_time_ > watchdog_->start_time_) - // Too many alarms must have taken place. - watchdog_->state_ = DISARMED; - continue; - } - } - watchdog_->state_ = DISARMED; // Only alarm at most once. - TimeTicks last_alarm_time = TimeTicks::Now(); - watchdog_->Alarm(); // Set a break point here to debug on alarms. - TimeDelta last_alarm_delay = TimeTicks::Now() - last_alarm_time; - if (last_alarm_delay > TimeDelta::FromMilliseconds(2)) { - // Ignore race of two alarms/breaks going off at roughly the same time. - AutoLock static_lock(static_lock_); - // This was a real debugger break. - last_debugged_alarm_time_ = last_alarm_time; - last_debugged_alarm_delay_ = last_alarm_delay; + continue; + } + // We overslept, so this seems like a real alarm. + // Watch out for a user that stopped the debugger on a different alarm! + { + AutoLock static_lock(static_lock_); + if (last_debugged_alarm_time_ > watchdog_->start_time_) { + // False alarm: we started our clock before the debugger break (last + // alarm time). + watchdog_->start_time_ += last_debugged_alarm_delay_; + if (last_debugged_alarm_time_ > watchdog_->start_time_) + // Too many alarms must have taken place. + watchdog_->state_ = DISARMED; + continue; } } + watchdog_->state_ = DISARMED; // Only alarm at most once. + TimeTicks last_alarm_time = TimeTicks::Now(); + watchdog_->Alarm(); // Set a break point here to debug on alarms. + TimeDelta last_alarm_delay = TimeTicks::Now() - last_alarm_time; + if (last_alarm_delay <= TimeDelta::FromMilliseconds(2)) + continue; + // Ignore race of two alarms/breaks going off at roughly the same time. + AutoLock static_lock(static_lock_); + // This was a real debugger break. + last_debugged_alarm_time_ = last_alarm_time; + last_debugged_alarm_delay_ = last_alarm_delay; } } diff --git a/base/watchdog_unittest.cc b/base/watchdog_unittest.cc index 1bfc09b..698bf9a 100644 --- a/base/watchdog_unittest.cc +++ b/base/watchdog_unittest.cc @@ -75,14 +75,18 @@ TEST(WatchdogTest, AlarmTest) { SPIN_FOR_TIMEDELTA_OR_UNTIL_TRUE(TimeDelta::FromMinutes(5), watchdog.alarm_counter() > 0); EXPECT_EQ(1, watchdog.alarm_counter()); +} - // Set a time greater than the timeout into the past. +// Make sure a basic alarm fires when the time has expired. +TEST(WatchdogTest, AlarmPriorTimeTest) { + WatchdogCounter watchdog(TimeDelta::TimeDelta(), "Enabled2", true); + // Set a time in the past. watchdog.ArmSomeTimeDeltaAgo(TimeDelta::FromSeconds(2)); // It should instantly go off, but certainly in less than 5 minutes. SPIN_FOR_TIMEDELTA_OR_UNTIL_TRUE(TimeDelta::FromMinutes(5), - watchdog.alarm_counter() > 1); + watchdog.alarm_counter() > 0); - EXPECT_EQ(2, watchdog.alarm_counter()); + EXPECT_EQ(1, watchdog.alarm_counter()); } // Make sure a disable alarm does nothing, even if we arm it. @@ -96,7 +100,7 @@ TEST(WatchdogTest, ConstructorDisabledTest) { // Make sure Disarming will prevent firing, even after Arming. TEST(WatchdogTest, DisarmTest) { - WatchdogCounter watchdog(TimeDelta::FromSeconds(5), "Enabled", true); + WatchdogCounter watchdog(TimeDelta::FromSeconds(5), "Enabled3", true); watchdog.Arm(); PlatformThread::Sleep(100); // Don't sleep too long watchdog.Disarm(); |