This CL changes the CookieStore obect to be a refcounted object to get a better handle on its lifetime as there are cases where this object is handed out to URLRequestContext instances which outlive the URLRequestContext object which created it.

Partial fix for http://code.google.com/p/chromium/issues/detail?id=15289

Bug=15289

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

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

6 files changed, 18 insertions, 29 deletions

diff --git a/chrome/browser/automation/automation_profile_impl.cc b/chrome/browser/automation/automation_profile_impl.cc
index 1cba8d6..7ea154c 100644
--- a/chrome/browser/automation/automation_profile_impl.cc
+++ b/chrome/browser/automation/automation_profile_impl.cc
@@ -113,10 +113,11 @@ void AutomationProfileImpl::Initialize(Profile* original_profile,
   original_profile_ = original_profile;
 
   URLRequestContext* original_context = original_profile_->GetRequestContext();
-  net::CookieStore* original_cookie_store = original_context->cookie_store();
-  alternate_cookie_store_.reset(new AutomationCookieStore(this,
-                                                          original_cookie_store,
-                                                          automation_client));
+  scoped_refptr<net::CookieStore> original_cookie_store =
+      original_context->cookie_store();
+  alternate_cookie_store_ = new AutomationCookieStore(this,
+                                                      original_cookie_store,
+                                                      automation_client);
   alternate_reqeust_context_ = new AutomationURLRequestContext(
       original_context, alternate_cookie_store_.get());
 }
diff --git a/chrome/browser/automation/automation_profile_impl.h b/chrome/browser/automation/automation_profile_impl.h
index 7a22fc3..c9aedbe 100644
--- a/chrome/browser/automation/automation_profile_impl.h
+++ b/chrome/browser/automation/automation_profile_impl.h
@@ -202,7 +202,7 @@ class AutomationProfileImpl : public Profile {
 
  protected:
   Profile* original_profile_;
-  scoped_ptr<net::CookieStore> alternate_cookie_store_;
+  scoped_refptr<net::CookieStore> alternate_cookie_store_;
   scoped_refptr<URLRequestContext> alternate_reqeust_context_;
   int tab_handle_;
 
diff --git a/chrome/browser/gtk/options/cookies_view_unittest.cc b/chrome/browser/gtk/options/cookies_view_unittest.cc
index e12969e..83b67de 100644
--- a/chrome/browser/gtk/options/cookies_view_unittest.cc
+++ b/chrome/browser/gtk/options/cookies_view_unittest.cc
@@ -22,9 +22,6 @@ class TestURLRequestContext : public URLRequestContext {
   TestURLRequestContext() {
     cookie_store_ = new net::CookieMonster();
   }
-  virtual ~TestURLRequestContext() {
-    delete cookie_store_;
-  }
 };
 
 class CookieTestingProfile : public TestingProfile {
diff --git a/chrome/browser/net/chrome_url_request_context.cc b/chrome/browser/net/chrome_url_request_context.cc
index 0bc5572..ed69b03 100644
--- a/chrome/browser/net/chrome_url_request_context.cc
+++ b/chrome/browser/net/chrome_url_request_context.cc
@@ -155,10 +155,12 @@ ChromeURLRequestContext* ChromeURLRequestContext::CreateOriginal(
   // setup cookie store
   if (!context->cookie_store_) {
     DCHECK(!cookie_store_path.empty());
-    context->cookie_db_.reset(new SQLitePersistentCookieStore(
-        cookie_store_path.ToWStringHack(),
-        g_browser_process->db_thread()->message_loop()));
-    context->cookie_store_ = new net::CookieMonster(context->cookie_db_.get());
+
+    scoped_refptr<SQLitePersistentCookieStore> cookie_db =
+        new SQLitePersistentCookieStore(
+            cookie_store_path.ToWStringHack(),
+            g_browser_process->db_thread()->message_loop());
+    context->cookie_store_ = new net::CookieMonster(cookie_db.get());
   }
 
   return context;
@@ -180,11 +182,12 @@ ChromeURLRequestContext* ChromeURLRequestContext::CreateOriginalForExtensions(
 
   // All we care about for extensions is the cookie store.
   DCHECK(!cookie_store_path.empty());
-  context->cookie_db_.reset(new SQLitePersistentCookieStore(
-      cookie_store_path.ToWStringHack(),
-      g_browser_process->db_thread()->message_loop()));
-  net::CookieMonster* cookie_monster = new net::CookieMonster(
-      context->cookie_db_.get());
+
+  scoped_refptr<SQLitePersistentCookieStore> cookie_db =
+      new SQLitePersistentCookieStore(
+          cookie_store_path.ToWStringHack(),
+          g_browser_process->db_thread()->message_loop());
+  net::CookieMonster* cookie_monster = new net::CookieMonster(cookie_db.get());
 
   // Enable cookies for extension URLs only.
   const char* schemes[] = {chrome::kExtensionScheme};
@@ -542,14 +545,4 @@ ChromeURLRequestContext::~ChromeURLRequestContext() {
 
   delete ftp_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.
-  // 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_;
 }
diff --git a/chrome/browser/net/chrome_url_request_context.h b/chrome/browser/net/chrome_url_request_context.h
index d0303d7..067cf10a 100644
--- a/chrome/browser/net/chrome_url_request_context.h
+++ b/chrome/browser/net/chrome_url_request_context.h
@@ -127,7 +127,6 @@ class ChromeURLRequestContext : public URLRequestContext,
 
   scoped_refptr<ChromeAppCacheService> appcache_service_;
 
-  scoped_ptr<SQLitePersistentCookieStore> cookie_db_;
   PrefService* prefs_;
   const Blacklist* blacklist_;
   bool is_media_;
diff --git a/chrome/browser/sync/glue/http_bridge.cc b/chrome/browser/sync/glue/http_bridge.cc
index dbfb072..54e93ed 100644
--- a/chrome/browser/sync/glue/http_bridge.cc
+++ b/chrome/browser/sync/glue/http_bridge.cc
@@ -79,7 +79,6 @@ HttpBridge::RequestContext::RequestContext(
 }
 
 HttpBridge::RequestContext::~RequestContext() {
-  delete cookie_store_;
   delete http_transaction_factory_;
 }