Make Profile own ProfileIOData.

Notably this makes ChromeURLRequestContextGetter not own its ChromeURLRequestContext, and instead it keeps a base::WeakPtr. If anything is holding onto a ChromeURLRequestContextGetter longer than it should, and tries to use it, then it will crash. Currently no tests fail with this assumption, so hopefully this will prevent regressions. All ChromeURLRequestContextGetters should not outlive the Profile to which they belong.

Note that this will cause problems if there are any URLRequests which outlive the Profile to which they belong, as they will reference URLRequestContexts which point to deleted memory. AFAICT this never happens, and I will enforce this later on by making URLRequests acquire weak pointers to URLRequestContexts. After that, I can stop refcounting URLRequestContext.

I'm hopeful this changelist will fix a large number of leaks in browser_tests. Removing the refcounting of ProfileIOData will bring sanity to the lifecycle management of ProfileIOData and a large number of objects it owns. I've used base::WeakPtr in a number of places in order to prevent use-after-free bugs and force them to be NULL dereferences (crashes). In previous changelists, I've flushed out bugs revealed by this behavior. So currently, this changelist is clean on the trybots, although it's possible new code paths will be discovered via crash reports.

Next steps:
* URLRequests use base::WeakPtr<URLRequestContext>
* Fix all other URLRequestContextGetters not to acquire references to URLRequestContext.
* Stop refcounting URLRequestContext.

BUG=none
TEST=none


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

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

1 files changed, 14 insertions, 47 deletions

diff --git a/chrome/browser/profiles/profile_io_data.h b/chrome/browser/profiles/profile_io_data.h
index b8478be..69c55c9 100644
--- a/chrome/browser/profiles/profile_io_data.h
+++ b/chrome/browser/profiles/profile_io_data.h
@@ -58,19 +58,13 @@ class DatabaseTracker;
 
 // Conceptually speaking, the ProfileIOData represents data that lives on the IO
 // thread that is owned by a Profile, such as, but not limited to, network
