Revert 76015 - Added back thread watcher changes by

reverting the revert.

will revert the changes right away. trying to test 
impact on memory on mac.

BUG=73915
TEST=performance tests

Review URL: http://codereview.chromium.org/6594011

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

7 files changed, 119 insertions, 43 deletions

diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc
index 3a701e0..a7d474c 100644
--- a/chrome/browser/browser_main.cc
+++ b/chrome/browser/browser_main.cc
@@ -45,6 +45,7 @@
 #include "chrome/browser/metrics/histogram_synchronizer.h"
 #include "chrome/browser/metrics/metrics_log.h"
 #include "chrome/browser/metrics/metrics_service.h"
+#include "chrome/browser/metrics/thread_watcher.h"
 #include "chrome/browser/net/blob_url_request_job_factory.h"
 #include "chrome/browser/net/chrome_dns_cert_provenance_checker.h"
 #include "chrome/browser/net/chrome_dns_cert_provenance_checker_factory.h"
@@ -702,6 +703,11 @@ void CreateChildThreads(BrowserProcessImpl* process) {
   process->process_launcher_thread();
   process->cache_thread();
   process->io_thread();
+  // Create watchdog thread after creating all other threads because it will
+  // watch the other threads and they must be running.
+#if defined(OS_WIN)
+  process->watchdog_thread();
+#endif
 }
 
 // Returns the new local state object, guaranteed non-NULL.
