Fixes bad URLRequest leak detection. nss_ocsp creates a

URLRequest. This URLRequest, if valid, is deleted when the IO thread
is being shutdown by way of a DestructionObserver attached to the IO
thread. Prior to this patch this proves problematic as the LeakTracker
is run before DestructionObservers, which means we were errorenously
detecting a leak and causing random UI failures (and perhaps other
tests).

To fix this I've moved the LeakTracker to run after the IO thread
MessageLoop has been deleted (which means DestructionObservers have
been notified). Doing this triggered a DCHECK in
ChromeNetLog::AddEntry:

  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));

Here's the full stack:

	StackTrace::StackTrace() [0xf247c1]
	logging::LogMessage::~LogMessage() [0xf3ef4f]
	ChromeNetLog::AddEntry() [0x5c9639]
	net::BoundNetLog::AddEntry() [0x147d8a3]
	net::BoundNetLog::AddEvent() [0x147d92a]
	net::internal::ClientSocketPoolBaseHelper::CancelRequest() [0x132d032]
	net::ClientSocketPoolBase<>::CancelRequest() [0x134aabd]
	net::TCPClientSocketPool::CancelRequest() [0x1348fdb]
	net::ClientSocketHandle::ResetInternal() [0x13273b7]
	net::ClientSocketHandle::Reset() [0x1327874]
	net::ClientSocketHandle::~ClientSocketHandle() [0x1327895]
	scoped_ptr<>::~scoped_ptr() [0x1304b97]
	net::HttpStreamRequest::~HttpStreamRequest() [0x1300933]
	base::RefCounted<>::Release() [0xa15b83]
	scoped_refptr<>::operator=() [0x12f2da4]
	net::HttpNetworkTransaction::~HttpNetworkTransaction() [0x12ee377]
	scoped_ptr<>::~scoped_ptr() [0x12e2724]
	net::HttpCache::Transaction::~Transaction() [0x12e10a6]
	scoped_ptr<>::reset() [0x12d1f0b]
	URLRequestHttpJob::DestroyTransaction() [0x142229e]
	URLRequestHttpJob::Kill() [0x14252e5]
	URLRequest::DoCancel() [0x137af1e]
	URLRequest::Cancel() [0x137b081]
	(anonymous namespace)::OCSPRequestSession::CancelURLRequest() [0x13142a7]
	(anonymous namespace)::OCSPRequestSession::WillDestroyCurrentMessageLoop() [0x1314b30]
	MessageLoop::~MessageLoop() [0xf421ab]
	base::Thread::ThreadMain() [0xf71395]
	ThreadFunc() [0xf54338]
	start_thread [0x7ff66c6d83f7]
	0x7ff669d5cbbd

This is still on the IO thread, but the DCHECK fails because at the
time the DestructionObservers are run Thread::message_loop() returns
NULL, which means the DCHECK fails. I've fixed this by changing the
DCHECK to pass if the current thread's message loop is NULL. I feel a
bit quesy about this as it seems a bit fragile (well, all this code is
fragile). I would be inclined to make Thread::message_loop() return
the MessageLoop until after the destructor has run, but this seems
equally risky. Let me know what you prefer.

BUG=52022
TEST=none

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

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

1 files changed, 5 insertions, 1 deletions

diff --git a/chrome/browser/net/chrome_net_log.cc b/chrome/browser/net/chrome_net_log.cc
index dd9c62f..1003dfa 100644
--- a/chrome/browser/net/chrome_net_log.cc
+++ b/chrome/browser/net/chrome_net_log.cc
@@ -46,7 +46,11 @@ void ChromeNetLog::AddEntry(EventType type,
                             const Source& source,
                             EventPhase phase,
                             EventParameters* params) {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
+  // This must be invoked when we're on the IO thread, or if the IO thread's
+  // message loop isn't valid. The later can happen if this is invoked when the
+  // IOThread is shuting down the MessageLoop.
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO) ||
+         !ChromeThread::IsMessageLoopValid(ChromeThread::IO));
 
   // Notify all of the log observers.
   FOR_EACH_OBSERVER(Observer, observers_,