Clear SDCH information on "Clear browsing data" path.

net/base/sdch_manager.cc

Index: net/base/sdch_manager.cc
diff --git a/net/base/sdch_manager.cc b/net/base/sdch_manager.cc
index ad260a58e3f0c28d099c33ec65e677f6a0f85818..e1f3e4a56525244fbe9bd68d4a2ae105d88cf2b8 100644
--- a/net/base/sdch_manager.cc
+++ b/net/base/sdch_manager.cc
@@ -219,11 +219,24 @@ SdchManager::~SdchManager() {
   DCHECK(CalledOnValidThread());
   while (!dictionaries_.empty()) {
     DictionaryMap::iterator it = dictionaries_.begin();
-    it->second->Release();
     dictionaries_.erase(it->first);
   }
 }
 
+void SdchManager::ClearData() {
+  blacklisted_domains_.clear();
+  exponential_blacklist_count_.clear();
+  allow_latency_experiment_.clear();
+  if (fetcher_.get())
+    fetcher_->Cancel();
+
+  // Note that this may result in not having dictionaries we've advertised
+  // for incoming responses.  The window is relatively small (as ClearData()
+  // is not expected to be called frequently), so we rely on meta-refresh
+  // to handle this case.
+  dictionaries_.clear();

[jar (doing other things), 2014/06/17 05:48:55]

I suspect this is the source of the leak.  You are clearing the list of
dictionaries, but not releasing the pointers one by one (as you would do in the
destructor, on lines 220-223).

With the new code patch set 10 code, this would almost work via the
scoped_ptr... but then you'd have worse problems during map reallocations.

[jar (doing other things), 2014/06/17 17:03:28]

(comment): Using the scoped_refptr<>.... this construct will be fine here, and
resolve the leak.

[Randy Smith (Not in Mondays), 2014/06/17 18:33:11]

Done.

[Randy Smith (Not in Mondays), 2014/06/17 18:33:11]

Done.

+}
+
 // static
 void SdchManager::SdchErrorRecovery(ProblemCodes problem) {
   UMA_HISTOGRAM_ENUMERATION("Sdch3.ProblemCodes_4", problem, MAX_PROBLEM_CODE);
@@ -252,9 +265,9 @@ void SdchManager::BlacklistDomain(const GURL& url) {
   if (count > 0)
     return;  // Domain is already blacklisted.
 
-  count = 1 + 2 * exponential_blacklist_count[domain];
+  count = 1 + 2 * exponential_blacklist_count_[domain];
   if (count > 0)
-    exponential_blacklist_count[domain] = count;
+    exponential_blacklist_count_[domain] = count;
   else
     count = INT_MAX;
 
@@ -265,13 +278,13 @@ void SdchManager::BlacklistDomainForever(const GURL& url) {
   SetAllowLatencyExperiment(url, false);
 
   std::string domain(StringToLowerASCII(url.host()));
-  exponential_blacklist_count[domain] = INT_MAX;
+  exponential_blacklist_count_[domain] = INT_MAX;
   blacklisted_domains_[domain] = INT_MAX;
 }
 
 void SdchManager::ClearBlacklistings() {
   blacklisted_domains_.clear();
-  exponential_blacklist_count.clear();
+  exponential_blacklist_count_.clear();
 }
 
 void SdchManager::ClearDomainBlacklisting(const std::string& domain) {
@@ -285,10 +298,10 @@ int SdchManager::BlackListDomainCount(const std::string& domain) {
 }
 
 int SdchManager::BlacklistDomainExponential(const std::string& domain) {
-  if (exponential_blacklist_count.end() ==
-      exponential_blacklist_count.find(domain))
+  if (exponential_blacklist_count_.end() ==
+      exponential_blacklist_count_.find(domain))
     return 0;
-  return exponential_blacklist_count[StringToLowerASCII(domain)];
+  return exponential_blacklist_count_[StringToLowerASCII(domain)];
 }
 
 bool SdchManager::IsInSupportedDomain(const GURL& url) {
@@ -455,8 +468,7 @@ bool SdchManager::AddSdchDictionary(const std::string& dictionary_text,
   Dictionary* dictionary =
       new Dictionary(dictionary_text, header_end + 2, client_hash,
                      dictionary_url, domain, path, expiration, ports);
-  dictionary->AddRef();
-  dictionaries_[server_hash] = dictionary;
+  dictionaries_[server_hash] = scoped_refptr<Dictionary>(dictionary);

[jar (doing other things), 2014/06/17 05:48:55]

This is actually a bad pattern. You typically can't put scoped_refptr<> into an
STL container.  See http://www.chromium.org/developers/smart-pointer-guidelines
for a longer explanation.

The existing code (manual AddRef and Release) would achieve what you wanted...
so this could not be the cause.

[jar (doing other things), 2014/06/17 17:03:28]

nit: If you're going to use scoped_refptr, there is no need for this cast, just
do the assignment.... or better yet... since you don't need to manually AddRef,
don't waste time with the temporary.

...but if you really like the temp... it is fine... just kill the cast.

[Randy Smith (Not in Mondays), 2014/06/17 18:33:11]

Done.

[Randy Smith (Not in Mondays), 2014/06/17 18:33:11]

Really done (to distinguish from the other "Done"s which are just dismissing
comments.)

   return true;
 }