Initial support for partitioning cookies for isolated apps.

chrome/browser/profiles/off_the_record_profile_io_data.cc

Index: chrome/browser/profiles/off_the_record_profile_io_data.cc
diff --git a/chrome/browser/profiles/off_the_record_profile_io_data.cc b/chrome/browser/profiles/off_the_record_profile_io_data.cc
index 663bbe187a8191664ff5a41851e8f1d1442786bd..a03f3cc02921cf2332f9c5fdf575dc20d177e478 100644
--- a/chrome/browser/profiles/off_the_record_profile_io_data.cc
+++ b/chrome/browser/profiles/off_the_record_profile_io_data.cc
@@ -14,6 +14,7 @@
 #include "chrome/browser/net/chrome_net_log.h"
 #include "chrome/browser/net/chrome_network_delegate.h"
 #include "chrome/browser/net/chrome_url_request_context.h"
+#include "chrome/common/extensions/extension.h"
 #include "chrome/common/url_constants.h"
 #include "content/browser/browser_thread.h"
 #include "net/ftp/ftp_network_layer.h"
@@ -37,6 +38,14 @@ OffTheRecordProfileIOData::Handle::~Handle() {
     main_request_context_getter_->CleanupOnUIThread();
   if (extensions_request_context_getter_)
     extensions_request_context_getter_->CleanupOnUIThread();
+
+  // Clean up all isolated app request contexts.
+  for (ChromeURLRequestContextGetterMap::iterator iter =
+           app_request_context_getter_map_.begin();
+       iter != app_request_context_getter_map_.end();
+       ++iter) {
+    iter->second->CleanupOnUIThread();
+  }
 }
 
 scoped_refptr<ChromeURLRequestContextGetter>
@@ -66,6 +75,28 @@ OffTheRecordProfileIOData::Handle::GetExtensionsRequestContextGetter() const {
   return extensions_request_context_getter_;
 }
 
+scoped_refptr<ChromeURLRequestContextGetter>
+OffTheRecordProfileIOData::Handle::GetIsolatedAppRequestContextGetter(
+    const Extension* installed_app) const {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  CHECK(installed_app);
+  LazyInitialize();
+
+  // Keep a map of request context getters, one per requested app ID.
+  std::string id = installed_app->id();
+  ChromeURLRequestContextGetterMap::iterator iter =
+      app_request_context_getter_map_.find(id);
+  if (iter != app_request_context_getter_map_.end())
+    return iter->second;
+
+  ChromeURLRequestContextGetter* context =
+      ChromeURLRequestContextGetter::CreateOffTheRecordForIsolatedApp(
+          profile_, io_data_, installed_app);
+  app_request_context_getter_map_[id] = context;
+
+  return context;
+}
+
 void OffTheRecordProfileIOData::Handle::LazyInitialize() const {
   if (!initialized_) {
     InitializeProfileParams(profile_, &io_data_->lazy_params_->profile_params);
@@ -130,6 +161,8 @@ void OffTheRecordProfileIOData::LazyInitializeInternal() const {
 
   main_request_context_->set_cookie_store(
       new net::CookieMonster(NULL, profile_params.cookie_monster_delegate));
+  main_request_context_->set_cookie_delegate(
+      profile_params.cookie_monster_delegate);
   // All we care about for extensions is the cookie store. For incognito, we
   // use a non-persistent cookie store.
 
@@ -163,6 +196,32 @@ void OffTheRecordProfileIOData::LazyInitializeInternal() const {
       new net::FtpNetworkLayer(main_request_context_->host_resolver()));
 }
 
+scoped_refptr<ProfileIOData::RequestContext>
+OffTheRecordProfileIOData::InitializeAppRequestContext(
+    scoped_refptr<ChromeURLRequestContext> main_context,
+    const Extension *app) const {
+  scoped_refptr<ProfileIOData::RequestContext> context = new RequestContext;
+
+  // Copy most state from the main context.
+  main_context->Copy(context);

[willchan no longer on Chromium, 2011/03/03 18:16:59]

As I read this now, I realize that Copy() makes it ambiguous as to the direction
of the copy. Can you change it to CopyFrom() and reverse orders? CopyFrom()
seems more common than CopyTo() in our codebase.

[Charlie Reis, 2011/03/04 22:34:53]

Fixed.

+
+  // Use a separate in-memory cookie store for the app.
+  context->set_cookie_store(
+      new net::CookieMonster(NULL, main_context->cookie_delegate()));
+  context->set_cookie_delegate(main_context->cookie_delegate());

[willchan no longer on Chromium, 2011/03/03 18:16:59]

Rather than forcing cookie_delegate to be a member of ChromeURLRequestContext,
can you have OffTheRecordProfileImpl keep a scoped_refptr to cookie_delegate?

But do you want to use the same CookieMonster::Delegate in the first place?
What's your goal here? A ChromeCookieMonsterDelegate is associated with a
Profile. Whenever a cookie changes for that Profile, we will send out a
notification, which is used by the cookie extensions API to monitor cookie
changes. So, by sharing the same CookieMonster::Delegate, you will have isolated
apps' cookie changes affect extensions in that same Profile, even though the
CookieStore for that Profile did not have its cookies changed.

[Charlie Reis, 2011/03/04 22:34:53]

Hmm, this is a tricky question to answer, since the Profile won't have a single
set of cookies anymore.  (An isolated app is actually part of the same Profile
as normal web sites.)

I see a few options:
1) Expose a unified cookie store via the cookie extensions API, containing the
union of cookies in the normal store and all isolated apps' stores.  This
doesn't seem right, since the stores aren't necessarily disjoint.  (You can load
an isolated app page in a frame of a normal page, giving it a different cookie
than in the app process.)

2) Hide isolated apps' cookies from the cookie extensions API.  This is probably
the simplest thing for now, but it's less than ideal.  I don't see a reason why
extensions shouldn't work on isolated apps.

3) Expose isolated apps' cookies as coming from a different cookie store in the
cookie extensions API.  Looking at the API, it already has a notion of different
cookie stores, so this might not be too hard.  One tricky issues is that
cookeStore.tabIds will change over time, since a tab can now switch which cookie
store it is using as the user navigates into or out of an isolated app.

