hclam: picked you for this review based on the revision log of ChromeURLRequestContext, but feel free to redirect if there is someone better.

Fix leak of media_request_context_. This is kind of a nasty
fix though. I think that ChromeURLRequestContext needs to be
refactored more, but not sure exactly how right now. If you
don't like this fix, I won't feel bad nuking it and letting
someone working on this area handle it.

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

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

2 files changed, 22 insertions, 6 deletions

diff --git a/chrome/browser/net/chrome_url_request_context.cc b/chrome/browser/net/chrome_url_request_context.cc
index 4c14381a..6d9dea8 100644
--- a/chrome/browser/net/chrome_url_request_context.cc
+++ b/chrome/browser/net/chrome_url_request_context.cc
@@ -82,6 +82,7 @@ ChromeURLRequestContext* ChromeURLRequestContext::CreateOriginal(
         record_mode ? net::HttpCache::RECORD : net::HttpCache::PLAYBACK);
   }
   context->http_transaction_factory_ = cache;
+  context->owns_http_transaction_factory_ = true;
 
   // The kNewFtp switch is Windows specific only because we have multiple FTP
   // implementations on Windows.
@@ -125,6 +126,7 @@ ChromeURLRequestContext* ChromeURLRequestContext::CreateOffTheRecord(
 
   context->http_transaction_factory_ =
       new net::HttpCache(context->proxy_service_, 0);
+  context->owns_http_transaction_factory_ = true;
   context->cookie_store_ = new net::CookieMonster;
 
   return context;
@@ -146,6 +148,8 @@ ChromeURLRequestContext* ChromeURLRequestContext::CreateRequestContextForMedia(
   URLRequestContext* original_context =
       profile->GetOriginalProfile()->GetRequestContext();
   ChromeURLRequestContext* context = new ChromeURLRequestContext(profile);
+  context->is_media_ = true;
+
   // Share the same proxy service of the common profile.
   context->proxy_service_ = original_context->proxy_service();
   // Also share the cookie store of the common profile.
@@ -167,11 +171,13 @@ ChromeURLRequestContext* ChromeURLRequestContext::CreateRequestContextForMedia(
         static_cast<net::HttpNetworkLayer*>(original_cache->network_layer());
     cache = new net::HttpCache(original_network_layer->GetSession(),
         disk_cache_path.ToWStringHack(), 240000000);
+    context->owns_http_transaction_factory_ = false;
   } else {
     // If original HttpCache doesn't exist, simply construct one with a whole
     // new set of network stack.
     cache = new net::HttpCache(original_context->proxy_service(),
         disk_cache_path.ToWStringHack(), 240000000);
+    context->owns_http_transaction_factory_ = true;
   }
 
   // Set the cache type to media.
@@ -187,7 +193,9 @@ ChromeURLRequestContext* ChromeURLRequestContext::CreateRequestContextForMedia(
 
 ChromeURLRequestContext::ChromeURLRequestContext(Profile* profile)
     : prefs_(profile->GetPrefs()),
-      is_off_the_record_(profile->IsOffTheRecord()) {
+      is_media_(false),
+      is_off_the_record_(profile->IsOffTheRecord()),
+      owns_http_transaction_factory_(false) {
   // Set up Accept-Language and Accept-Charset header values
   accept_language_ = net::HttpUtil::GenerateAcceptLanguageHeader(
       WideToASCII(prefs_->GetString(prefs::kAcceptLanguages)));
@@ -310,12 +318,18 @@ ChromeURLRequestContext::~ChromeURLRequestContext() {
       Source<URLRequestContext>(this),
       NotificationService::NoDetails());
 
-  delete cookie_store_;
   delete ftp_transaction_factory_;
-  delete http_transaction_factory_;
 
-  // Do not delete the proxy service in the case of OTR, as it is owned by the
-  // original URLRequestContext.
-  if (!is_off_the_record_)
+  if (owns_http_transaction_factory_)
+    delete http_transaction_factory_;
+
+  // Do not delete the cookie store in the case of the media context, as it is
+  // owned by the original context.
+  if (!is_media_)
+    delete cookie_store_;
+
+  // Do not delete the proxy service in the case of OTR or media contexts, as
+  // it is owned by the original URLRequestContext.
+  if (!is_off_the_record_ && !is_media_)
     delete proxy_service_;
 }
diff --git a/chrome/browser/net/chrome_url_request_context.h b/chrome/browser/net/chrome_url_request_context.h
index 7f31ef2..3419df59 100644
--- a/chrome/browser/net/chrome_url_request_context.h
+++ b/chrome/browser/net/chrome_url_request_context.h
@@ -93,5 +93,7 @@ class ChromeURLRequestContext : public URLRequestContext,
 
   scoped_ptr<SQLitePersistentCookieStore> cookie_db_;
   PrefService* prefs_;
+  bool is_media_;
   bool is_off_the_record_;
+  bool owns_http_transaction_factory_;
 };