Make SdchManager per-profile.

chrome/browser/profiles/profile_impl_io_data.cc

Index: chrome/browser/profiles/profile_impl_io_data.cc
diff --git a/chrome/browser/profiles/profile_impl_io_data.cc b/chrome/browser/profiles/profile_impl_io_data.cc
index 25a6ffbdf7e97588f4d0571e1a4d2ee0456cc985..0f1a5f5998c27d4422f94928ddee11497607388b 100644
--- a/chrome/browser/profiles/profile_impl_io_data.cc
+++ b/chrome/browser/profiles/profile_impl_io_data.cc
@@ -26,6 +26,7 @@
 #include "chrome/browser/net/cookie_store_util.h"
 #include "chrome/browser/net/http_server_properties_manager.h"
 #include "chrome/browser/net/predictor.h"
+#include "chrome/browser/net/sdch_dictionary_fetcher.h"
 #include "chrome/browser/net/sqlite_server_bound_cert_store.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_constants.h"
@@ -41,6 +42,7 @@
 #include "extensions/browser/extension_protocols.h"
 #include "extensions/common/constants.h"
 #include "net/base/cache_type.h"
+#include "net/base/sdch_manager.h"
 #include "net/ftp/ftp_network_layer.h"
 #include "net/http/http_cache.h"
 #include "net/ssl/server_bound_cert_service.h"
@@ -522,6 +524,18 @@ void ProfileImplIOData::InitializeInternal(
   InitializeExtensionsRequestContext(profile_params);
 #endif
 
+  // Setup the SDCHManager for this profile.
+  sdch_manager_.reset(new net::SdchManager);
+  sdch_manager_->set_sdch_fetcher(
+      new SdchDictionaryFetcher(
+          sdch_manager_.get(),
+          // SdchDictionaryFetcher takes a reference to the Getter, and
+          // hence implicitly takes ownership.
+          new net::TrivialURLRequestContextGetter(

[mmenke, 2014/06/12 16:48:31]

This makes me sad, but doing something nicer seems beyond the scope of this CL.

[Randy Smith (Not in Mondays), 2014/06/12 18:06:40]

I chatted with Will as to whether or not this should be considered a reasonable
idiom, and his opinion was that it was a reasonable idiom
(https://codereview.chromium.org/292063002).  I don't know if that increases or
decreases your sadness :-}.

The "something nicer" would be to rewrite the dictionary fetcher to use
URLRequest rather than URLFetcher, which is a good idea for a couple of reasons,
not just this one.  But I suspect you understand why I don't want it on my
critical path :-J.

[mmenke, 2014/06/12 18:11:16]

Actually, I'd be fine with using the real main request context getter - my issue
here is just with creating another one.  I'm fine with using URLFetcher for this
(For some value of "fine" - URLFetchers don't, by default, limit body sizes, so
anyone who feels like crashing Chrome may be able to do so).

+              main_context,
+              content::BrowserThread::GetMessageLoopProxyForThread(
+                  content::BrowserThread::IO))));

[mmenke, 2014/06/12 16:48:31]

Shouldn't you be calling main_context.set_sdch_manager?

[mmenke, 2014/06/12 16:48:31]

What about the OTR profile?  It would end up using the in-memory cache, so I
think it would be safe to give its own manager, I assume.

[Randy Smith (Not in Mondays), 2014/06/12 18:06:39]

Tricky question.  I think the answer for this CL is "yes", but there's a privacy
issue that'll show up coming down the pike around this.  We're going to need to
limit memory consumed by SDCH on a global basis (http://crbug.com/374917) and
that'll provide some information leakage between profiles.  But I'll worry about
this issue at that point.  Done.

[Randy Smith (Not in Mondays), 2014/06/12 18:06:39]

Yikes.  Thank you.  Yes.  (And I won't land without at least smoke testing. 
This is a refactor, so I didn't add any tests, but SDCH clearly needs a browser
test.)

+
   // Create a media request context based on the main context, but using a
   // media cache.  It shares the same job factory as the main context.
   StoragePartitionDescriptor details(profile_path_, false);