Initial support for partitioning cookies for isolated apps.

chrome/browser/profiles/profile_impl.cc

Index: chrome/browser/profiles/profile_impl.cc
diff --git a/chrome/browser/profiles/profile_impl.cc b/chrome/browser/profiles/profile_impl.cc
index 22e3911ff890380575f3a058670c883ba2d9f6bf..de8f9d52f6aec582e3665c4d6e55202054179ba4 100644
--- a/chrome/browser/profiles/profile_impl.cc
+++ b/chrome/browser/profiles/profile_impl.cc
@@ -816,6 +816,17 @@ URLRequestContextGetter* ProfileImpl::GetRequestContext() {
   return request_context;
 }
 
+URLRequestContextGetter* ProfileImpl::GetRequestContextForPossibleApp(
+    const Extension* installed_app) {
+  if (CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableExperimentalAppManifests) &&
+      installed_app != NULL &&
+      installed_app->is_storage_isolated())
+    return GetRequestContextForIsolatedApp(installed_app);
+
+  return GetRequestContext();
+}
+
 URLRequestContextGetter* ProfileImpl::GetRequestContextForMedia() {
   return io_data_.GetMediaRequestContextGetter();
 }
@@ -833,6 +844,35 @@ URLRequestContextGetter* ProfileImpl::GetRequestContextForExtensions() {
   return io_data_.GetExtensionsRequestContextGetter();
 }
 
+URLRequestContextGetter* ProfileImpl::GetRequestContextForIsolatedApp(
+    const Extension* installed_app) {
+  // Create a request context specific to this app on demand.
+  FilePath app_path = GetPath().Append(chrome::kIsolatedAppStateDirname);

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

How about add app_path as a parameter to the Handle::Init(). Then copy it to
ProfileImplIOData rather than the LazyParams, since we'll throw away the
LazyParams later. Then have ProfileImplIOData::InitializeAppRequestContext()
construct the cookie and cache paths. I say this because I don't think it's
actually necessary to have a LazyParamsMap.

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

Fixed.  Much cleaner.

+  app_path = app_path.AppendASCII(installed_app->id());
+  FilePath cookie_path = app_path.Append(chrome::kCookieFilename);
+  FilePath cache_path = app_path.Append(chrome::kCacheDirname);
+
+  // Create the paths if they don't yet exist.
+  BrowserThread::PostTask(
+      BrowserThread::FILE, FROM_HERE,
+      NewRunnableFunction(&ProfileImpl::CreateIsolatedAppPaths,

[Matt Perry, 2011/03/03 22:59:21]

Are these paths accessed only on the file thread? If not, I think this would be
a race - the IO thread could try to write to them before this code runs on the
file thread.

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

I was under the impression all file I/O had to be done on the File thread, but
maybe I'm mistaken.  I think the ProfileImpl constructor only gets away with it
because it's done at startup.

Calling file_util::CreateDirectory on the IO thread definitely crashes with an
ASSERT, so we can't do it here (or in InitializeAppRequestContext).  I'd hate to
block until it's finished-- are you sure that the IO thread can access them?

[Charlie Reis, 2011/03/04 23:49:41]

Hmm, looks like you may be right, Matt:
http://build.chromium.org/p/tryserver.chromium/builders/mac/builds/15800/step...
[16400:23811:0304/145455:4178059394375:FATAL:connection.cc(415)] Check failed:
false. unable to open database file
Backtrace:
	0   browser_tests                       0x00032f20
base::debug::StackTrace::StackTrace() + 32
	1   browser_tests                       0x0003383c
logging::LogMessage::~LogMessage() + 42
	2   browser_tests                       0x00d8c202
sql::Connection::OnSqliteError(int, sql::Statement*) + 238
	3   browser_tests                       0x00d8c706
sql::Connection::OpenInternal(std::string const&) + 242
	4   browser_tests                       0x00d8cca8
sql::Connection::Open(FilePath const&) + 32
	5   browser_tests                       0x006eac21
SQLitePersistentCookieStore::Backend::Load(std::vector<net::CookieMonster::CanonicalCookie*,
std::allocator<net::CookieMonster::CanonicalCookie*> >*) + 247
	6   browser_tests                       0x006eb2a1
SQLitePersistentCookieStore::Load(std::vector<net::CookieMonster::CanonicalCookie*,
std::allocator<net::CookieMonster::CanonicalCookie*> >*) + 37
	7   browser_tests                       0x0268ad7a
net::CookieMonster::InitStore() + 304

Looks like perhaps the AppCache's DB thread is accessing it.  Any suggestions
for how we should create the directory on demand (hopefully without blocking)?

[Matt Perry, 2011/03/04 23:53:53]

I think you might just have to do it earlier in startup, like before the
appcache DB thread is created, if possible. But I'm not very familiar with the
code that accesses these paths.

[Charlie Reis, 2011/03/08 00:56:48]

I'm uneasy about creating the path when the app is loaded or installed.  First,
there's lots of ways an extension can be loaded, and there's a separation
between the file handling logic and the logic that realizes it's an isolated
app.  That makes it awkward to know the correct path.

Second, it's still possible to get a browser crash if the directory is deleted
between loading the app and visiting it.

Third, it looks like other file handling logic in Chrome can already create
directories on demand.  For example, the Cache directory is created on demand in
disk_cache::BackendImpl::InitBackingStore.

I think we should be doing the same thing in
SQLitePersistentCookieStore::Backend::Load before trying to read cookies from
disk.  That will ensure the directory is only created when needed.  It has the
added advantage that it will prevent a browser crash if the profile directory is
deleted between startup and the first cookie read.  (Yeah, it's unlikely, but it
is possible today.)

I'm working on doing it that way, but I just discovered that we're actually
reading the cookies on a thread that's not supposed to go to disk.  That makes
it harder to create the directory on demand.  I've sent an email about it and
I'll follow up here when I know more.

[Charlie Reis, 2011/03/08 02:34:04]

Ugh.  It's a known issue and isn't likely to be fixed soon:
http://code.google.com/p/chromium/issues/detail?id=68657

I'll see if there's another place to create the directory...

[Charlie Reis, 2011/03/08 20:44:41]

Chatted with rdsmith@, and we agreed that the directory should be created on
demand in SQLitePersistentCookieStore (just like the cache directory, etc).  I
found there's a way to make an exception for file IO there, which makes sense
because that thread is going to disk anyway (which will be fixed in
http://crbug.com/52909).

Fixed.

+                          app_path,
+                          cache_path));
+
+  // Initialize the app context IO data on demand.
+  // TODO(creis): Determine the correct cache size.
+  io_data_.InitIsolatedApp(installed_app, cookie_path, cache_path, 0);
+
+  return io_data_.GetIsolatedAppRequestContextGetter(installed_app);
+}
+
+void ProfileImpl::CreateIsolatedAppPaths(const FilePath& app_path,
+                                         const FilePath& cache_path) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+  file_util::CreateDirectory(app_path);
+  file_util::CreateDirectory(cache_path);
+}
+
 void ProfileImpl::RegisterExtensionWithRequestContexts(
     const Extension* extension) {
   // AddRef to ensure the data lives until the other thread gets it. Balanced in