Make the URLFetcher observe IO thread shutdown and auto-cancel its request, if any.

BUG=41547
TEST=none
Review URL: http://codereview.chromium.org/2849003

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

4 files changed, 18 insertions, 16 deletions

diff --git a/chrome/browser/geolocation/network_location_provider_unittest.cc b/chrome/browser/geolocation/network_location_provider_unittest.cc
index c3900f1..4e6fdbd 100644
--- a/chrome/browser/geolocation/network_location_provider_unittest.cc
+++ b/chrome/browser/geolocation/network_location_provider_unittest.cc
@@ -116,7 +116,6 @@ class GeolocationNetworkProviderTest : public testing::Test {
     WifiDataProvider::ResetFactory();
     RadioDataProvider::ResetFactory();
     URLFetcher::set_factory(NULL);
-    base::LeakTracker<URLFetcher>::CheckForLeaks();
   }
 
   LocationProviderBase* CreateProvider(bool set_permission_granted) {
diff --git a/chrome/browser/io_thread.cc b/chrome/browser/io_thread.cc
index 0382ec4d..5dfd284 100644
--- a/chrome/browser/io_thread.cc
+++ b/chrome/browser/io_thread.cc
@@ -183,17 +183,7 @@ void IOThread::CleanUp() {
   delete globals_;
   globals_ = NULL;
 
-  // URLFetcher and URLRequest instances must NOT outlive the IO thread.
-  //
-  // Strictly speaking, URLFetcher's CheckForLeaks() should be done on the
-  // UI thread. However, since there _shouldn't_ be any instances left
-  // at this point, it shouldn't be a race.
-  //
-  // We check URLFetcher first, since if it has leaked then an associated
-  // URLRequest will also have leaked. However it is more useful to
-  // crash showing the callstack of URLFetcher's allocation than its
-  // URLRequest member.
-  base::LeakTracker<URLFetcher>::CheckForLeaks();
+  // URLRequest instances must NOT outlive the IO thread.
   base::LeakTracker<URLRequest>::CheckForLeaks();
 
   BrowserProcessSubThread::CleanUp();
diff --git a/chrome/common/net/url_fetcher.cc b/chrome/common/net/url_fetcher.cc
index c01eb6a..a74255b 100644
--- a/chrome/common/net/url_fetcher.cc
+++ b/chrome/common/net/url_fetcher.cc
@@ -24,6 +24,7 @@ bool URLFetcher::g_interception_enabled = false;
 
 class URLFetcher::Core
     : public base::RefCountedThreadSafe<URLFetcher::Core>,
+      public MessageLoop::DestructionObserver,
       public URLRequest::Delegate {
  public:
   // For POST requests, set |content_type| to the MIME type of the content
@@ -47,7 +48,11 @@ class URLFetcher::Core
   // safe to call this multiple times.
   void Stop();
 
-  // URLRequest::Delegate implementations
+  // MessageLoop::DestructionObserver implementation.  We are only registered as
+  // a DestructionObserver when |request_| exists.
+  virtual void WillDestroyCurrentMessageLoop();
+
+  // URLRequest::Delegate implementation.
   virtual void OnResponseStarted(URLRequest* request);
   virtual void OnReadCompleted(URLRequest* request, int bytes_read);
 
@@ -168,6 +173,13 @@ void URLFetcher::Core::Stop() {
   }
 }
 
+void URLFetcher::Core::WillDestroyCurrentMessageLoop() {
+  CancelURLRequest();
+  // Don't bother to try and notify the delegate thread portion of this object,
+  // since if the IO thread is shutting down, everything is shutting down, and
+  // we just want to avoid leaks.
+}
+
 void URLFetcher::Core::OnResponseStarted(URLRequest* request) {
   DCHECK(request == request_);
   DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
@@ -207,6 +219,7 @@ void URLFetcher::Core::OnReadCompleted(URLRequest* request, int bytes_read) {
         this, &Core::OnCompletedURLRequest, request_->status()));
     delete request_;
     request_ = NULL;
+    MessageLoop::current()->RemoveDestructionObserver(this);
   }
 }
 
@@ -222,6 +235,8 @@ void URLFetcher::Core::StartURLRequest() {
   CHECK(request_context_getter_);
   DCHECK(!request_);
 
+  MessageLoop::current()->AddDestructionObserver(this);
+
   request_ = new URLRequest(original_url_, this);
   int flags = request_->load_flags() | load_flags_;
   if (!g_interception_enabled) {
@@ -266,6 +281,7 @@ void URLFetcher::Core::CancelURLRequest() {
     request_->Cancel();
     delete request_;
     request_ = NULL;
+    MessageLoop::current()->RemoveDestructionObserver(this);
   }
   // Release the reference to the request context. There could be multiple
   // references to URLFetcher::Core at this point so it may take a while to
diff --git a/chrome/common/net/url_fetcher.h b/chrome/common/net/url_fetcher.h
index 763a660..9572478 100644
--- a/chrome/common/net/url_fetcher.h
+++ b/chrome/common/net/url_fetcher.h
@@ -12,7 +12,6 @@
 
 #include <string>
 
-#include "base/leak_tracker.h"
 #include "base/message_loop.h"
 #include "base/ref_counted.h"
 #include "base/time.h"
@@ -177,8 +176,6 @@ class URLFetcher {
 
   static Factory* factory_;
 
-  base::LeakTracker<URLFetcher> leak_tracker_;
-
   // If |automatically_retry_on_5xx_| is false, 5xx responses will be
   // propagated to the observer, if it is true URLFetcher will automatically
   // re-execute the request, after the back-off delay has expired.