Implement the ability to obliterate a storage partition from disk.

Implement the ability to obliterate a storage partition from disk.

On the uninstall of an extension with isolated storage, we want to delete
all the data for the extension from disk as soon as possible.  Because we
cannot know when when various objects with state on disk 
(eg., FileSystemContext) have all be deleted, we do a best-effort delete
for any directory that we know isn't being used.

The way this gets projected into the content modulue is that each extension
defines one partition_domain. If an extension has a <webview> tag, it will
also have multiple StoragePartitions, each with a different partition_name.
If it doesn't have a <webview> tag, the partition_name is considered empty
which yields the default partition. The default partition, and all webview
partitions are peers inside the partition_domain's root directory.

This CL introduces a function that allows us to delete partiton domain.
Special care is taken to not accidentally instantiate a StoragePartition for
the domain if none current exists. This is necessary to allow us to actually
delete the whole partition domain directory.

BUG=85127

[awong, 2012-11-15 03:13:34]

Nasko, Charlie, could one of you do the primary review? (not sure who is more
suited).

I'll get matt to look at the DataDeleter hook-in once we have some confidence
that the rest of the code seems sane.

[nasko, 2012-11-15 17:48:10]

Mostly nits and clarifications.

https://codereview.chromium.org/11362268/diff/5001/content/browser/storage_pa...
File content/browser/storage_partition_impl.cc (right):

https://codereview.chromium.org/11362268/diff/5001/content/browser/storage_pa...
content/browser/storage_partition_impl.cc:79: // Clear out appcache.
If I get it right ClearQuotaManagedOriginsOnIOThread should take care of all
quota clients, which is more than appcache. If I got this right, the comment is
incorrect and should be fixed.

