Fixes a crash source in test_shell when running DB tests....

Fixes a crash source in test_shell when running DB tests.

BUG=none
TEST=none

[michaeln, 2009-11-18 00:35:39]

http://codereview.chromium.org/404013/diff/1/3
File webkit/tools/test_shell/simple_database_system.cc (right):

http://codereview.chromium.org/404013/diff/1/3#newcode107
Line 107: file_name = file_names_[vfs_file_name];
This repeated block of code really could go in a method so its not repeated so
many times... maybe call that method

FilePath GetFullFilePathForVfsFile(vfs_file_name) {
  AutoLock file_names_auto_lock(file_names_lock_);
  DCHECK(file_names_.find(vfs_file_name) != file_names_.end());
  return file_names_[vfs_file_name];
}

Then your callsites would look a lot more like they used to...

return VfsBackend::GetFileSize(
    GetFullFilePathForVfsFile(vfs_file));

http://codereview.chromium.org/404013/diff/1/3#newcode131
Line 131: FilePath::FromWStringHack(file_name.ToWStringHack() +
This new block of code could also go into its own private method... something
like...

void SetFullFilePathsForVfsFile(origin_identifier, database_name) {
  ...
}

Then you could document in the .h file why this pair of methods are needed.

[dumi, 2009-11-18 01:14:26]

http://codereview.chromium.org/404013/diff/1/3
File webkit/tools/test_shell/simple_database_system.cc (right):

http://codereview.chromium.org/404013/diff/1/3#newcode107
Line 107: file_name = file_names_[vfs_file_name];
On 2009/11/18 00:35:39, michaeln wrote:
> This repeated block of code really could go in a method so its not repeated so
> many times... maybe call that method
> 
> FilePath GetFullFilePathForVfsFile(vfs_file_name) {
>   AutoLock file_names_auto_lock(file_names_lock_);
>   DCHECK(file_names_.find(vfs_file_name) != file_names_.end());
>   return file_names_[vfs_file_name];
> }
> 
> Then your callsites would look a lot more like they used to...
> 
> return VfsBackend::GetFileSize(
>     GetFullFilePathForVfsFile(vfs_file));

done.

http://codereview.chromium.org/404013/diff/1/3#newcode131
Line 131: FilePath::FromWStringHack(file_name.ToWStringHack() +
On 2009/11/18 00:35:39, michaeln wrote:
> This new block of code could also go into its own private method... something
> like...
> 
> void SetFullFilePathsForVfsFile(origin_identifier, database_name) {
>   ...
> }
> 
> Then you could document in the .h file why this pair of methods are needed.

done.

[michaeln, 2009-11-18 01:31:57]

a couple of things... otherwise LGTM

http://codereview.chromium.org/404013/diff/5001/5003
File webkit/tools/test_shell/simple_database_system.cc (right):

http://codereview.chromium.org/404013/diff/5001/5003#newcode140
Line 140: file_util::Delete(db_tracker_->DatabaseDirectory(), true);
I think you should clear the file_names_ collection here too just to be complete
about clearing things.

http://codereview.chromium.org/404013/diff/5001/5002
File webkit/tools/test_shell/simple_database_system.h (right):

http://codereview.chromium.org/404013/diff/5001/5002#newcode59
Line 59: // only on the main thread. However, the VFS calls run on the DB thread
nit: one of the two only's could go

[dumi, 2009-11-18 01:42:09]

http://codereview.chromium.org/404013/diff/5001/5003
File webkit/tools/test_shell/simple_database_system.cc (right):

http://codereview.chromium.org/404013/diff/5001/5003#newcode140
Line 140: file_util::Delete(db_tracker_->DatabaseDirectory(), true);
On 2009/11/18 01:31:57, michaeln wrote:
> I think you should clear the file_names_ collection here too just to be
complete
> about clearing things.

done.

http://codereview.chromium.org/404013/diff/5001/5002
File webkit/tools/test_shell/simple_database_system.h (right):

http://codereview.chromium.org/404013/diff/5001/5002#newcode59
Line 59: // only on the main thread. However, the VFS calls run on the DB thread
On 2009/11/18 01:31:57, michaeln wrote:
> nit: one of the two only's could go

oops, done.