Added command line switches "crash-on-hang-threads" and "crash-on-hang-seconds"

chrome/browser/metrics/thread_watcher.h

Index: chrome/browser/metrics/thread_watcher.h
===================================================================
--- chrome/browser/metrics/thread_watcher.h	(revision 88502)
+++ chrome/browser/metrics/thread_watcher.h	(working copy)
@@ -25,17 +25,22 @@
 //
 //   base::TimeDelta sleep_time = base::TimeDelta::FromSeconds(5);
 //   base::TimeDelta unresponsive_time = base::TimeDelta::FromSeconds(10);
+//   uint32 crash_on_unresponsive_count = ThreadWatcherList::kUnresponsiveCount;
+//   bool crash_on_hang = false;
 //   ThreadWatcher::StartWatching(BrowserThread::IO, "IO", sleep_time,
-//                                unresponsive_time);
+//                                unresponsive_time,
+//                                crash_on_unresponsive_count, crash_on_hang);
 
 #ifndef CHROME_BROWSER_METRICS_THREAD_WATCHER_H_
 #define CHROME_BROWSER_METRICS_THREAD_WATCHER_H_
 
 #include <map>
+#include <set>
 #include <string>
 #include <vector>
 
 #include "base/basictypes.h"
+#include "base/command_line.h"
 #include "base/gtest_prod_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
@@ -55,19 +60,25 @@
 // This class performs health check on threads that would like to be watched.
 class ThreadWatcher {
  public:
-  // This method starts performing health check on the given thread_id. It will
-  // create ThreadWatcher object for the given thread_id, thread_name,
-  // sleep_time and unresponsive_time. sleep_time_ is the wait time between ping
-  // messages. unresponsive_time_ is the wait time after ping message is sent,
-  // to check if we have received pong message or not. It will register that
-  // ThreadWatcher object and activate the thread watching of the given
-  // thread_id.
+  // This method starts performing health check on the given |thread_id|. It
+  // will create ThreadWatcher object for the given |thread_id|, |thread_name|.
+  // |sleep_time| is the wait time between ping messages. |unresponsive_time| is
+  // the wait time after ping message is sent, to check if we have received pong
+  // message or not. |crash_on_unresponsive_count| is used to determine if the
+  // thread is responsive or not. The watched thread is considered unresponsive
+  // if it hasn't responded with a pong message for
+  // |crash_on_unresponsive_count| number of ping messages. |crash_on_hang|
+  // specifies if browser should be crashed when the watched thread is
+  // unresponsive. It will register that ThreadWatcher object and activate the
+  // thread watching of the given thread_id.
   static void StartWatching(const BrowserThread::ID& thread_id,
                             const std::string& thread_name,
                             const base::TimeDelta& sleep_time,
-                            const base::TimeDelta& unresponsive_time);
+                            const base::TimeDelta& unresponsive_time,
+                            uint32 crash_on_unresponsive_count,
+                            bool crash_on_hang);
 
-  // Return the thread_id of the thread being watched.
+  // Return the |thread_id_| of the thread being watched.
   BrowserThread::ID thread_id() const { return thread_id_; }
 
   // Return the name of the thread being watched.
@@ -82,20 +93,27 @@
   // Returns true if we are montioring the thread.
   bool active() const { return active_; }
 
-  // Returns ping_time_ (used by unit tests).
+  // Returns |ping_time_| (used by unit tests).
   base::TimeTicks ping_time() const { return ping_time_; }
 
-  // Returns ping_sequence_number_ (used by unit tests).
+  // Returns |ping_sequence_number_| (used by unit tests).
   uint64 ping_sequence_number() const { return ping_sequence_number_; }
 
  protected:
-  // Construct a ThreadWatcher for the given thread_id. sleep_time_ is the
-  // wait time between ping messages. unresponsive_time_ is the wait time after
+  // Construct a ThreadWatcher for the given |thread_id|. |sleep_time| is the
+  // wait time between ping messages. |unresponsive_time| is the wait time after
   // ping message is sent, to check if we have received pong message or not.