https://codereview.chromium.org/11362268/diff/5001/content/browser/storage_pa...
content/browser/storage_partition_impl.cc:261:
base::Bind(&OnLocalStorageUsageInfo,
nit: This should fit on a single line.

https://codereview.chromium.org/11362268/diff/5001/content/browser/storage_pa...
File content/browser/storage_partition_impl_map.cc (right):

https://codereview.chromium.org/11362268/diff/5001/content/browser/storage_pa...
content/browser/storage_partition_impl_map.cc:245: #if defined(OS_POSIX)
Why not put these right before the function that uses them?

https://codereview.chromium.org/11362268/diff/5001/content/browser/storage_pa...
content/browser/storage_partition_impl_map.cc:295: // Reduce |active_paths| set
to those under the root and actually on disk.
What is |active_paths|?

https://codereview.chromium.org/11362268/diff/5001/content/browser/storage_pa...
content/browser/storage_partition_impl_map.cc:300: if (root.IsParent(*it) &&
file_util::PathExists(*it)) {
nit: no braces needed.

https://codereview.chromium.org/11362268/diff/5001/content/browser/storage_pa...
content/browser/storage_partition_impl_map.cc:337: // a persistent one that
return the same FilePath for GetPath().
Adding a path component might not work with the quota manager. I remember you
calling out specificly the levels of nested directories matters. We should just
mix it with the hash input.

https://codereview.chromium.org/11362268/diff/5001/content/browser/storage_pa...
content/browser/storage_partition_impl_map.cc:432: if (!config.in_memory) {
nit: no braces needed.

[Charlie Reis, 2012-11-15 18:28:21]

Yeah, I guess we have to keep certain directories around if they're still in
use.  Seems good at a high level.

https://codereview.chromium.org/11362268/diff/5001/chrome/browser/extensions/...
File chrome/browser/extensions/data_deleter.cc (right):

https://codereview.chromium.org/11362268/diff/5001/chrome/browser/extensions/...
chrome/browser/extensions/data_deleter.cc:27: // TODO(ajwong): If
|is_storage_isolated|, we should just blowaway the
Does this CL resolve this TODO?

https://codereview.chromium.org/11362268/diff/5001/content/browser/storage_pa...
File content/browser/storage_partition_impl_map.cc (right):

https://codereview.chromium.org/11362268/diff/5001/content/browser/storage_pa...
content/browser/storage_partition_impl_map.cc:267: void
ObliterateOneDirectory(const FilePath& current_dir,
Why so violent?  :)  Obliterate seems like an unusually strong term, especially
if it's going to be leaving some paths behind.  Would just using Delete not
convey something?

[awong, 2012-11-16 00:36:03]

PTAL. It got more complicated. :-/

https://codereview.chromium.org/11362268/diff/5001/chrome/browser/extensions/...
File chrome/browser/extensions/data_deleter.cc (right):

https://codereview.chromium.org/11362268/diff/5001/chrome/browser/extensions/...
chrome/browser/extensions/data_deleter.cc:27: // TODO(ajwong): If
|is_storage_isolated|, we should just blowaway the
On 2012/11/15 18:28:21, creis wrote:
> Does this CL resolve this TODO?

Done.

https://codereview.chromium.org/11362268/diff/5001/content/browser/storage_pa...
File content/browser/storage_partition_impl.cc (right):

https://codereview.chromium.org/11362268/diff/5001/content/browser/storage_pa...
content/browser/storage_partition_impl.cc:79: // Clear out appcache.
On 2012/11/15 17:48:10, nasko wrote:
> If I get it right ClearQuotaManagedOriginsOnIOThread should take care of all
> quota clients, which is more than appcache. If I got this right, the comment
is
> incorrect and should be fixed.

Done.

https://codereview.chromium.org/11362268/diff/5001/content/browser/storage_pa...
content/browser/storage_partition_impl.cc:261:
base::Bind(&OnLocalStorageUsageInfo,
On 2012/11/15 17:48:10, nasko wrote:
> nit: This should fit on a single line.

Done.

https://codereview.chromium.org/11362268/diff/5001/content/browser/storage_pa...
File content/browser/storage_partition_impl_map.cc (right):

https://codereview.chromium.org/11362268/diff/5001/content/browser/storage_pa...
content/browser/storage_partition_impl_map.cc:245: #if defined(OS_POSIX)
On 2012/11/15 17:48:10, nasko wrote:
> Why not put these right before the function that uses them?

Was keeping all constants grouped...

https://codereview.chromium.org/11362268/diff/5001/content/browser/storage_pa...
content/browser/storage_partition_impl_map.cc:267: void
ObliterateOneDirectory(const FilePath& current_dir,
On 2012/11/15 18:28:21, creis wrote:
> Why so violent?  :)  Obliterate seems like an unusually strong term,
especially
> if it's going to be leaving some paths behind.  Would just using Delete not
> convey something?

I was avoiding "Delete" because there's so many DeleteXXX methods on the various
classes (eg., QuotaManager::DeleteOriginData()) that remove the data from their
database, but leave the database around.

Obliterate comes for perforce
(http://www.perforce.com/perforce/r12.1/manuals/cmdref/obliterate.html) so if
anyone is violent, it's them.  I assure you that I only use my "rm -rf" for
peaceful purposes.

https://codereview.chromium.org/11362268/diff/5001/content/browser/storage_pa...
content/browser/storage_partition_impl_map.cc:295: // Reduce |active_paths| set
to those under the root and actually on disk.
On 2012/11/15 17:48:10, nasko wrote:
> What is |active_paths|?

Old name. Fixed.

https://codereview.chromium.org/11362268/diff/5001/content/browser/storage_pa...
content/browser/storage_partition_impl_map.cc:300: if (root.IsParent(*it) &&
file_util::PathExists(*it)) {
On 2012/11/15 17:48:10, nasko wrote:
> nit: no braces needed.

Done.

https://codereview.chromium.org/11362268/diff/5001/content/browser/storage_pa...
content/browser/storage_partition_impl_map.cc:337: // a persistent one that
return the same FilePath for GetPath().
On 2012/11/15 17:48:10, nasko wrote:
> Adding a path component might not work with the quota manager. I remember you
> calling out specificly the levels of nested directories matters. We should
just
> mix it with the hash input.

The more I think about this, the more I think we should just leave persist:
inside the name rather than stripping it out in BrowserGuest.

https://codereview.chromium.org/11362268/diff/5001/content/browser/storage_pa...
content/browser/storage_partition_impl_map.cc:432: if (!config.in_memory) {
On 2012/11/15 17:48:10, nasko wrote:
> nit: no braces needed.

Done.

[Charlie Reis, 2012-11-16 01:45:10]

Hate to demand this, but this would really benefit from a test, given how many
ways we've seen it can go wrong.  I'm ok with doing that in a follow-up CL if
necessary.

Aside from that, a few questions and comments below.

https://codereview.chromium.org/11362268/diff/6010/chrome/browser/chrome_cont...
File chrome/browser/chrome_content_browser_client.cc (right):

https://codereview.chromium.org/11362268/diff/6010/chrome/browser/chrome_cont...
chrome/browser/chrome_content_browser_client.cc:510: partition_name, in_memory);
nit: Wrong indent.

https://codereview.chromium.org/11362268/diff/6010/content/browser/storage_pa...
File content/browser/storage_partition_impl.cc (right):

https://codereview.chromium.org/11362268/diff/6010/content/browser/storage_pa...
content/browser/storage_partition_impl.cc:34: // The QuotaManager managers all
storage other than cookies, LocalStorage,
nit: manages

https://codereview.chromium.org/11362268/diff/6010/content/browser/storage_pa...
content/browser/storage_partition_impl.cc:35: // and SessionStorage. This loop
wipe out most HTML5 storage for the given
nit: wipes

https://codereview.chromium.org/11362268/diff/6010/content/browser/storage_pa...
content/browser/storage_partition_impl.cc:36: // origin.
nit: origins

https://codereview.chromium.org/11362268/diff/6010/content/browser/storage_pa...
content/browser/storage_partition_impl.cc:67: quota::kStorageTypeTemporary);
Is there a reason to pass in type here, rather than just having
ClearQuotaManagedOriginsOnIOThread take care of both known types?  That
simplifies this call site and the one below.

https://codereview.chromium.org/11362268/diff/6010/content/browser/storage_pa...
File content/browser/storage_partition_impl_map.cc (right):

https://codereview.chromium.org/11362268/diff/6010/content/browser/storage_pa...
content/browser/storage_partition_impl_map.cc:308: void
BlockingObliteratePath(const FilePath& root,
This all looks correct, though it is a ton of effort.  What's the consequence of
just deleting the whole directory on Posix systems?  I know it will fail on
Windows, but there's a DeleteAfterReboot in file_util.h for OS_WIN.

Just seeing if there's an easier way.  If not, we'll go with your approach.

https://codereview.chromium.org/11362268/diff/6010/content/browser/storage_pa...
content/browser/storage_partition_impl_map.cc:437: // from disk, start deletions
in all of them.
Sorry, can you clarify "removed from disk" vs "deletions" here?  You've
explained it to me in person, but it's not immediately obvious.

https://codereview.chromium.org/11362268/diff/6010/content/browser/storage_pa...
content/browser/storage_partition_impl_map.cc:454: // StoragePartition state
that just happens to not been used during this run
nit: not have been

https://codereview.chromium.org/11362268/diff/6010/content/browser/storage_pa...
File content/browser/storage_partition_impl_map.h (right):

https://codereview.chromium.org/11362268/diff/6010/content/browser/storage_pa...
content/browser/storage_partition_impl_map.h:37: // |partition_map|.
nit: There's no |partition_map| here.  Can you clarify?

https://codereview.chromium.org/11362268/diff/6010/content/public/browser/con...
File content/public/browser/content_browser_client.h (right):

https://codereview.chromium.org/11362268/diff/6010/content/public/browser/con...
content/public/browser/content_browser_client.h:300: // (such as data deleters)
need to access the previous configuration.
nit: that need

https://codereview.chromium.org/11362268/diff/6010/content/public/browser/con...
content/public/browser/content_browser_client.h:305: virtual void
ParseNonDefaultStoragePartitionConfig(
This is too complex to expose as its own function in the public API.  It's very
hard to understand how these two functions differ.

Another proposal: add something like a |might_be_default| parameter to
GetStoragePartitionConfigForSite.  If it's true, we behave as before.  If it's
false, we behave like your ParseNonDefaultStoragePartitionConfig.  It's clearer
to understand how it affects the outcome, and it seems to fit into your code
without too much effort.  Thoughts?

[awong, 2012-11-16 02:56:10]

Totally agreed with the test.  But not making it due to deadline.

Can you file a bug against me?  I will do it after thanksgiving.

http://codereview.chromium.org/11362268/diff/6010/content/browser/storage_par...
File content/browser/storage_partition_impl.cc (right):

http://codereview.chromium.org/11362268/diff/6010/content/browser/storage_par...
content/browser/storage_partition_impl.cc:34: // The QuotaManager managers all
storage other than cookies, LocalStorage,
On 2012/11/16 01:45:10, creis wrote:
> nit: manages

Done.

http://codereview.chromium.org/11362268/diff/6010/content/browser/storage_par...
content/browser/storage_partition_impl.cc:35: // and SessionStorage. This loop
wipe out most HTML5 storage for the given
On 2012/11/16 01:45:10, creis wrote:
> nit: wipes

Done.

http://codereview.chromium.org/11362268/diff/6010/content/browser/storage_par...
content/browser/storage_partition_impl.cc:36: // origin.
On 2012/11/16 01:45:10, creis wrote:
> nit: origins

Done.

http://codereview.chromium.org/11362268/diff/6010/content/browser/storage_par...
content/browser/storage_partition_impl.cc:67: quota::kStorageTypeTemporary);
On 2012/11/16 01:45:10, creis wrote:
> Is there a reason to pass in type here, rather than just having
> ClearQuotaManagedOriginsOnIOThread take care of both known types?  That
> simplifies this call site and the one below.

Actually, the one below requires it stay as is.  The callback that
GetOriginsModifiedSince() expects needs to be able to take the type as the
second parameter.

http://codereview.chromium.org/11362268/diff/6010/content/browser/storage_par...
File content/browser/storage_partition_impl_map.cc (right):

http://codereview.chromium.org/11362268/diff/6010/content/browser/storage_par...
content/browser/storage_partition_impl_map.cc:308: void
BlockingObliteratePath(const FilePath& root,
On 2012/11/16 01:45:10, creis wrote:
> This all looks correct, though it is a ton of effort.  What's the consequence
of
> just deleting the whole directory on Posix systems?  I know it will fail on
> Windows, but there's a DeleteAfterReboot in file_util.h for OS_WIN.
> 
> Just seeing if there's an easier way.  If not, we'll go with your approach.

The browser will eventually crash when one of the active HTML5 Storage contexts
tries to access the directory that housed its data.  I tested this actually and
it does after a little bit.

http://codereview.chromium.org/11362268/diff/6010/content/browser/storage_par...
content/browser/storage_partition_impl_map.cc:437: // from disk, start deletions
in all of them.
On 2012/11/16 01:45:10, creis wrote:
> Sorry, can you clarify "removed from disk" vs "deletions" here?  You've
> explained it to me in person, but it's not immediately obvious.

Done.

http://codereview.chromium.org/11362268/diff/6010/content/browser/storage_par...
content/browser/storage_partition_impl_map.cc:454: // StoragePartition state
that just happens to not been used during this run
On 2012/11/16 01:45:10, creis wrote:
> nit: not have been

Done.

http://codereview.chromium.org/11362268/diff/6010/content/browser/storage_par...
File content/browser/storage_partition_impl_map.h (right):

http://codereview.chromium.org/11362268/diff/6010/content/browser/storage_par...
content/browser/storage_partition_impl_map.h:37: // |partition_map|.
On 2012/11/16 01:45:10, creis wrote:
> nit: There's no |partition_map| here.  Can you clarify?

Was referring to current object.  Just removed that chunk since it's implied by
being a method.

http://codereview.chromium.org/11362268/diff/6010/content/public/browser/cont...
File content/public/browser/content_browser_client.h (right):

http://codereview.chromium.org/11362268/diff/6010/content/public/browser/cont...
content/public/browser/content_browser_client.h:300: // (such as data deleters)
need to access the previous configuration.
On 2012/11/16 01:45:10, creis wrote:
> nit: that need

Function removed.

http://codereview.chromium.org/11362268/diff/6010/content/public/browser/cont...
content/public/browser/content_browser_client.h:305: virtual void
ParseNonDefaultStoragePartitionConfig(
On 2012/11/16 01:45:10, creis wrote:
> This is too complex to expose as its own function in the public API.  It's
very
> hard to understand how these two functions differ.
> 
> Another proposal: add something like a |might_be_default| parameter to
> GetStoragePartitionConfigForSite.  If it's true, we behave as before.  If it's
> false, we behave like your ParseNonDefaultStoragePartitionConfig.  It's
clearer
> to understand how it affects the outcome, and it seems to fit into your code
> without too much effort.  Thoughts?

Went with your solution.

[awong, 2012-11-16 02:58:13]

FYI, testing consisted of:

Setup:
  1) Loading platform app
  2) Creating webview tags with multiple persistent and in-memory partitions
  3) inspecting profile to see that we have correct directory structures.
  4) Set something using localStorage on the app's main page.

Test:
  1) Uninstall the app immedately after the above.  Ensure none of the state is
removed from disk.
  2) Restart browser. Reinstall app. Uninstall before starting. Ensure that all
directories except "def" are deleted.
  3) Check that the localStorage that we set previously isn't available.

On 2012/11/16 02:56:10, awong wrote:
> Totally agreed with the test.  But not making it due to deadline.
> 
> Can you file a bug against me?  I will do it after thanksgiving.
> 
>
http://codereview.chromium.org/11362268/diff/6010/content/browser/storage_par...
> File content/browser/storage_partition_impl.cc (right):
> 
>
http://codereview.chromium.org/11362268/diff/6010/content/browser/storage_par...
> content/browser/storage_partition_impl.cc:34: // The QuotaManager managers all
> storage other than cookies, LocalStorage,
> On 2012/11/16 01:45:10, creis wrote:
> > nit: manages
> 
> Done.
> 
>
http://codereview.chromium.org/11362268/diff/6010/content/browser/storage_par...
> content/browser/storage_partition_impl.cc:35: // and SessionStorage. This loop
> wipe out most HTML5 storage for the given
> On 2012/11/16 01:45:10, creis wrote:
> > nit: wipes
> 
> Done.
> 
>
http://codereview.chromium.org/11362268/diff/6010/content/browser/storage_par...
> content/browser/storage_partition_impl.cc:36: // origin.
> On 2012/11/16 01:45:10, creis wrote:
> > nit: origins
> 
> Done.
> 
>
http://codereview.chromium.org/11362268/diff/6010/content/browser/storage_par...
> content/browser/storage_partition_impl.cc:67: quota::kStorageTypeTemporary);
> On 2012/11/16 01:45:10, creis wrote:
> > Is there a reason to pass in type here, rather than just having
> > ClearQuotaManagedOriginsOnIOThread take care of both known types?  That
> > simplifies this call site and the one below.
> 
> Actually, the one below requires it stay as is.  The callback that
> GetOriginsModifiedSince() expects needs to be able to take the type as the
> second parameter.
> 
>
http://codereview.chromium.org/11362268/diff/6010/content/browser/storage_par...
> File content/browser/storage_partition_impl_map.cc (right):
> 
>
http://codereview.chromium.org/11362268/diff/6010/content/browser/storage_par...
> content/browser/storage_partition_impl_map.cc:308: void
> BlockingObliteratePath(const FilePath& root,
> On 2012/11/16 01:45:10, creis wrote:
> > This all looks correct, though it is a ton of effort.  What's the
consequence
> of
> > just deleting the whole directory on Posix systems?  I know it will fail on
> > Windows, but there's a DeleteAfterReboot in file_util.h for OS_WIN.
> > 
> > Just seeing if there's an easier way.  If not, we'll go with your approach.
> 
> The browser will eventually crash when one of the active HTML5 Storage
contexts
> tries to access the directory that housed its data.  I tested this actually
and
> it does after a little bit.
> 
>
http://codereview.chromium.org/11362268/diff/6010/content/browser/storage_par...
> content/browser/storage_partition_impl_map.cc:437: // from disk, start
deletions
> in all of them.
> On 2012/11/16 01:45:10, creis wrote:
> > Sorry, can you clarify "removed from disk" vs "deletions" here?  You've
> > explained it to me in person, but it's not immediately obvious.
> 
> Done.
> 
>
http://codereview.chromium.org/11362268/diff/6010/content/browser/storage_par...
> content/browser/storage_partition_impl_map.cc:454: // StoragePartition state
> that just happens to not been used during this run
> On 2012/11/16 01:45:10, creis wrote:
> > nit: not have been
> 
> Done.
> 
>
http://codereview.chromium.org/11362268/diff/6010/content/browser/storage_par...
> File content/browser/storage_partition_impl_map.h (right):
> 
>
http://codereview.chromium.org/11362268/diff/6010/content/browser/storage_par...
> content/browser/storage_partition_impl_map.h:37: // |partition_map|.
> On 2012/11/16 01:45:10, creis wrote:
> > nit: There's no |partition_map| here.  Can you clarify?
> 
> Was referring to current object.  Just removed that chunk since it's implied
by
> being a method.
> 
>
http://codereview.chromium.org/11362268/diff/6010/content/public/browser/cont...
> File content/public/browser/content_browser_client.h (right):
> 
>
http://codereview.chromium.org/11362268/diff/6010/content/public/browser/cont...
> content/public/browser/content_browser_client.h:300: // (such as data
deleters)
> need to access the previous configuration.
> On 2012/11/16 01:45:10, creis wrote:
> > nit: that need
> 
> Function removed.
> 
>
http://codereview.chromium.org/11362268/diff/6010/content/public/browser/cont...
> content/public/browser/content_browser_client.h:305: virtual void
> ParseNonDefaultStoragePartitionConfig(
> On 2012/11/16 01:45:10, creis wrote:
> > This is too complex to expose as its own function in the public API.  It's
> very
> > hard to understand how these two functions differ.
> > 
> > Another proposal: add something like a |might_be_default| parameter to
> > GetStoragePartitionConfigForSite.  If it's true, we behave as before.  If
it's
> > false, we behave like your ParseNonDefaultStoragePartitionConfig.  It's
> clearer
> > to understand how it affects the outcome, and it seems to fit into your code
> > without too much effort.  Thoughts?
> 
> Went with your solution.

[awong, 2012-11-16 07:09:36]

hmm...try servers failing. Need to diagnose. :(

[awong, 2012-11-16 15:28:09]

On 2012/11/16 07:09:36, awong wrote:
> hmm...try servers failing. Need to diagnose. :(

Found a logic bug and sent for new try jobs.

[Charlie Reis, 2012-11-16 16:14:59]

One question, then I think we're ready to go.  (Also, most recent try jobs
failed to compile.)

https://codereview.chromium.org/11362268/diff/6010/content/browser/storage_pa...
File content/browser/storage_partition_impl_map.cc (right):

https://codereview.chromium.org/11362268/diff/6010/content/browser/storage_pa...
content/browser/storage_partition_impl_map.cc:437: // from disk, start deletions
in all of them.
On 2012/11/16 02:56:10, awong wrote:
> On 2012/11/16 01:45:10, creis wrote:
> > Sorry, can you clarify "removed from disk" vs "deletions" here?  You've
> > explained it to me in person, but it's not immediately obvious.
> 
> Done.

Much better, thanks!

https://codereview.chromium.org/11362268/diff/13014/content/browser/storage_p...
File content/browser/storage_partition_impl.cc (left):

https://codereview.chromium.org/11362268/diff/13014/content/browser/storage_p...
content/browser/storage_partition_impl.cc:51: // Clear out the HTML5 filesystem.
Who takes care of this now?  Is it part of ClearQuotaManagedOriginsOnIOThread,
even though that's on the IO thread and this was on the FILE thread?

[awong, 2012-11-16 16:28:56]

I'll fix the compile failures.  My last try job accidentally picked up garbage
on my client.

https://codereview.chromium.org/11362268/diff/13014/content/browser/storage_p...
File content/browser/storage_partition_impl.cc (left):

https://codereview.chromium.org/11362268/diff/13014/content/browser/storage_p...
content/browser/storage_partition_impl.cc:51: // Clear out the HTML5 filesystem.
On 2012/11/16 16:14:59, creis wrote:
> Who takes care of this now?  Is it part of ClearQuotaManagedOriginsOnIOThread,
> even though that's on the IO thread and this was on the FILE thread?

Yep.  The quota manager API abstracts all the thread bouncing.

[Charlie Reis, 2012-11-16 16:32:36]

Great.  LGTM.

On 2012/11/16 16:28:56, awong wrote:
> I'll fix the compile failures.  My last try job accidentally picked up garbage
> on my client.
> 
>
https://codereview.chromium.org/11362268/diff/13014/content/browser/storage_p...
> File content/browser/storage_partition_impl.cc (left):
> 
>
https://codereview.chromium.org/11362268/diff/13014/content/browser/storage_p...
> content/browser/storage_partition_impl.cc:51: // Clear out the HTML5
filesystem.
> On 2012/11/16 16:14:59, creis wrote:
> > Who takes care of this now?  Is it part of
ClearQuotaManagedOriginsOnIOThread,
> > even though that's on the IO thread and this was on the FILE thread?
> 
> Yep.  The quota manager API abstracts all the thread bouncing.

[awong, 2012-11-16 16:37:30]

Matt, can you do owners check for chrome/browser/extensions, and assuming all
looks good, hit the CQ button?

I'll be on a plane in another 2 hours, so unless you have _really_ fast
turnaround, I won't be able to hit the commit.

[Matt Perry, 2012-11-16 19:58:11]

lgtm

[commit-bot: I haz the power, 2012-11-16 19:58:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ajwong@chromium.org/11362268/11004

[commit-bot: I haz the power, 2012-11-16 19:58:27]

Presubmit check for 11362268-11004 failed and returned exit status 1.


Running presubmit commit checks ...

** Presubmit Warnings **
Your #include order seems to be broken. Send mail to
marja@chromium.org if this is not the case.
      content/browser/storage_partition_impl.cc:10 \
      content/browser/storage_partition_impl.cc:19

Presubmit checks took 1.8s to calculate.

[Charlie Reis, 2012-11-16 20:01:43]

On 2012/11/16 19:58:27, I haz the power (commit-bot) wrote:
> Presubmit check for 11362268-11004 failed and returned exit status 1.
> 
> 
> Running presubmit commit checks ...
> 
> ** Presubmit Warnings **
> Your #include order seems to be broken. Send mail to
> mailto:marja@chromium.org if this is not the case.
>       content/browser/storage_partition_impl.cc:10 \
>       content/browser/storage_partition_impl.cc:19
> 
> Presubmit checks took 1.8s to calculate.

Drat.  I guess I'll try to upload a fixed version of this patch in a separate
review.

[Charlie Reis, 2012-11-16 20:11:52]

On 2012/11/16 20:01:43, creis wrote:
> On 2012/11/16 19:58:27, I haz the power (commit-bot) wrote:
> > Presubmit check for 11362268-11004 failed and returned exit status 1.
> > 
> > 
> > Running presubmit commit checks ...
> > 
> > ** Presubmit Warnings **
> > Your #include order seems to be broken. Send mail to
> > mailto:marja@chromium.org if this is not the case.
> >       content/browser/storage_partition_impl.cc:10 \
> >       content/browser/storage_partition_impl.cc:19
> > 
> > Presubmit checks took 1.8s to calculate.
> 
> Drat.  I guess I'll try to upload a fixed version of this patch in a separate
> review.

Moved review to https://codereview.chromium.org/11280030/, since Albert isn't
around to fix the include order.