Ensure that isolated apps use the right cookies for media requests.

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 d4b0dff99ee88839c17e384d4872eda9ecb5b492..d28bf11b9a0c37e6ce0f58ae47d22c2eb0030bf6 100644
--- a/chrome/browser/profiles/profile_impl_io_data.cc
+++ b/chrome/browser/profiles/profile_impl_io_data.cc
@@ -70,6 +70,14 @@ ProfileImplIOData::Handle::~Handle() {
     iter->second->CleanupOnUIThread();
   }
 
+  // Clean up all isolated media request contexts.
+  for (ChromeURLRequestContextGetterMap::iterator iter =
+           isolated_media_request_context_getter_map_.begin();
+       iter != isolated_media_request_context_getter_map_.end();
+       ++iter) {
+    iter->second->CleanupOnUIThread();
+  }
+
   if (io_data_->http_server_properties_manager())
     io_data_->http_server_properties_manager()->ShutdownOnUIThread();
   io_data_->ShutdownOnUIThread();
@@ -107,8 +115,11 @@ void ProfileImplIOData::Handle::Init(
 
   io_data_->lazy_params_.reset(lazy_params);
 
-  // Keep track of isolated app path separately so we can use it on demand.
+  // Keep track of isolated app path and cache sizes separately so we can use
+  // them on demand.
   io_data_->app_path_ = app_path;
+  io_data_->app_cache_max_size_ = cache_max_size;
+  io_data_->app_media_cache_max_size_ = media_cache_max_size;
 
   io_data_->predictor_.reset(predictor);
 
@@ -194,7 +205,7 @@ scoped_refptr<ChromeURLRequestContextGetter>
 ProfileImplIOData::Handle::GetIsolatedAppRequestContextGetter(
     const std::string& app_id) const {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK(!app_id.empty());
+  CHECK(!app_id.empty());
   LazyInitialize();
 
   // Keep a map of request context getters, one per requested app ID.
@@ -211,6 +222,28 @@ ProfileImplIOData::Handle::GetIsolatedAppRequestContextGetter(
   return context;
 }
 
+scoped_refptr<ChromeURLRequestContextGetter>
+ProfileImplIOData::Handle::GetIsolatedMediaRequestContextGetter(
+    const std::string& app_id) const {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  // We must have an app ID, or this will act like the default media context.
+  CHECK(!app_id.empty());
+  LazyInitialize();
+
+  // Keep a map of request context getters, one per requested app ID.
+  ChromeURLRequestContextGetterMap::iterator iter =
+      isolated_media_request_context_getter_map_.find(app_id);
+  if (iter != isolated_media_request_context_getter_map_.end())
+    return iter->second;
+
+  ChromeURLRequestContextGetter* context =
+      ChromeURLRequestContextGetter::CreateOriginalForIsolatedMedia(
+          profile_, io_data_, app_id);
+  isolated_media_request_context_getter_map_[app_id] = context;
+
+  return context;
+}
+
 void ProfileImplIOData::Handle::ClearNetworkingHistorySince(
     base::Time time) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ -267,8 +300,6 @@ void ProfileImplIOData::LazyInitializeInternal(
     ProfileParams* profile_params) const {
   ChromeURLRequestContext* main_context = main_request_context();
   ChromeURLRequestContext* extensions_context = extensions_request_context();
-  media_request_context_.reset(new ChromeURLRequestContext(
-      ChromeURLRequestContext::CONTEXT_TYPE_MEDIA, cache_stats()));
 
   IOThread* const io_thread = profile_params->io_thread;
   IOThread::Globals* const io_thread_globals = io_thread->globals();
@@ -283,55 +314,37 @@ void ProfileImplIOData::LazyInitializeInternal(
   // Initialize context members.
 
   ApplyProfileParamsToContext(main_context);
-  ApplyProfileParamsToContext(media_request_context_.get());
   ApplyProfileParamsToContext(extensions_context);
 
   if (http_server_properties_manager())
     http_server_properties_manager()->InitializeOnIOThread();
 
   main_context->set_transport_security_state(transport_security_state());
-  media_request_context_->set_transport_security_state(
-      transport_security_state());
   extensions_context->set_transport_security_state(transport_security_state());
 
   main_context->set_net_log(io_thread->net_log());
-  media_request_context_->set_net_log(io_thread->net_log());
   extensions_context->set_net_log(io_thread->net_log());
 
   main_context->set_network_delegate(network_delegate());
-  media_request_context_->set_network_delegate(network_delegate());
 
   main_context->set_http_server_properties(http_server_properties_manager());
-  media_request_context_->set_http_server_properties(
-      http_server_properties_manager());
 
   main_context->set_host_resolver(
       io_thread_globals->host_resolver.get());
-  media_request_context_->set_host_resolver(
-      io_thread_globals->host_resolver.get());
   main_context->set_cert_verifier(
       io_thread_globals->cert_verifier.get());
-  media_request_context_->set_cert_verifier(
-      io_thread_globals->cert_verifier.get());
   main_context->set_http_auth_handler_factory(
       io_thread_globals->http_auth_handler_factory.get());
-  media_request_context_->set_http_auth_handler_factory(
-      io_thread_globals->http_auth_handler_factory.get());
 
   main_context->set_fraudulent_certificate_reporter(
       fraudulent_certificate_reporter());
-  media_request_context_->set_fraudulent_certificate_reporter(
-      fraudulent_certificate_reporter());
 
   main_context->set_throttler_manager(
       io_thread_globals->throttler_manager.get());
-  media_request_context_->set_throttler_manager(
-      io_thread_globals->throttler_manager.get());
   extensions_context->set_throttler_manager(
       io_thread_globals->throttler_manager.get());
 
   main_context->set_proxy_service(proxy_service());
-  media_request_context_->set_proxy_service(proxy_service());
 
   scoped_refptr<net::CookieStore> cookie_store = NULL;
   net::ServerBoundCertService* server_bound_cert_service = NULL;
@@ -371,7 +384,6 @@ void ProfileImplIOData::LazyInitializeInternal(
   extensions_cookie_store->SetCookieableSchemes(schemes, 2);
 
   main_context->set_cookie_store(cookie_store);
-  media_request_context_->set_cookie_store(cookie_store);
   extensions_context->set_cookie_store(extensions_cookie_store);
 
   // Setup server bound cert service.
@@ -389,8 +401,6 @@ void ProfileImplIOData::LazyInitializeInternal(
 
   set_server_bound_cert_service(server_bound_cert_service);
   main_context->set_server_bound_cert_service(server_bound_cert_service);
-  media_request_context_->set_server_bound_cert_service(
-      server_bound_cert_service);
 
   std::string trusted_spdy_proxy;
   if (command_line.HasSwitch(switches::kTrustedSpdyProxy)) {
@@ -418,33 +428,25 @@ void ProfileImplIOData::LazyInitializeInternal(
       main_backend,
       trusted_spdy_proxy);
 
-  net::HttpCache::DefaultBackend* media_backend =
-      new net::HttpCache::DefaultBackend(
-          net::MEDIA_CACHE, lazy_params_->media_cache_path,
-          lazy_params_->media_cache_max_size,
-          BrowserThread::GetMessageLoopProxyForThread(BrowserThread::CACHE));
-  net::HttpNetworkSession* main_network_session = main_cache->GetSession();
-  net::HttpCache* media_cache =
-      new net::HttpCache(main_network_session, media_backend);
-
   if (record_mode || playback_mode) {
     main_cache->set_mode(
         record_mode ? net::HttpCache::RECORD : net::HttpCache::PLAYBACK);
   }
 
   main_http_factory_.reset(main_cache);
-  media_http_factory_.reset(media_cache);
   main_context->set_http_transaction_factory(main_cache);
-  media_request_context_->set_http_transaction_factory(media_cache);
 
   ftp_factory_.reset(
       new net::FtpNetworkLayer(io_thread_globals->host_resolver.get()));
   main_context->set_ftp_transaction_factory(ftp_factory_.get());
-  media_request_context_->set_ftp_transaction_factory(ftp_factory_.get());
 
   main_context->set_chrome_url_data_manager_backend(
       chrome_url_data_manager_backend());
 
+  // Create a media request context based on the main context, but using a
+  // media cache.
+  media_request_context_.reset(InitializeMediaRequestContext(main_context, ""));
+
   main_job_factory_.reset(new net::URLRequestJobFactory);
   media_request_job_factory_.reset(new net::URLRequestJobFactory);
   extensions_job_factory_.reset(new net::URLRequestJobFactory);
@@ -477,20 +479,17 @@ ChromeURLRequestContext*
 ProfileImplIOData::InitializeAppRequestContext(
     ChromeURLRequestContext* main_context,
     const std::string& app_id) const {
-  AppRequestContext* context = new AppRequestContext(cache_stats());
-
   // If this is for a guest process, we should not persist cookies and http
   // cache.
-  bool guest_process = (app_id.find("guest-") != std::string::npos);
+  bool is_guest_process = (app_id.find("guest-") != std::string::npos);
 
   // Copy most state from the main context.
+  AppRequestContext* context = new AppRequestContext(cache_stats());
   context->CopyFrom(main_context);
 
   FilePath app_path = app_path_.AppendASCII(app_id);
   FilePath cookie_path = app_path.Append(chrome::kCookieFilename);
   FilePath cache_path = app_path.Append(chrome::kCacheDirname);
-  // TODO(creis): Determine correct cache size.
-  int cache_max_size = 0;
 
   const CommandLine& command_line = *CommandLine::ForCurrentProcess();
   // Only allow Record Mode if we are in a Debug build or where we are running
@@ -502,13 +501,13 @@ ProfileImplIOData::InitializeAppRequestContext(
 
   // Use a separate HTTP disk cache for isolated apps.
   net::HttpCache::BackendFactory* app_backend = NULL;
-  if (guest_process) {
+  if (is_guest_process) {
     app_backend = net::HttpCache::DefaultBackend::InMemory(0);
   } else {
     app_backend = new net::HttpCache::DefaultBackend(
         net::DISK_CACHE,
         cache_path,
-        cache_max_size,
+        app_cache_max_size_,
         BrowserThread::GetMessageLoopProxyForThread(BrowserThread::CACHE));
   }
   net::HttpNetworkSession* main_network_session =
@@ -517,7 +516,7 @@ ProfileImplIOData::InitializeAppRequestContext(
       new net::HttpCache(main_network_session, app_backend);
 
   scoped_refptr<net::CookieStore> cookie_store = NULL;
-  if (guest_process) {
+  if (is_guest_process) {
     cookie_store = new net::CookieMonster(NULL, NULL);
   } else if (record_mode || playback_mode) {
     // Don't use existing cookies and use an in-memory store.
@@ -541,6 +540,7 @@ ProfileImplIOData::InitializeAppRequestContext(
     cookie_store = new net::CookieMonster(cookie_db.get(), NULL);
   }
 
+  // Transfer ownership of the cookies and cache to AppRequestContext.
   context->SetCookieStore(cookie_store);
   context->SetHttpTransactionFactory(app_http_cache);
 
@@ -548,6 +548,48 @@ ProfileImplIOData::InitializeAppRequestContext(
 }
 
 ChromeURLRequestContext*
+ProfileImplIOData::InitializeMediaRequestContext(

[willchan no longer on Chromium, 2012/08/20 23:04:51]

I think this is right. I assume you've manually tested this? This code always
scares me, because subtle bugs get introduced by missing a line or mistyping
stuff here and they often don't show up as test breakages. But I think you've
got it right :)

[Charlie Reis, 2012/08/20 23:49:44]

I manually tested it for cookies and cache, yes, and I carefully inspected
context->CopyFrom(...) to make sure that everything we initialized before is
still being initialized.

That said, I agree that this area of the code is tricky.  I'm hoping this
actually simplifies things a little by relying on CopyFrom instead of repeating
each bit of initialization manually in LazyInitializeInternal.

+    ChromeURLRequestContext* original_context,
+    const std::string& app_id) const {
+  // If this is for a guest process, we do not persist storage, so we can
+  // simply use the app's in-memory cache (like off-the-record mode).
+  if (app_id.find("guest-") != std::string::npos)
+    return original_context;
+
+  // Copy most state from the original context.
+  MediaRequestContext* context = new MediaRequestContext(cache_stats());
+  context->CopyFrom(original_context);
+
+  FilePath app_path = app_path_.AppendASCII(app_id);
+  FilePath cache_path;
+  int cache_max_size = app_media_cache_max_size_;
+  if (app_id.empty()) {
+    // lazy_params_ is only valid for the default media context creation.
+    cache_path = lazy_params_->media_cache_path;
+    cache_max_size = lazy_params_->media_cache_max_size;
+  } else {
+    cache_path = app_path.Append(chrome::kMediaCacheDirname);
+  }
+
+  // Use a separate HTTP disk cache for isolated apps.
+  net::HttpCache::BackendFactory* media_backend =
+      new net::HttpCache::DefaultBackend(
+          net::MEDIA_CACHE,
+          cache_path,
+          cache_max_size,
+          BrowserThread::GetMessageLoopProxyForThread(BrowserThread::CACHE));
+  net::HttpNetworkSession* main_network_session =
+      main_http_factory_->GetSession();
+  net::HttpCache* media_http_cache =
+      new net::HttpCache(main_network_session, media_backend);
+
+  // Transfer ownership of the cache to MediaRequestContext.
+  context->SetHttpTransactionFactory(media_http_cache);
+
+  return context;
+}
+
+ChromeURLRequestContext*
 ProfileImplIOData::AcquireMediaRequestContext() const {
   DCHECK(media_request_context_.get());
   return media_request_context_.get();
@@ -564,6 +606,17 @@ ProfileImplIOData::AcquireIsolatedAppRequestContext(
   return app_request_context;
 }
 
+ChromeURLRequestContext*
+ProfileImplIOData::AcquireIsolatedMediaRequestContext(
+    ChromeURLRequestContext* app_context,
+    const std::string& app_id) const {
+  // We create per-app media contexts on demand, unlike the others above.
+  ChromeURLRequestContext* media_request_context =
+      InitializeMediaRequestContext(app_context, app_id);
+  DCHECK(media_request_context);
+  return media_request_context;
+}
+
 chrome_browser_net::CacheStats* ProfileImplIOData::GetCacheStats(
     IOThread::Globals* io_thread_globals) const {
   return io_thread_globals->cache_stats.get();