+  // |crash_on_unresponsive_count| is used to determine if the thread is
+  // responsive or not. The watched thread is considered unresponsive if it
+  // hasn't responded with a pong message for |crash_on_unresponsive_count|
+  // number of ping messages. |crash_on_hang| specifies if browser should be
+  // crashed when the watched thread is unresponsive.
   ThreadWatcher(const BrowserThread::ID& thread_id,
                 const std::string& thread_name,
                 const base::TimeDelta& sleep_time,
-                const base::TimeDelta& unresponsive_time);
+                const base::TimeDelta& unresponsive_time,
+                uint32 crash_on_unresponsive_count,
+                bool crash_on_hang);
   virtual ~ThreadWatcher();
 
   // This method activates the thread watching which starts ping/pong messaging.
@@ -105,29 +123,29 @@
   virtual void DeActivateThreadWatching();
 
   // This will ensure that the watching is actively taking place, and awaken
-  // (i.e., post a PostPingMessage) if the watcher has stopped pinging due to
-  // lack of user activity. It will also reset ping_count_ to kPingCount.
+  // (i.e., post a PostPingMessage()) if the watcher has stopped pinging due to
+  // lack of user activity. It will also reset |ping_count_| to |kPingCount|.
   virtual void WakeUp();
 
   // This method records when ping message was sent and it will Post a task
-  // (OnPingMessage) to the watched thread that does nothing but respond with
-  // OnPongMessage. It also posts a task (OnCheckResponsiveness) to check
+  // (OnPingMessage()) to the watched thread that does nothing but respond with
+  // OnPongMessage(). It also posts a task (OnCheckResponsiveness()) to check
   // responsiveness of monitored thread that would be called after waiting
-  // unresponsive_time_.
+  // |unresponsive_time_|.
   // This method is accessible on WatchDogThread.
   virtual void PostPingMessage();
 
   // This method handles a Pong Message from watched thread. It will track the
   // response time (pong time minus ping time) via histograms. It posts a
-  // PostPingMessage task that would be called after waiting sleep_time_.  It
-  // increments ping_sequence_number_ by 1.
+  // PostPingMessage() task that would be called after waiting |sleep_time_|. It
+  // increments |ping_sequence_number_| by 1.
   // This method is accessible on WatchDogThread.
   virtual void OnPongMessage(uint64 ping_sequence_number);
 
   // This method will determine if the watched thread is responsive or not. If
-  // the latest ping_sequence_number_ is not same as the ping_sequence_number
-  // that is passed in, then we can assume that watched thread has responded
-  // with a pong message.
+  // the latest |ping_sequence_number_| is not same as the
+  // |ping_sequence_number| that is passed in, then we can assume that watched
+  // thread has responded with a pong message.
   // This method is accessible on WatchDogThread.
   virtual bool OnCheckResponsiveness(uint64 ping_sequence_number);
 
@@ -150,26 +168,24 @@
   static void OnPingMessage(const BrowserThread::ID& thread_id,
                             Task* callback_task);
 
-  // This method resets unresponsive_count_ to zero because watched thread is
+  // This method resets |unresponsive_count_| to zero because watched thread is
   // responding to the ping message with a pong message.
   void ResetHangCounters();
 
   // This method records watched thread is not responding to the ping message.
-  // It increments unresponsive_count_ by 1.
+  // It increments |unresponsive_count_| by 1.
   void GotNoResponse();
 
+  // This method returns true if the watched thread has not responded with a
+  // pong message for |crash_on_unresponsive_count_| number of ping messages.
+  bool CrashOnUnresponsiveness();
+
   // This is the number of ping messages to be sent when the user is idle.
   // ping_count_ will be initialized to kPingCount whenever user becomes active.
   static const int kPingCount;
 
