Skip to content

GitLab

T
tangled
Commit 42aed890 authored by willchan@chromium.org's avatar willchan@chromium.org
Browse files
Options

Fix up ordering in ProfileIOData. 

It's important to destroy the ResourceContext before we start destroying the URLRequestContext and the members it owns, because destroying the ResourceContext will cancel the pending URLRequests for that ResourceContext in the ResourceDispatcherHost. Those URLRequests may reference the URLRequestContexts in their destructors, so we need to make sure ResourceContext is destroyed first.

While doing this, I did some other cleanup:
  * discovered that we could eliminate HostContentSettingsMap from ProfileIOData, w00t!
  * reordered URLRequestContext after the members it points to, so it gets destroyed before they get destroyed
  * moved the virtuals in ProfileIOData::ResourceContext to be public, since the parent class changed them to be public

BUG=90971
TEST=none


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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@133793 0039d316-1c4b-4281-b951-d872f2087c98
parent 6dc408a4
Hide whitespace changes
Inline Side-by-side
Showing with 43 additions and 27 deletions
chrome/browser/profiles/off_the_record_profile_io_data.cc
View file @ 42aed890
......@@ -146,7 +146,9 @@ void OffTheRecordProfileIOData::Handle::LazyInitialize() const {
OffTheRecordProfileIOData::OffTheRecordProfileIOData()
: ProfileIOData(true) {}
OffTheRecordProfileIOData::~OffTheRecordProfileIOData() {}
OffTheRecordProfileIOData::~OffTheRecordProfileIOData() {
DestroyResourceContext();
}
void OffTheRecordProfileIOData::LazyInitializeInternal(
ProfileParams* profile_params) const {
......
chrome/browser/profiles/profile_impl_io_data.cc
View file @ 42aed890
......@@ -245,6 +245,8 @@ ProfileImplIOData::ProfileImplIOData()
: ProfileIOData(false),
clear_local_state_on_exit_(false) {}
ProfileImplIOData::~ProfileImplIOData() {
DestroyResourceContext();
if (media_request_context_)
media_request_context_->AssertNoURLRequests();
}
......
chrome/browser/profiles/profile_impl_io_data.h
View file @ 42aed890
......@@ -141,14 +141,14 @@ class ProfileImplIOData : public ProfileIOData {
// Lazy initialization params.
mutable scoped_ptr<LazyParams> lazy_params_;
mutable scoped_refptr<ChromeURLRequestContext> media_request_context_;
mutable scoped_ptr<net::HttpTransactionFactory> main_http_factory_;
mutable scoped_ptr<net::HttpTransactionFactory> media_http_factory_;
mutable scoped_ptr<net::FtpTransactionFactory> ftp_factory_;
mutable scoped_ptr<chrome_browser_net::Predictor> predictor_;
mutable scoped_refptr<ChromeURLRequestContext> media_request_context_;
// Parameters needed for isolated apps.
FilePath app_path_;
mutable bool clear_local_state_on_exit_;
......
chrome/browser/profiles/profile_io_data.cc
View file @ 42aed890
......@@ -188,7 +188,6 @@ void ProfileIOData::InitializeOnUIThread(Profile* profile) {
params->io_thread = g_browser_process->io_thread();
params->host_content_settings_map = profile->GetHostContentSettingsMap();
params->cookie_settings = CookieSettings::Factory::GetForProfile(profile);
params->ssl_config_service = profile->GetSSLConfigService();
base::Callback<Profile*(void)> profile_getter =
......@@ -258,7 +257,8 @@ ProfileIOData::ProfileParams::~ProfileParams() {}
ProfileIOData::ProfileIOData(bool is_incognito)
: initialized_(false),
ALLOW_THIS_IN_INITIALIZER_LIST(resource_context_(this)),
ALLOW_THIS_IN_INITIALIZER_LIST(
resource_context_(new ResourceContext(this))),
initialized_on_UI_thread_(false) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
}
......@@ -317,7 +317,7 @@ bool ProfileIOData::IsHandledURL(const GURL& url) {
}
content::ResourceContext* ProfileIOData::GetResourceContext() const {
return &resource_context_;
return resource_context_.get();
}
ChromeURLDataManagerBackend*
......@@ -367,10 +367,6 @@ ExtensionInfoMap* ProfileIOData::GetExtensionInfoMap() const {
return extension_info_map_;
}
HostContentSettingsMap* ProfileIOData::GetHostContentSettingsMap() const {
return host_content_settings_map_;
}
CookieSettings* ProfileIOData::GetCookieSettings() const {
return cookie_settings_;
}
......@@ -503,15 +499,14 @@ void ProfileIOData::LazyInitialize() const {
#endif // defined(OS_CHROMEOS) && !defined(GOOGLE_CHROME_BUILD)
// Take ownership over these parameters.
host_content_settings_map_ = profile_params_->host_content_settings_map;
cookie_settings_ = profile_params_->cookie_settings;
#if defined(ENABLE_NOTIFICATIONS)
notification_service_ = profile_params_->notification_service;
#endif
extension_info_map_ = profile_params_->extension_info_map;
resource_context_.host_resolver_ = io_thread_globals->host_resolver.get();
resource_context_.request_context_ = main_request_context_;
resource_context_->host_resolver_ = io_thread_globals->host_resolver.get();
resource_context_->request_context_ = main_request_context_;
LazyInitializeInternal(profile_params_.get());
......@@ -547,3 +542,7 @@ void ProfileIOData::set_server_bound_cert_service(
net::ServerBoundCertService* server_bound_cert_service) const {
server_bound_cert_service_.reset(server_bound_cert_service);
}
void ProfileIOData::DestroyResourceContext() {
resource_context_.reset();
}
chrome/browser/profiles/profile_io_data.h
View file @ 42aed890
......@@ -85,7 +85,6 @@ class ProfileIOData {
// with a content::ResourceContext, and they want access to Chrome data for
// that profile.
ExtensionInfoMap* GetExtensionInfoMap() const;
HostContentSettingsMap* GetHostContentSettingsMap() const;
CookieSettings* GetCookieSettings() const;
#if defined(ENABLE_NOTIFICATIONS)
......@@ -142,7 +141,6 @@ class ProfileIOData {
std::string accept_charset;
std::string referrer_charset;
IOThread* io_thread;
scoped_refptr<HostContentSettingsMap> host_content_settings_map;
scoped_refptr<CookieSettings> cookie_settings;
scoped_refptr<net::SSLConfigService> ssl_config_service;
scoped_refptr<net::CookieMonster::Delegate> cookie_monster_delegate;
......@@ -217,18 +215,24 @@ class ProfileIOData {
return main_request_context_;
}
// Destroys the ResourceContext first, to cancel any URLRequests that are
// using it still, before we destroy the member variables that those
// URLRequests may be accessing.
void DestroyResourceContext();
private:
class ResourceContext : public content::ResourceContext {
public:
explicit ResourceContext(ProfileIOData* io_data);
virtual ~ResourceContext();
private:
friend class ProfileIOData;
// ResourceContext implementation:
virtual net::HostResolver* GetHostResolver() OVERRIDE;
virtual net::URLRequestContext* GetRequestContext() OVERRIDE;
private:
friend class ProfileIOData;
void EnsureInitialized();
ProfileIOData* const io_data_;
......@@ -263,6 +267,19 @@ class ProfileIOData {
scoped_refptr<ChromeURLRequestContext> main_context,
const std::string& app_id) const = 0;
// The order *DOES* matter for the majority of these member variables, so
// don't move them around unless you know what you're doing!
// General rules:
// * ResourceContext references the URLRequestContexts, so
// URLRequestContexts must outlive ResourceContext, hence ResourceContext
// should be destroyed first.
// * URLRequestContexts reference a whole bunch of members, so
// URLRequestContext needs to be destroyed before them.
// * Therefore, ResourceContext should be listed last, and then the
// URLRequestContexts, and then the URLRequestContext members.
// * Note that URLRequestContext members have a directed dependency graph
// too, so they must themselves be ordered correctly.
// Tracks whether or not we've been lazily initialized.
mutable bool initialized_;
......@@ -293,19 +310,10 @@ class ProfileIOData {
mutable scoped_ptr<chrome_browser_net::HttpServerPropertiesManager>
http_server_properties_manager_;
// Pointed to by ResourceContext.
// TODO(willchan): Remove from ResourceContext.
mutable scoped_refptr<ExtensionInfoMap> extension_info_map_;
mutable scoped_refptr<HostContentSettingsMap> host_content_settings_map_;
mutable scoped_refptr<CookieSettings> cookie_settings_;
#if defined(ENABLE_NOTIFICATIONS)
mutable DesktopNotificationService* notification_service_;
#endif
mutable ResourceContext resource_context_;
mutable scoped_ptr<TransportSecurityPersister>
transport_security_persister_;
......@@ -316,6 +324,11 @@ class ProfileIOData {
// One AppRequestContext per isolated app.
mutable AppRequestContextMap app_request_context_map_;
mutable scoped_ptr<ResourceContext> resource_context_;
mutable scoped_refptr<ExtensionInfoMap> extension_info_map_;
mutable scoped_refptr<CookieSettings> cookie_settings_;
// TODO(jhawkins): Remove once crbug.com/102004 is fixed.
bool initialized_on_UI_thread_;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment