From 1e744f200179976645ef75859569eaa475b820be Mon Sep 17 00:00:00 2001 From: "aa@chromium.org" Date: Wed, 8 Apr 2009 01:00:17 +0000 Subject: 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 --- chrome/browser/net/chrome_url_request_context.cc | 26 ++++++++++++++++++------ chrome/browser/net/chrome_url_request_context.h | 2 ++ 2 files changed, 22 insertions(+), 6 deletions(-) (limited to 'chrome/browser/net') 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(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(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 cookie_db_; PrefService* prefs_; + bool is_media_; bool is_off_the_record_; + bool owns_http_transaction_factory_; }; -- cgit v1.1