-  // This value is used to determine if the watched thread is responsive or not.
-  // If unresponsive_count_ is less than kUnresponsiveCount then watched thread
-  // is considered as responsive (in responsive_count_histogram_) otherwise it
-  // is considered as unresponsive (in unresponsive_count_histogram_).
-  static const int kUnresponsiveCount;
-
-  // The thread_id of the thread being watched. Only one instance can exist for
-  // the given thread_id of the thread being watched.
+  // The |thread_id_| of the thread being watched. Only one instance can exist
+  // for the given |thread_id_| of the thread being watched.
   const BrowserThread::ID thread_id_;
 
   // The name of the thread being watched.
@@ -207,29 +223,39 @@
   base::Histogram* response_time_histogram_;
 
   // Histogram that keeps track of unresponsive time since the last pong message
-  // when we got no response (GotNoResponse) from the watched thread.
+  // when we got no response (GotNoResponse()) from the watched thread.
   base::Histogram* unresponsive_time_histogram_;
 
   // Histogram that keeps track of how many threads are responding when we got
-  // no response (GotNoResponse) from the watched thread.
+  // no response (GotNoResponse()) from the watched thread.
   base::Histogram* responsive_count_histogram_;
 
   // Histogram that keeps track of how many threads are not responding when we
-  // got no response (GotNoResponse) from the watched thread. Count includes the
-  // thread that got no response.
+  // got no response (GotNoResponse()) from the watched thread. Count includes
+  // the thread that got no response.
   base::Histogram* unresponsive_count_histogram_;
 
   // This counter tracks the unresponsiveness of watched thread. If this value
   // is zero then watched thread has responded with a pong message. This is
-  // incremented by 1 when we got no response (GotNoResponse) from the watched
+  // incremented by 1 when we got no response (GotNoResponse()) from the watched
   // thread.
-  int unresponsive_count_;
+  uint32 unresponsive_count_;
 
   // This is set to true when we would have crashed the browser because the
   // watched thread hasn't responded atleast 6 times. It is reset to false when
   // watched thread responds with a pong message.
   bool hung_processing_complete_;
 
+  // This is used to determine if the watched thread is responsive or not. If
+  // watched thread's |unresponsive_count_| is greater than or equal to
+  // |crash_on_unresponsive_count_| then we would consider it as unresponsive.
+  uint32 crash_on_unresponsive_count_;

[jar (doing other things), 2011/06/14 00:56:27]

nit: Consider changing the name.  The count is about a threshold, but it may or
may not involve crashing.  How about:

unresponsive_threshold_

[ramant (doing other things), 2011/06/16 22:26:45]

Done.

+
+  // This is set to true if we want to the crash the browser when watched
+  // thread has not responded with a pong message for
+  // |crash_on_unresponsive_count_| number of ping messages.

[jar (doing other things), 2011/06/14 00:56:27]

Some of the comment are repetetive.  It is nicer when possible to refer to other
items for details.  For instance, this could be:

This is set to true if we want to crash the browser when the watch thread has
become sufficiently unresponsive, while other threads are sufficiently
responsive.


The advantage of this is that IF we change something, we don't need to change a
ton of comments.  It best, these little micro-comments could duplicate a header
comment, which describes the whole picture.  You shouldn't have to keep
mentioning the count in each of these comments though.  Also note that it is
more than count, as we want a couple of things to to have happened.

[ramant (doing other things), 2011/06/16 22:26:45]

Done.

+  bool crash_on_hang_;
+
   // We use this factory to create callback tasks for ThreadWatcher object. We
   // use this during ping-pong messaging between WatchDog thread and watched
   // thread.
@@ -248,7 +274,7 @@
   typedef std::map<BrowserThread::ID, ThreadWatcher*> RegistrationList;
 
   // This singleton holds the global list of registered ThreadWatchers.