@@ -1353,6 +1359,11 @@ int BrowserMain(const MainFunctionParams& parameters) {
   scoped_refptr<HistogramSynchronizer> histogram_synchronizer(
       new HistogramSynchronizer());
 
+  // Initialize thread watcher system. This is a singleton and is used by
+  // WATCHDOG thread to keep track of information about threads that are being
+  // watched.
+  scoped_ptr<ThreadWatcherList> thread_watcher_list(new ThreadWatcherList());
+
   // Initialize the prefs of the local state.
   browser::RegisterLocalState(local_state);
 
@@ -1903,6 +1914,9 @@ int BrowserMain(const MainFunctionParams& parameters) {
 
   process_singleton.Cleanup();
 
+  // Stop all tasks that might run on WATCHDOG thread.
+  ThreadWatcherList::StopWatchingAll();
+
   metrics->Stop();
 
   // browser_shutdown takes care of deleting browser_process, so we need to
diff --git a/chrome/browser/browser_process.h b/chrome/browser/browser_process.h
index c1275db..8a1ab65a 100644
--- a/chrome/browser/browser_process.h
+++ b/chrome/browser/browser_process.h
@@ -39,6 +39,7 @@ class ResourceDispatcherHost;
 class SidebarManager;
 class TabCloseableStateWatcher;
 class ThumbnailGenerator;
+class WatchDogThread;
 
 namespace base {
 class Thread;
@@ -115,6 +116,9 @@ class BrowserProcess {
   virtual base::Thread* background_x11_thread() = 0;
 #endif
 
+  // Returns the thread that is used for health check of all browser threads.
+  virtual WatchDogThread* watchdog_thread() = 0;
+
   virtual policy::BrowserPolicyConnector* browser_policy_connector() = 0;
 
   virtual IconManager* icon_manager() = 0;
diff --git a/chrome/browser/browser_process_impl.cc b/chrome/browser/browser_process_impl.cc
index f65c65b..9f0fcee 100644
--- a/chrome/browser/browser_process_impl.cc
+++ b/chrome/browser/browser_process_impl.cc
@@ -32,6 +32,7 @@
 #include "chrome/browser/intranet_redirect_detector.h"
 #include "chrome/browser/io_thread.h"
 #include "chrome/browser/metrics/metrics_service.h"
+#include "chrome/browser/metrics/thread_watcher.h"
 #include "chrome/browser/net/chrome_net_log.h"
 #include "chrome/browser/net/predictor_api.h"
 #include "chrome/browser/net/sdch_dictionary_fetcher.h"
@@ -97,6 +98,7 @@ BrowserProcessImpl::BrowserProcessImpl(const CommandLine& command_line)
       created_db_thread_(false),
       created_process_launcher_thread_(false),
       created_cache_thread_(false),
+      created_watchdog_thread_(false),
       created_profile_manager_(false),
       created_local_state_(false),
       created_icon_manager_(false),
@@ -245,6 +247,9 @@ BrowserProcessImpl::~BrowserProcessImpl() {
   // on the db thread too.
   db_thread_.reset();
 
+  // Stop the watchdog thread after stopping other threads.
+  watchdog_thread_.reset();
+
   // At this point, no render process exist and the file, io, db, and
   // webkit threads in this process have all terminated, so it's safe
   // to access local state data such as cookies, database, or local storage.
@@ -390,6 +395,14 @@ base::Thread* BrowserProcessImpl::background_x11_thread() {
 }
 #endif
 
+WatchDogThread* BrowserProcessImpl::watchdog_thread() {
+  DCHECK(CalledOnValidThread());
+  if (!created_watchdog_thread_)
+    CreateWatchdogThread();
+  DCHECK(watchdog_thread_.get() != NULL);
+  return watchdog_thread_.get();
+}
+
 ProfileManager* BrowserProcessImpl::profile_manager() {
   DCHECK(CalledOnValidThread());
   if (!created_profile_manager_)
@@ -725,6 +738,16 @@ void BrowserProcessImpl::CreateCacheThread() {
   cache_thread_.swap(thread);
 }
 
+void BrowserProcessImpl::CreateWatchdogThread() {
+  DCHECK(!created_watchdog_thread_ && watchdog_thread_.get() == NULL);
+  created_watchdog_thread_ = true;
+
+  scoped_ptr<WatchDogThread> thread(new WatchDogThread());
+  if (!thread->Start())
+    return;
+  watchdog_thread_.swap(thread);
+}
+
 void BrowserProcessImpl::CreateProfileManager() {
   DCHECK(!created_profile_manager_ && profile_manager_.get() == NULL);
   created_profile_manager_ = true;
diff --git a/chrome/browser/browser_process_impl.h b/chrome/browser/browser_process_impl.h
index 4926812..162b82a 100644
--- a/chrome/browser/browser_process_impl.h
+++ b/chrome/browser/browser_process_impl.h
@@ -57,6 +57,7 @@ class BrowserProcessImpl : public BrowserProcess,
 #if defined(USE_X11)
   virtual base::Thread* background_x11_thread();
 #endif
+  virtual WatchDogThread* watchdog_thread();
   virtual ProfileManager* profile_manager();
   virtual PrefService* local_state();
   virtual DevToolsManager* devtools_manager();
@@ -123,6 +124,7 @@ class BrowserProcessImpl : public BrowserProcess,
   void CreateDBThread();
   void CreateProcessLauncherThread();
   void CreateCacheThread();
+  void CreateWatchdogThread();
   void CreateTemplateURLModel();
   void CreateProfileManager();
   void CreateWebDataService();
@@ -170,6 +172,9 @@ class BrowserProcessImpl : public BrowserProcess,
   bool created_cache_thread_;
   scoped_ptr<base::Thread> cache_thread_;
 
+  bool created_watchdog_thread_;
+  scoped_ptr<WatchDogThread> watchdog_thread_;
+
   bool created_profile_manager_;
   scoped_ptr<ProfileManager> profile_manager_;
 
diff --git a/chrome/browser/metrics/thread_watcher.cc b/chrome/browser/metrics/thread_watcher.cc
index ececcac..9a07dde 100644
--- a/chrome/browser/metrics/thread_watcher.cc
+++ b/chrome/browser/metrics/thread_watcher.cc
@@ -2,8 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#if 0
-
 #include "base/threading/thread_restrictions.h"
 #include "build/build_config.h"
 #include "chrome/browser/metrics/metrics_service.h"
@@ -192,7 +190,7 @@ ThreadWatcherList::ThreadWatcherList()
   // Assert we are not running on WATCHDOG thread. Would be ideal to assert we
   // are on UI thread, but Unit tests are not running on UI thread.
   DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::WATCHDOG));
-  DCHECK(!global_);
+  CHECK(!global_);
   global_ = this;
   // Register Notifications observer.
 #if defined(OS_WIN)
@@ -202,11 +200,6 @@ ThreadWatcherList::ThreadWatcherList()
 
 ThreadWatcherList::~ThreadWatcherList() {
   base::AutoLock auto_lock(lock_);
-  while (!registered_.empty()) {
-    RegistrationList::iterator it = registered_.begin();
-    delete it->second;
-    registered_.erase(it->first);
-  }
   DCHECK(this == global_);
   global_ = NULL;
 }
@@ -224,15 +217,15 @@ void ThreadWatcherList::StopWatchingAll() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   if (!global_)
     return;
-  base::AutoLock auto_lock(global_->lock_);
-  for (RegistrationList::iterator it = global_->registered_.begin();
-       global_->registered_.end() != it;
-       ++it)
-    BrowserThread::PostTask(
-        BrowserThread::WATCHDOG,
-        FROM_HERE,
-        NewRunnableMethod(
-            it->second, &ThreadWatcher::DeActivateThreadWatching));
+
+  // Remove all notifications for all watched threads.
+  RemoveNotifications();
+
+  // Delete all thread watcher objects on WATCHDOG thread.
+  BrowserThread::PostTask(
+      BrowserThread::WATCHDOG,
+      FROM_HERE,
+      NewRunnableMethod(global_, &ThreadWatcherList::DeleteAll));
 }
 
 // static
@@ -246,6 +239,16 @@ void ThreadWatcherList::RemoveNotifications() {
 #endif
 }
 
+void ThreadWatcherList::DeleteAll() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::WATCHDOG));
+  base::AutoLock auto_lock(lock_);
+  while (!registered_.empty()) {
+    RegistrationList::iterator it = registered_.begin();
+    delete it->second;
+    registered_.erase(it->first);
+  }
+}
+
 void ThreadWatcherList::Observe(NotificationType type,
                                 const NotificationSource& source,
                                 const NotificationDetails& details) {
@@ -303,8 +306,6 @@ WatchDogThread::WatchDogThread()
 }
 
 WatchDogThread::~WatchDogThread() {
-  // Remove all notifications for all watched threads.
-  ThreadWatcherList::RemoveNotifications();
   // We cannot rely on our base class to stop the thread since we want our
   // CleanUp function to run.
   Stop();
@@ -317,19 +318,42 @@ void WatchDogThread::Init() {
   BrowserProcessSubThread::Init();
 
 #if defined(OS_WIN)
+  BrowserThread::PostDelayedTask(
+      BrowserThread::WATCHDOG,
+      FROM_HERE,
+      NewRunnableFunction(&WatchDogThread::StartWatchingAll),
+      base::TimeDelta::FromSeconds(10).InMilliseconds());
+#endif
+}
+
+void WatchDogThread::CleanUp() {
+  BrowserProcessSubThread::CleanUp();
+}
+
+void WatchDogThread::CleanUpAfterMessageLoopDestruction() {
+  // This will delete the |notification_service_|.  Make sure it's done after
+  // anything else can reference it.
+  BrowserProcessSubThread::CleanUpAfterMessageLoopDestruction();
+}
+
+void WatchDogThread::StartWatchingAll() {
   const base::TimeDelta kSleepTime = base::TimeDelta::FromSeconds(5);
   const base::TimeDelta kUnresponsiveTime = base::TimeDelta::FromSeconds(10);
-  ThreadWatcher::StartWatching(BrowserThread::UI, "UI", kSleepTime,
-                               kUnresponsiveTime);
-  ThreadWatcher::StartWatching(BrowserThread::IO, "IO", kSleepTime,
-                               kUnresponsiveTime);
-  ThreadWatcher::StartWatching(BrowserThread::DB, "DB", kSleepTime,
-                               kUnresponsiveTime);
-  ThreadWatcher::StartWatching(BrowserThread::FILE, "FILE", kSleepTime,
-                               kUnresponsiveTime);
-  ThreadWatcher::StartWatching(BrowserThread::CACHE, "CACHE", kSleepTime,
-                               kUnresponsiveTime);
-#endif
+  if (BrowserThread::IsMessageLoopValid(BrowserThread::UI))
+    ThreadWatcher::StartWatching(BrowserThread::UI, "UI", kSleepTime,
+                                 kUnresponsiveTime);
+  if (BrowserThread::IsMessageLoopValid(BrowserThread::IO))
+    ThreadWatcher::StartWatching(BrowserThread::IO, "IO", kSleepTime,
+                                 kUnresponsiveTime);
+  if (BrowserThread::IsMessageLoopValid(BrowserThread::DB))
+    ThreadWatcher::StartWatching(BrowserThread::DB, "DB", kSleepTime,
+                                 kUnresponsiveTime);
+  if (BrowserThread::IsMessageLoopValid(BrowserThread::FILE))
+    ThreadWatcher::StartWatching(BrowserThread::FILE, "FILE", kSleepTime,
+                                 kUnresponsiveTime);
+  if (BrowserThread::IsMessageLoopValid(BrowserThread::CACHE))
+    ThreadWatcher::StartWatching(BrowserThread::CACHE, "CACHE", kSleepTime,
+                                 kUnresponsiveTime);
 }
 
-#endif  // 0
+
diff --git a/chrome/browser/metrics/thread_watcher.h b/chrome/browser/metrics/thread_watcher.h
index f7765c8..7d93561 100644
--- a/chrome/browser/metrics/thread_watcher.h
+++ b/chrome/browser/metrics/thread_watcher.h
@@ -8,8 +8,6 @@
 #define CHROME_BROWSER_METRICS_THREAD_WATCHER_H_
 #pragma once
 
-#if 0
-
 #include <map>
 #include <string>
 #include <vector>
@@ -206,25 +204,33 @@ class ThreadWatcherList : public NotificationObserver {
   // This method posts a task on WATCHDOG thread to RevokeAll tasks and to
   // deactive thread watching of other threads and tell NotificationService to
   // stop calling Observe.
+  // This method is accessible on UI thread.
   static void StopWatchingAll();
 
   // RemoveAll NotificationTypes that are being observed.
+  // This method is accessible on UI thread.
   static void RemoveNotifications();
 
  private:
   // Allow tests to access our innards for testing purposes.
   FRIEND_TEST(ThreadWatcherTest, Registration);
 
+  // Delete all thread watcher objects and remove them from global map.
+  // This method is accessible on WATCHDOG thread.
+  void DeleteAll();
+
   // This will ensure that the watching is actively taking place. It will wakeup
   // all thread watchers every 2 seconds. This is the implementation of
   // NotificationObserver. When a matching notification is posted to the
   // notification service, this method is called.
+  // This method is accessible on UI thread.
   virtual void Observe(NotificationType type,
                        const NotificationSource& source,
                        const NotificationDetails& details);
 
   // This will ensure that the watching is actively taking place, and awaken
   // all thread watchers that are registered.
+  // This method is accessible on WATCHDOG thread.
   virtual void WakeUpAll();
 
   // The Find() method can be used to test to see if a given ThreadWatcher was
@@ -259,6 +265,11 @@ class WatchDogThread : public BrowserProcessSubThread {
 
  protected:
   virtual void Init();
+  virtual void CleanUp();
+  virtual void CleanUpAfterMessageLoopDestruction();
+
+  // Start watching all browser threads.
+  static void StartWatchingAll();
 
   DISALLOW_COPY_AND_ASSIGN(WatchDogThread);
 };
@@ -267,6 +278,4 @@ class WatchDogThread : public BrowserProcessSubThread {
 DISABLE_RUNNABLE_METHOD_REFCOUNT(ThreadWatcher);
 DISABLE_RUNNABLE_METHOD_REFCOUNT(ThreadWatcherList);
 
-#endif  // 0
-
 #endif  // CHROME_BROWSER_METRICS_THREAD_WATCHER_H_
diff --git a/chrome/browser/metrics/thread_watcher_unittest.cc b/chrome/browser/metrics/thread_watcher_unittest.cc
index 52a4288..b077492 100644
--- a/chrome/browser/metrics/thread_watcher_unittest.cc
+++ b/chrome/browser/metrics/thread_watcher_unittest.cc
@@ -2,8 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#if 0
-
 #include "base/basictypes.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
@@ -13,10 +11,13 @@
 #include "base/synchronization/lock.h"
 #include "base/threading/platform_thread.h"
 #include "base/time.h"
+#include "build/build_config.h"
 #include "chrome/browser/metrics/thread_watcher.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/platform_test.h"
 
+#if defined(OS_WIN)
+
 using base::TimeDelta;
 using base::TimeTicks;
 
@@ -245,10 +246,6 @@ class ThreadWatcherTest : public ::testing::Test {
   CustomThreadWatcher* webkit_watcher_;
 
   ThreadWatcherTest() {
-  }
-
- protected:
-  virtual void SetUp() {
     webkit_thread_.reset(new BrowserThread(BrowserThread::WEBKIT));
     io_thread_.reset(new BrowserThread(BrowserThread::IO));
     watchdog_thread_.reset(new BrowserThread(BrowserThread::WATCHDOG));
@@ -268,7 +265,7 @@ class ThreadWatcherTest : public ::testing::Test {
         webkit_thread_id, webkit_thread_name, kSleepTime, kUnresponsiveTime);
   }
 
-  virtual void TearDown() {
+  ~ThreadWatcherTest() {
     // io_thread_->Stop();
     // webkit_thread_->Stop();
     // watchdog_thread_->Stop();
@@ -482,4 +479,4 @@ TEST_F(ThreadWatcherTest, MultipleThreadsNotResponding) {
           webkit_watcher_, &ThreadWatcher::DeActivateThreadWatching));
 }
 
-#endif  // 0
+#endif  // OS_WIN