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..19d7822804a0dda42753045b5049b90f1834d79b 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();
@@ -211,6 +219,27 @@ ProfileImplIOData::Handle::GetIsolatedAppRequestContextGetter(
   return context;
 }
 
+scoped_refptr<ChromeURLRequestContextGetter>
+ProfileImplIOData::Handle::GetIsolatedMediaRequestContextGetter(
+    const std::string& app_id) const {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(!app_id.empty());

[awong, 2012/08/17 18:23:16]

What happens if app_id is empty?  Will this work correctly still, or will it do
something bad?

If it does something bad, we should CHECK.  Same commeht for the
Get?IsolatedAppRequestContextGetter() above.

[Charlie Reis, 2012/08/17 21:04:38]

Switched these to CHECKs.  (Otherwise it would act like the default media
context.)

+  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 +296,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 +310,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 +380,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 +397,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 +424,27 @@ 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, ""));
+
+  // TODO(creis): Ensure these job factories get created for isolated app and
+  // isolated media request contexts.
   main_job_factory_.reset(new net::URLRequestJobFactory);
   media_request_job_factory_.reset(new net::URLRequestJobFactory);
   extensions_job_factory_.reset(new net::URLRequestJobFactory);
@@ -477,13 +477,12 @@ 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);
@@ -502,7 +501,7 @@ 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(
@@ -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,9 +540,58 @@ 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);
 
+  // TODO(creis): This is missing the job factory initialization from
+  // LazyInitializeInternal.

[awong, 2012/08/17 18:23:16]

I'm unclear on the implications of this TODO as well as what the correct
behavior should be.

[Charlie Reis, 2012/08/17 21:04:38]

Looks like we at least copy the existing job factory from the main context as
part of CopyFrom, so this doesn't seem critical.  I've posted a comment to
http://crbug.com/142703 and removed the TODOs.

+
+  return context;
+}
+
+ChromeURLRequestContext*
+ProfileImplIOData::InitializeMediaRequestContext(
+    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;
+  // TODO(creis): Determine correct cache size to use for isolated media.

[awong, 2012/08/17 18:23:16]

File bug?

[Charlie Reis, 2012/08/17 21:04:38]

Fixed instead, similar to how we handle app_path_.

+  int cache_max_size = 0;
+  if (app_id.empty()) {
+    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);
+
+  // TODO(creis): This is missing the job factory initialization from
+  // LazyInitializeInternal.
+
   return context;
 }
 
@@ -564,6 +612,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();