-  ThreadWatcherList();
+  explicit ThreadWatcherList(const CommandLine& command_line);
   // Destructor deletes all registered ThreadWatcher instances.
   virtual ~ThreadWatcherList();
 
@@ -259,7 +285,8 @@
   static bool IsRegistered(const BrowserThread::ID thread_id);
 
   // This method posts a task on WatchDogThread to start watching all browser
-  // threads.
+  // threads. It calls ThreadWatcherList::StartWatching() on each browser thread
+  // that is watched.
   // This method is accessible on UI thread.
   static void StartWatchingAll();
 
@@ -273,15 +300,35 @@
   // This method is accessible on UI thread.
   static void RemoveNotifications();
 
-  // This method returns number of watched threads that have responded and
-  // threads that have not responded with a pong message.
+  // This method returns number of responsive and unresponsive watched threads.
   static void GetStatusOfThreads(int* no_of_responding_threads,

[jar (doing other things), 2011/06/14 00:56:27]

nit: abreviations should not be used in names in this case "no" means "number." 
Suggest:
responding_thread_count
unresponding_thread_count

[ramant (doing other things), 2011/06/16 22:26:45]

Done.

                                  int* no_of_unresponding_threads);
 
  private:
   // Allow tests to access our innards for testing purposes.
+  friend class CustomThreadWatcher;
+  FRIEND_TEST_ALL_PREFIXES(ThreadWatcherTest, CommandLineArgs);
   FRIEND_TEST_ALL_PREFIXES(ThreadWatcherTest, Registration);
 
+  // This method calls ThreadWatcher::StartWatching() to perform health check on
+  // the given |thread_id|, |thread_name|. |sleep_time| is the wait time between
+  // ping messages. |unresponsive_time| is the wait time after ping message is
+  // sent, to check if we have received pong message or not.
+  static void StartWatching(const BrowserThread::ID& thread_id,
+                            const std::string& thread_name,
+                            const base::TimeDelta& sleep_time,
+                            const base::TimeDelta& unresponsive_time);
+
+  // Helper methods to access crash_on_unresponsive_count_ (for unit tests).
+  uint32 CrashOnUnresponsiveCount();
+  uint32 PreLockedCrashOnUnresponsiveCount() const {
+      return crash_on_unresponsive_count_;
+  }
+
+  // Returns true if the |thread_name| is in the |crash_on_hang_thread_names_|.
+  bool CrashOnHang(const std::string& thread_name);
+  bool PreLockedCrashOnHang(const std::string& thread_name) const;
+
   // Delete all thread watcher objects and remove them from global map.
   // This method is accessible on WatchDogThread.
   void DeleteAll();
@@ -316,6 +363,9 @@
   // received pong message or not.
   static const int kUnresponsiveSeconds;
 
+  // Default values for |crash_on_unresponsive_count_|.
+  static const int kUnresponsiveCount;
+
   // Lock for access to registered_.
   base::Lock lock_;
 
@@ -328,6 +378,20 @@
   // This is the last time when woke all thread watchers up.
   base::TimeTicks last_wakeup_time_;
 
+  // This is used to determine if the watched thread is responsive or not. If
+  // watched thread's |unresponsive_count_| is greater than or equal to
+  // |crash_on_unresponsive_count_| then we could crash the browser if the
+  // watched thread is in |crash_on_hang_thread_names_|.
+  // |crash_on_unresponsive_count_| is initialized with |kUnresponsiveCount|,
+  // but can be overwritten by the command line switch
+  // "--crash-on-hang-seconds".
+  uint32 crash_on_unresponsive_count_;
+
+  // This is the set of watched thread's names that are to be crashed if they
+  // have not responded with a pong message for |crash_on_unresponsive_count_|
+  // number of ping messages.
+  std::set<std::string> crash_on_hang_thread_names_;
+
   DISALLOW_COPY_AND_ASSIGN(ThreadWatcherList);
 };