Integrated obfuscation with quota; all unit tests now pass

Integrated obfuscation with quota; all unit tests now pass with or without the flag that turns on obfuscation.

BUG=none
TEST=unit tests


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=86351

[kinuko, 2011-05-23 05:15:41]

nice to see the code is getting formed

http://codereview.chromium.org/7057032/diff/2001/webkit/fileapi/sandbox_mount...
File webkit/fileapi/sandbox_mount_point_provider.cc (right):

http://codereview.chromium.org/7057032/diff/2001/webkit/fileapi/sandbox_mount...
webkit/fileapi/sandbox_mount_point_provider.cc:551: return
base_path.AppendASCII(FileSystemUsageCache::kUsageFileName);
In obfuscated cases we create this file under the origin/{'t', 'p'} directory
but since it's not added to the db it won't be visible to the users... correct? 
(If fsck code may become a bit complicated because of this we can create a file
under the origin directory with a name like '.usage-t' '.usage-p' or something
like that.  If that's not the case lgtm for this particular part)

[ericu, 2011-05-23 05:23:25]

http://codereview.chromium.org/7057032/diff/2001/webkit/fileapi/sandbox_mount...
File webkit/fileapi/sandbox_mount_point_provider.cc (right):

http://codereview.chromium.org/7057032/diff/2001/webkit/fileapi/sandbox_mount...
webkit/fileapi/sandbox_mount_point_provider.cc:551: return
base_path.AppendASCII(FileSystemUsageCache::kUsageFileName);
On 2011/05/23 05:15:41, kinuko wrote:
> In obfuscated cases we create this file under the origin/{'t', 'p'} directory
> but since it's not added to the db it won't be visible to the users...
correct? 
> (If fsck code may become a bit complicated because of this we can create a
file
> under the origin directory with a name like '.usage-t' '.usage-p' or something
> like that.  If that's not the case lgtm for this particular part)

Right, the usage file is FileSystem/[origin #]/t/[usage file name], and since
it's not in the database, it's invisible to the user.
The data files are e.g. FileSystem/000/t/01/00000102 [or
FileSystem/000/t/Legacy/path/to/file.txt for migrated files], so it will be easy
for the fsck code to tell them apart from FileSystem/000/t/.usage.

[kinuko, 2011-05-23 05:54:28]

http://codereview.chromium.org/7057032/diff/2001/webkit/fileapi/sandbox_mount...
File webkit/fileapi/sandbox_mount_point_provider.cc (right):

http://codereview.chromium.org/7057032/diff/2001/webkit/fileapi/sandbox_mount...
webkit/fileapi/sandbox_mount_point_provider.cc:551: return
base_path.AppendASCII(FileSystemUsageCache::kUsageFileName);
On 2011/05/23 05:23:26, ericu wrote:
> On 2011/05/23 05:15:41, kinuko wrote:
> > In obfuscated cases we create this file under the origin/{'t', 'p'}
directory
> > but since it's not added to the db it won't be visible to the users...
> correct? 
> > (If fsck code may become a bit complicated because of this we can create a
> file
> > under the origin directory with a name like '.usage-t' '.usage-p' or
something
> > like that.  If that's not the case lgtm for this particular part)
> 
> Right, the usage file is FileSystem/[origin #]/t/[usage file name], and since
> it's not in the database, it's invisible to the user.
> The data files are e.g. FileSystem/000/t/01/00000102 [or
> FileSystem/000/t/Legacy/path/to/file.txt for migrated files], so it will be
easy
> for the fsck code to tell them apart from FileSystem/000/t/.usage.

thx, sgtm.

[michaeln, 2011-05-23 16:15:18]

lgtm

http://codereview.chromium.org/7057032/diff/2001/webkit/fileapi/obfuscated_fi...
File webkit/fileapi/obfuscated_file_system_file_util.cc (right):

http://codereview.chromium.org/7057032/diff/2001/webkit/fileapi/obfuscated_fi...
webkit/fileapi/obfuscated_file_system_file_util.cc:613: : origins_(origins),
base_path_(base_path) {
maybe avoid copying the 'origins' vector

http://codereview.chromium.org/7057032/diff/2001/webkit/fileapi/obfuscated_fi...
webkit/fileapi/obfuscated_file_system_file_util.cc:645: OriginRecord current_;
a comment about what .first and .second are would have helped, makes me think a
struct with named data members would be better for this data type

[ericu, 2011-05-23 18:05:01]

http://codereview.chromium.org/7057032/diff/2001/webkit/fileapi/obfuscated_fi...
File webkit/fileapi/obfuscated_file_system_file_util.cc (right):

http://codereview.chromium.org/7057032/diff/2001/webkit/fileapi/obfuscated_fi...
webkit/fileapi/obfuscated_file_system_file_util.cc:613: : origins_(origins),
base_path_(base_path) {
On 2011/05/23 16:15:18, michaeln wrote:
> maybe avoid copying the 'origins' vector

I could do this by making it swap() with the input parameter, destroying it. 
However, it wouldn't be the most intuitive interface.  I expect this code to be
called rarely, only for quota operations, and I don't expect the vector to be
all that large, so I'd rather not optimize this bit at the expense of clarity
until we know there's a problem here.

http://codereview.chromium.org/7057032/diff/2001/webkit/fileapi/obfuscated_fi...
webkit/fileapi/obfuscated_file_system_file_util.cc:645: OriginRecord current_;
On 2011/05/23 16:15:18, michaeln wrote:
> a comment about what .first and .second are would have helped, makes me think
a
> struct with named data members would be better for this data type

Changed to struct.

[michaeln, 2011-05-23 18:20:21]

http://codereview.chromium.org/7057032/diff/2001/webkit/fileapi/obfuscated_fi...
File webkit/fileapi/obfuscated_file_system_file_util.cc (right):

http://codereview.chromium.org/7057032/diff/2001/webkit/fileapi/obfuscated_fi...
webkit/fileapi/obfuscated_file_system_file_util.cc:613: : origins_(origins),
base_path_(base_path) {
On 2011/05/23 18:05:01, ericu wrote:
> On 2011/05/23 16:15:18, michaeln wrote:
> > maybe avoid copying the 'origins' vector
> 
> I could do this by making it swap() with the input parameter, destroying it. 
> However, it wouldn't be the most intuitive interface.  I expect this code to
be
> called rarely, only for quota operations, and I don't expect the vector to be
> all that large, so I'd rather not optimize this bit at the expense of clarity
> until we know there's a problem here.

That would be a funky interface. You could pass a ptr to the origin_database to
the ctor and have it call ListOrigins (if non-null).

[ericu, 2011-05-23 18:31:42]

http://codereview.chromium.org/7057032/diff/2001/webkit/fileapi/obfuscated_fi...
File webkit/fileapi/obfuscated_file_system_file_util.cc (right):

http://codereview.chromium.org/7057032/diff/2001/webkit/fileapi/obfuscated_fi...
webkit/fileapi/obfuscated_file_system_file_util.cc:613: : origins_(origins),
base_path_(base_path) {
On 2011/05/23 18:20:21, michaeln wrote:
> On 2011/05/23 18:05:01, ericu wrote:
> > On 2011/05/23 16:15:18, michaeln wrote:
> > > maybe avoid copying the 'origins' vector
> > 
> > I could do this by making it swap() with the input parameter, destroying it.

> > However, it wouldn't be the most intuitive interface.  I expect this code to
> be
> > called rarely, only for quota operations, and I don't expect the vector to
be
> > all that large, so I'd rather not optimize this bit at the expense of
clarity
> > until we know there's a problem here.
> 
> That would be a funky interface. You could pass a ptr to the origin_database
to
> the ctor and have it call ListOrigins (if non-null).

Oh, of course.  That's a much better idea.  Done.

[commit-bot: I haz the power, 2011-05-23 18:55:19]

Try job failure for 7057032-14001 on linux:
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux&numb...

[commit-bot: I haz the power, 2011-05-23 22:06:29]

Change committed as 86351