Checks to enforce relative lifetimes for SdchManager.

BUG=454198
R=mmenke@chromium.org

Review URL: https://codereview.chromium.org/998803003

Cr-Commit-Position: refs/heads/master@{#320164}

6 files changed, 36 insertions, 8 deletions

diff --git a/net/base/sdch_manager.cc b/net/base/sdch_manager.cc
index bcb5e32..34fc76a 100644
--- a/net/base/sdch_manager.cc
+++ b/net/base/sdch_manager.cc
@@ -243,7 +243,7 @@ void SdchManager::DictionarySet::AddDictionary(
   dictionaries_[server_hash] = dictionary;
 }
 
-SdchManager::SdchManager() {
+SdchManager::SdchManager() : factory_(this) {
   DCHECK(thread_checker_.CalledOnValidThread());
 }
 
@@ -620,6 +620,11 @@ SdchManager::CreateEmptyDictionarySetForTesting() {
   return scoped_ptr<DictionarySet>(new DictionarySet).Pass();
 }
 
+// For investigation of http://crbug.com/454198; remove when resolved.
+base::WeakPtr<SdchManager> SdchManager::GetWeakPtr() {
+  return factory_.GetWeakPtr();
+}
+
 // static
 void SdchManager::UrlSafeBase64Encode(const std::string& input,
                                       std::string* output) {
diff --git a/net/base/sdch_manager.h b/net/base/sdch_manager.h
index c83d3a4..93f8a74 100644
--- a/net/base/sdch_manager.h
+++ b/net/base/sdch_manager.h
@@ -23,6 +23,7 @@
 #include "base/gtest_prod_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
 #include "base/threading/thread_checker.h"
 #include "base/time/time.h"
@@ -292,6 +293,9 @@ class NET_EXPORT SdchManager {
 
   static scoped_ptr<DictionarySet> CreateEmptyDictionarySetForTesting();
 
+  // For investigation of http://crbug.com/454198; remove when resolved.
+  base::WeakPtr<SdchManager> GetWeakPtr();
+
  private:
   struct BlacklistInfo {
     BlacklistInfo() : count(0), exponential_count(0), reason(SDCH_OK) {}
@@ -338,6 +342,8 @@ class NET_EXPORT SdchManager {
 
   base::ThreadChecker thread_checker_;
 
+  base::WeakPtrFactory<SdchManager> factory_;
+
   DISALLOW_COPY_AND_ASSIGN(SdchManager);
 };
 
diff --git a/net/sdch/sdch_owner.cc b/net/sdch/sdch_owner.cc
index b8d29d9..a8348707 100644
--- a/net/sdch/sdch_owner.cc
+++ b/net/sdch/sdch_owner.cc
@@ -267,7 +267,7 @@ const size_t SdchOwner::kMaxTotalDictionarySize = 20 * 1000 * 1000;
 const size_t SdchOwner::kMinSpaceForDictionaryFetch = 50 * 1000;
 
 SdchOwner::SdchOwner(SdchManager* sdch_manager, URLRequestContext* context)
-    : manager_(sdch_manager),
+    : manager_(sdch_manager->GetWeakPtr()),
       fetcher_(new SdchDictionaryFetcher(context)),
       total_dictionary_bytes_(0),
       clock_(new base::DefaultClock),
@@ -296,6 +296,7 @@ SdchOwner::~SdchOwner() {
   CHECK_EQ(0u, destroyed_);
   CHECK(clock_.get());
   clock_.reset();
+  CHECK(manager_.get());
 #endif
 
   for (DictionaryPreferenceIterator it(pref_store_); !it.IsAtEnd();
diff --git a/net/sdch/sdch_owner.h b/net/sdch/sdch_owner.h
index a187a6e..134bb86 100644
--- a/net/sdch/sdch_owner.h
+++ b/net/sdch/sdch_owner.h
@@ -10,6 +10,7 @@
 
 #include "base/memory/memory_pressure_listener.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
 #include "base/prefs/pref_store.h"
 #include "net/base/sdch_observer.h"
 #include "net/url_request/sdch_dictionary_fetcher.h"
@@ -115,7 +116,8 @@ class NET_EXPORT SdchOwner : public net::SdchObserver,
 
   bool IsPersistingDictionaries() const;
 
-  net::SdchManager* manager_;
+  // For investigation of http://crbug.com/454198; remove when resolved.
+  base::WeakPtr<net::SdchManager> manager_;
   scoped_ptr<net::SdchDictionaryFetcher> fetcher_;
 
   size_t total_dictionary_bytes_;
diff --git a/net/url_request/url_request_context.cc b/net/url_request/url_request_context.cc
index aa3ec91..5a9cf99 100644
--- a/net/url_request/url_request_context.cc
+++ b/net/url_request/url_request_context.cc
@@ -31,7 +31,9 @@ URLRequestContext::URLRequestContext()
       http_transaction_factory_(NULL),
       job_factory_(NULL),
       throttler_manager_(NULL),
-      sdch_manager_(NULL),
+      // For investigation of http://crbug.com/454198; restore when resolved.
+      // sdch_manager_(NULL),
+      have_sdch_manager_(false),
       url_requests_(new std::set<const URLRequest*>) {
 }
 
@@ -57,7 +59,9 @@ void URLRequestContext::CopyFrom(const URLRequestContext* other) {
   set_http_transaction_factory(other->http_transaction_factory_);
   set_job_factory(other->job_factory_);
   set_throttler_manager(other->throttler_manager_);
-  set_sdch_manager(other->sdch_manager_);
+  // For investigation of http://crbug.com/454198; remove when resolved.
+  CHECK(!other->have_sdch_manager_ || other->sdch_manager_.get());
+  set_sdch_manager(other->sdch_manager_.get());
   set_http_user_agent_settings(other->http_user_agent_settings_);
 }
 
diff --git a/net/url_request/url_request_context.h b/net/url_request/url_request_context.h
index 992f985..fc07098 100644
--- a/net/url_request/url_request_context.h
+++ b/net/url_request/url_request_context.h
@@ -20,6 +20,7 @@
 #include "net/base/net_export.h"
 #include "net/base/net_log.h"
 #include "net/base/request_priority.h"
+#include "net/base/sdch_manager.h"
 #include "net/http/http_network_session.h"
 #include "net/http/http_server_properties.h"
 #include "net/http/transport_security_state.h"
@@ -186,10 +187,17 @@ class NET_EXPORT URLRequestContext
 
   // May be NULL.
   SdchManager* sdch_manager() const {
-    return sdch_manager_;
+    // For investigation of http://crbug.com/454198; remove ?: when resolved.
+    CHECK(!have_sdch_manager_ || sdch_manager_.get());
+    return have_sdch_manager_ ? sdch_manager_.get() : NULL;
   }
   void set_sdch_manager(SdchManager* sdch_manager) {
-    sdch_manager_ = sdch_manager;
+    // For investigation of http://crbug.com/454198; simplify when resolved.
+    have_sdch_manager_ = !!sdch_manager;
+    if (have_sdch_manager_)
+      sdch_manager_ = sdch_manager->GetWeakPtr();
+    else
+      sdch_manager_.reset();
   }
 
   // Gets the URLRequest objects that hold a reference to this
@@ -238,7 +246,9 @@ class NET_EXPORT URLRequestContext
   HttpTransactionFactory* http_transaction_factory_;
   const URLRequestJobFactory* job_factory_;
   URLRequestThrottlerManager* throttler_manager_;
-  SdchManager* sdch_manager_;
+  // For investigation of http://crbug.com/454198; remove WeakPtr when resolved.
+  bool have_sdch_manager_;
+  base::WeakPtr<SdchManager> sdch_manager_;
 
   // ---------------------------------------------------------------------------
   // Important: When adding any new members below, consider whether they need to