Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(95)

Unified Diff: chrome/browser/net/chrome_url_request_context.cc

Issue 165430: Reference count ProxyService.... (Closed) Base URL: svn://chrome-svn/chrome/trunk/src/
Patch Set: Sync Created 11 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | chrome/browser/net/resolve_proxy_msg_helper.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/net/chrome_url_request_context.cc
===================================================================
--- chrome/browser/net/chrome_url_request_context.cc (revision 23574)
+++ chrome/browser/net/chrome_url_request_context.cc (working copy)
@@ -196,8 +196,11 @@
ChromeURLRequestContext* context = new ChromeURLRequestContext(profile);
// Share the same proxy service and host resolver as the original profile.
- // This proxy service's lifespan is dependent on the lifespan of the original
- // profile which we reference (see above).
+ // TODO(eroman): although ProxyService is reference counted, this sharing
+ // still has a subtle dependency on the lifespan of the original profile --
+ // ProxyService holds a (non referencing) pointer to the URLRequestContext
+ // it uses to download PAC scripts, which in this case is the original
+ // profile...
context->host_resolver_ =
profile->GetOriginalProfile()->GetRequestContext()->host_resolver();
context->proxy_service_ =
@@ -513,11 +516,11 @@
// Do not delete the cookie store in the case of the media context, as it is
// owned by the original context.
+ // TODO(eroman): The lifetime expectation of cookie_store_ is not right.
+ // The assumption here is that the original request context (which owns
+ // cookie_store_) is going to outlive the media context (which uses it).
+ // However based on the destruction order of profiles this is not true.
+ // http://crbug.com/15289.
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_;
}
« no previous file with comments | « no previous file | chrome/browser/net/resolve_proxy_msg_helper.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698