I'm leaning toward option 2 for this CL, with a followup CL to address the
cookie extensions API.  (Glad we ran into this.)  Is the CookieDelegate used for
anything else, or can I use NULL for now, like the extensions_cookie_store?

+
+  // Use a separate in-memory cache for the app.
+  net::HttpCache::BackendFactory* app_backend =
+      net::HttpCache::DefaultBackend::InMemory(0);
+  net::HttpNetworkSession* main_network_session =
+      main_http_factory_->GetSession();
+  net::HttpCache* app_cache =

[willchan no longer on Chromium, 2011/03/03 18:16:59]

app_cache is used elsewhere in ChromeURLRequestContext (for the
ChromeAppCacheService), so naming a variable app_cache is misleading. I
recommend you name it app_http_cache.

[Charlie Reis, 2011/03/04 22:34:53]

Done.

+      new net::HttpCache(main_network_session, app_backend);
+  context->set_http_transaction_factory(app_cache);

[willchan no longer on Chromium, 2011/03/03 18:16:59]

Are you leaking the HttpCache?

Note that URLRequestContext::http_transaction_factory_ is a raw pointer. It's
possible we could switch it to being a scoped_ptr. I don't *think* we have any
situations where a different URLRequestContext shares the same
HttpTransactionFactory, but then again, I don't think it's impossible. It might
be best for you to add a set of HttpTransactionFactory objects into the
OffTheRecordProfileImpl and delete all of them in the destructor. Indeed, we
delete the other HttpTransactionFactory instances in the destructor.

[Charlie Reis, 2011/03/04 22:34:53]

Ah!  That answers my question from before about what main_http_factory_ is used
for. 
(http://codereview.chromium.org/6201005/diff/10001/chrome/browser/net/chrome_u...)

Fixed using a hash_map (since HttpTransactionFactory isn't hashable for a
hash_set).

+
+  return context;
+}
+
 scoped_refptr<ChromeURLRequestContext>
 OffTheRecordProfileIOData::AcquireMainRequestContext() const {
   DCHECK(main_request_context_);
@@ -186,3 +245,15 @@ OffTheRecordProfileIOData::AcquireExtensionsRequestContext() const {
   extensions_request_context_ = NULL;
   return context;
 }
+
+scoped_refptr<ChromeURLRequestContext>
+OffTheRecordProfileIOData::AcquireIsolatedAppRequestContext(
+    scoped_refptr<ChromeURLRequestContext> main_context,
+    const Extension* installed_app) const {
+  // We create per-app contexts on demand, unlike the others above.
+  scoped_refptr<RequestContext> app_request_context =
+      InitializeAppRequestContext(main_context, installed_app);
+  DCHECK(app_request_context);
+  app_request_context->set_profile_io_data(this);
+  return app_request_context;
+}