-// objects like CookieMonster, HttpTransactionFactory, etc. The Profile
-// implementation will maintain a reference to the ProfileIOData. The
-// ProfileIOData will originally own a reference to the ChromeURLRequestContexts
-// that reference its members. When an accessor for a ChromeURLRequestContext is
-// invoked, then ProfileIOData will release its reference to the
-// ChromeURLRequestContext and the ChromeURLRequestContext will acquire a
-// reference to the ProfileIOData, so they exchange ownership. This is done
-// because it's possible for a context's accessor never to be invoked, so this
-// ownership reversal prevents shutdown leaks. ProfileIOData will lazily
-// initialize its members on the first invocation of a ChromeURLRequestContext
-// accessor.
-class ProfileIOData : public base::RefCountedThreadSafe<ProfileIOData> {
+// objects like CookieMonster, HttpTransactionFactory, etc.  Profile owns
+// ProfileIOData, but will make sure to delete it on the IO thread (except
+// possibly in unit tests where there is no IO thread).
+class ProfileIOData {
  public:
+  virtual ~ProfileIOData();
+
   // Returns true if |scheme| is handled in Chrome, or by default handlers in
   // net::URLRequest.
   static bool IsHandledProtocol(const std::string& scheme);
@@ -105,7 +99,7 @@ class ProfileIOData : public base::RefCountedThreadSafe<ProfileIOData> {
   }
 
   ChromeURLRequestContext* extensions_request_context() const {
-    return weak_extensions_request_context_.get();
+    return extensions_request_context_.get();
   }
 
   BooleanPrefMember* safe_browsing_enabled() const {
@@ -113,35 +107,15 @@ class ProfileIOData : public base::RefCountedThreadSafe<ProfileIOData> {
   }
 
  protected:
-  friend class base::RefCountedThreadSafe<ProfileIOData>;
-
-  class RequestContext : public ChromeURLRequestContext {
+  class AppRequestContext : public ChromeURLRequestContext {
    public:
-    RequestContext();
-    virtual ~RequestContext();
-
-    // Setter is used to transfer ownership of the ProfileIOData to the context.
-    void set_profile_io_data(ProfileIOData* profile_io_data) {
-      profile_io_data_ = profile_io_data;
-    }
-
-   protected:
-    ProfileIOData* profile_io_data() { return profile_io_data_; }
-
-   private:
-    scoped_refptr<ProfileIOData> profile_io_data_;
-  };
-
-  class AppRequestContext : public RequestContext {
-   public:
-    explicit AppRequestContext(const std::string& app_id);
+    AppRequestContext();
     virtual ~AppRequestContext();
 
     void SetCookieStore(net::CookieStore* cookie_store);
     void SetHttpTransactionFactory(net::HttpTransactionFactory* http_factory);
 
    private:
-    const std::string app_id_;
     scoped_refptr<net::CookieStore> cookie_store_;
     scoped_ptr<net::HttpTransactionFactory> http_factory_;
   };
@@ -183,7 +157,6 @@ class ProfileIOData : public base::RefCountedThreadSafe<ProfileIOData> {
   };
 
   explicit ProfileIOData(bool is_incognito);
-  virtual ~ProfileIOData();
 
   void InitializeProfileParams(Profile* profile);
   void ApplyProfileParamsToContext(ChromeURLRequestContext* context) const;
@@ -237,7 +210,7 @@ class ProfileIOData : public base::RefCountedThreadSafe<ProfileIOData> {
     const ProfileIOData* const io_data_;
   };
 
-  typedef base::hash_map<std::string, ChromeURLRequestContext*>
+  typedef base::hash_map<std::string, scoped_refptr<ChromeURLRequestContext> >
       AppRequestContextMap;
 
   // --------------------------------------------
@@ -250,7 +223,7 @@ class ProfileIOData : public base::RefCountedThreadSafe<ProfileIOData> {
 
   // Does an on-demand initialization of a RequestContext for the given
   // isolated app.
-  virtual scoped_refptr<RequestContext> InitializeAppRequestContext(
+  virtual scoped_refptr<ChromeURLRequestContext> InitializeAppRequestContext(
       scoped_refptr<ChromeURLRequestContext> main_context,
       const std::string& app_id) const = 0;
 
@@ -258,7 +231,7 @@ class ProfileIOData : public base::RefCountedThreadSafe<ProfileIOData> {
   // context from ProfileIOData to the URLRequestContextGetter.
   virtual scoped_refptr<ChromeURLRequestContext>
       AcquireMediaRequestContext() const = 0;
-  virtual scoped_refptr<RequestContext>
+  virtual scoped_refptr<ChromeURLRequestContext>
       AcquireIsolatedAppRequestContext(
           scoped_refptr<ChromeURLRequestContext> main_context,
           const std::string& app_id) const = 0;
@@ -301,17 +274,11 @@ class ProfileIOData : public base::RefCountedThreadSafe<ProfileIOData> {
 
   // These are only valid in between LazyInitialize() and their accessor being
   // called.
-  mutable scoped_refptr<RequestContext> main_request_context_;
-  mutable scoped_refptr<RequestContext> extensions_request_context_;
+  mutable scoped_refptr<ChromeURLRequestContext> main_request_context_;
+  mutable scoped_refptr<ChromeURLRequestContext> extensions_request_context_;
   // One AppRequestContext per isolated app.
   mutable AppRequestContextMap app_request_context_map_;
 
-  // Weak pointers to the request contexts. Only valid after LazyInitialize.
-  // These are weak so that they don't hold a reference to the RequestContext,
-  // because that holds a reference back to ProfileIOData.
-  mutable base::WeakPtr<ChromeURLRequestContext>
-      weak_extensions_request_context_;
-
   DISALLOW_COPY_AND_ASSIGN(ProfileIOData);
 };