Added back thread watcher changes.

Added back thread watcher changes.

BUG=71378, 73915, 73844, 73975
TEST=performance tests


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=76468

[ramant (doing other things), 2011-02-26 09:57:51]

Hi Jim,
  This patch is same as the first cut thread watcher changes we had checked in
earlier this week, except version#1 has bugs we had fixed today.

thanks,
raman

[jar (doing other things), 2011-02-26 15:33:57]

LGTM

[jam, 2011-02-26 19:16:26]

Since this has been reverted, I prefer we don't check it back in until the
BrowserThread changes are reverted.

On Sat, Feb 26, 2011 at 7:33 AM, <[email protected]> wrote:

> LGTM
>
>
>
>
> http://codereview.chromium.org/6588039/
>

[ramant (doing other things), 2011-02-28 06:59:48]

Hi Jim,
  Added 48k as thread's stack size (for experiment) to see if that is enough for
watchdog thread and also wanted to verify we don't use the 8MB default stack
size on Mac.

  Removed WATCHDOG from browser threads. Posted tasks to CurrentMessageLoop of
the watchdog thread. Added lock and checks for watchdog thread and message_loop
not being NULL before posting tasks.

  Moved StartWatchingAll to browser_main (we Stop Watching All threads in
browser_main. During unit tests, didn't want to start watching all threads when
we start watchdog thread).

  Would appreciate your comments.

thanks very much,
raman

[ramant (doing other things), 2011-03-01 00:39:30]

Hi Jim,
  As we had discussed, implemented PostTask methods on WatchDog threads so that
Task get posted during lock. Would appreciate comments.

thanks,
raman

[ramant (doing other things), 2011-03-01 01:54:24]

Hi Jim and John,
  Made the changes to remove WATCHDOG thread from browser_thread.cc. Would
appreciate if you could review the changes.

thanks much,
raman

[jar (doing other things), 2011-03-01 02:02:39]

changes LGTM.

Please be sure jam@ is satisfied before landing.

Thanks!

[ramant (doing other things), 2011-03-01 06:41:14]

Hi Jim and John,
  ThreadWatcher tests were suppressed in tsan. Removed the suppressed lines.
While testing the unit tests, tsan found a race condition accessing the data in
ThreadWatcher object. In CustomThreadWatcher (the derived class used for testing
purposes), updated the data under locks because unit tests are running on
separate thread from watchdog thread.

  Verified Linux valgrind and tsan tests have passed. Sorry for making the
changes so late.

thanks,
raman

[jar (doing other things), 2011-03-01 17:59:53]

Changes to remove races LGTM.

Small question below.

Thanks.

http://codereview.chromium.org/6588039/diff/6037/chrome/browser/metrics/threa...
File chrome/browser/metrics/thread_watcher_unittest.cc (right):

http://codereview.chromium.org/6588039/diff/6037/chrome/browser/metrics/threa...
chrome/browser/metrics/thread_watcher_unittest.cc:278: // io_thread_->Stop();
Why do we have this commented out code?

[jam, 2011-03-01 18:54:05]

lgtm

[ramant (doing other things), 2011-03-01 19:17:12]

Thanks very much Jim and John. Made the changes Jim suggested. Will land the
changes asap.

-raman

http://codereview.chromium.org/6588039/diff/6037/chrome/browser/metrics/threa...
File chrome/browser/metrics/thread_watcher_unittest.cc (right):

http://codereview.chromium.org/6588039/diff/6037/chrome/browser/metrics/threa...
chrome/browser/metrics/thread_watcher_unittest.cc:278: // io_thread_->Stop();
On 2011/03/01 17:59:54, jar wrote:
> Why do we have this commented out code?

Done.