Fix GetChannel interface misuse in ThreadWatcher and ShutdownWatcher.

chrome/browser/metrics/thread_watcher.cc

Index: chrome/browser/metrics/thread_watcher.cc
===================================================================
--- chrome/browser/metrics/thread_watcher.cc	(revision 100589)
+++ chrome/browser/metrics/thread_watcher.cc	(working copy)
@@ -423,11 +423,14 @@
   // Determine |unresponsive_threshold| based on switches::kCrashOnHangSeconds.
   *unresponsive_threshold = kUnresponsiveCount;
 
-  if (chrome::VersionInfo::GetChannel() == chrome::VersionInfo::CHANNEL_BETA) {
-    // Increase the unresponsive_threshold in Beta channel to reduce the number
-    // of crashes due to ThreadWatcher.
+  chrome::VersionInfo::Channel channel = chrome::VersionInfo::GetChannel();
+  if (channel == chrome::VersionInfo::CHANNEL_STABLE ||

[jar (doing other things), 2011/09/10 19:20:31]

I agree. This addition makes the code less fragile.

If you want to break this out, I'd suggest that the stable channel should have a
higher multiplier (by default) than the beta channel.  Perhaps at least a factor
of 2 yet again larger.

+      channel == chrome::VersionInfo::CHANNEL_BETA) {
+    // Increase the unresponsive_threshold in Stable and Beta channels to
+    // reduce the number of crashes due to ThreadWatcher.
     *unresponsive_threshold *= 2;

[Mark Mentovai, 2011/09/10 18:50:38]

I couldn’t figure out why you would want to do this on the beta channel and then
stop doing it when beta is promoted to stable. Checking explicitly for
CHANNEL_STABLE here means that this will behave as I expect was intended if the
stable channel’s default for crash_on_hang_threads changes, or if someone uses
--crash-on-hang-threads without --crash-on-hang-seconds in a stable-channel
build. As it was written, the logic was prone to providing surprising results
under future maintenance.

-  } else {
+  } else if (channel == chrome::VersionInfo::CHANNEL_DEV ||
+             channel == chrome::VersionInfo::CHANNEL_CANARY) {
     // In Canary and Dev channels, for Windows XP (old systems), double the
     // unresponsive_threshold to give OS a chance to schedule UI/IO threads a
     // time slice to respond with a pong message (to get around limitations with
@@ -452,7 +455,7 @@
 
   // Default to crashing the browser if UI or IO threads are not responsive
   // except in stable channel.
-  if (chrome::VersionInfo::GetChannel() == chrome::VersionInfo::CHANNEL_STABLE)
+  if (channel == chrome::VersionInfo::CHANNEL_STABLE)
     crash_on_hang_threads = "";
   else
     crash_on_hang_threads = "UI,IO";
@@ -734,7 +737,7 @@
   } else if (channel == chrome::VersionInfo::CHANNEL_BETA ||
              channel == chrome::VersionInfo::CHANNEL_DEV) {
     actual_duration *= 25;
-  } else {
+  } else if (channel == chrome::VersionInfo::CHANNEL_CANARY) {
     // In Canary, for Windows XP, give twice the time for shutdown.
 #if defined(OS_WIN)
     if (base::win::GetVersion() <= base::win::VERSION_XP)