Adding Size() to AbstractFileEnumerator

BUG=94060
TEST='ObfuscatedFileSystemFileUtil.*'


Review URL: http://codereview.chromium.org/7717020

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@98769 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 81 insertions, 36 deletions

diff --git a/webkit/fileapi/file_system_file_util.cc b/webkit/fileapi/file_system_file_util.cc
index c6b4fac..467c38e 100644
--- a/webkit/fileapi/file_system_file_util.cc
+++ b/webkit/fileapi/file_system_file_util.cc
@@ -520,21 +520,28 @@ class FileSystemFileEnumerator
 
   ~FileSystemFileEnumerator() {}
 
-  virtual FilePath Next();
-  virtual bool IsDirectory();
+  virtual FilePath Next() OVERRIDE;
+  virtual int64 Size() OVERRIDE;
+  virtual bool IsDirectory() OVERRIDE;
 
  private:
   file_util::FileEnumerator file_enum_;
+  file_util::FileEnumerator::FindInfo file_util_info_;
 };
 
 FilePath FileSystemFileEnumerator::Next() {
-  return file_enum_.Next();
+  FilePath rv = file_enum_.Next();
+  if (!rv.empty())
+    file_enum_.GetFindInfo(&file_util_info_);
+  return rv;
+}
+
+int64 FileSystemFileEnumerator::Size() {
+  return file_util::FileEnumerator::GetFilesize(file_util_info_);
 }
 
 bool FileSystemFileEnumerator::IsDirectory() {
-  file_util::FileEnumerator::FindInfo file_util_info;
-  file_enum_.GetFindInfo(&file_util_info);
-  return file_util::FileEnumerator::IsDirectory(file_util_info);
+  return file_util::FileEnumerator::IsDirectory(file_util_info_);
 }
 
 FileSystemFileUtil::AbstractFileEnumerator*
diff --git a/webkit/fileapi/file_system_file_util.h b/webkit/fileapi/file_system_file_util.h
index a2788da..8372784 100644
--- a/webkit/fileapi/file_system_file_util.h
+++ b/webkit/fileapi/file_system_file_util.h
@@ -5,6 +5,8 @@
 #ifndef WEBKIT_FILEAPI_FILE_SYSTEM_FILE_UTIL_H_
 #define WEBKIT_FILEAPI_FILE_SYSTEM_FILE_UTIL_H_
 
+#include <vector>
+
 #include "base/callback.h"
 #include "base/file_path.h"
 #include "base/file_util.h"
@@ -223,20 +225,25 @@ class FileSystemFileUtil {
     // Returns an empty string if there are no more results.
     virtual FilePath Next() = 0;
 
+    virtual int64 Size() = 0;
     virtual bool IsDirectory() = 0;
   };
 
   class EmptyFileEnumerator : public AbstractFileEnumerator {
-    virtual FilePath Next() { return FilePath(); }
-    virtual bool IsDirectory() { return false; }
+    virtual FilePath Next() OVERRIDE { return FilePath(); }
+    virtual int64 Size() OVERRIDE { return 0; }
+    virtual bool IsDirectory() OVERRIDE { return false; }
   };
 
   // Returns a pointer to a new instance of AbstractFileEnumerator which is
   // implemented for each FileUtil subclass. The instance needs to be freed
   // by the caller, and its lifetime should not extend past when the current
   // call returns to the main FILE message loop.
+  //
+  // The supplied context must remain valid at least lifetime of the enumerator
+  // instance.
   virtual AbstractFileEnumerator* CreateFileEnumerator(
-      FileSystemOperationContext* unused,
+      FileSystemOperationContext* context,
       const FilePath& root_path);
 
  protected:
diff --git a/webkit/fileapi/local_file_system_file_util.cc b/webkit/fileapi/local_file_system_file_util.cc
index ca028e4..0ed3b94 100644
--- a/webkit/fileapi/local_file_system_file_util.cc
+++ b/webkit/fileapi/local_file_system_file_util.cc
@@ -241,11 +241,13 @@ class LocalFileSystemFileEnumerator
 
   ~LocalFileSystemFileEnumerator() {}
 
-  virtual FilePath Next();
-  virtual bool IsDirectory();
+  virtual FilePath Next() OVERRIDE;
+  virtual int64 Size() OVERRIDE;
+  virtual bool IsDirectory() OVERRIDE;
 
  private:
   file_util::FileEnumerator file_enum_;
+  file_util::FileEnumerator::FindInfo file_util_info_;
   FilePath platform_root_path_;
   FilePath virtual_root_path_;
 };
@@ -254,16 +256,19 @@ FilePath LocalFileSystemFileEnumerator::Next() {
   FilePath next = file_enum_.Next();
   if (next.empty())
     return next;
+  file_enum_.GetFindInfo(&file_util_info_);
 
   FilePath path;
   platform_root_path_.AppendRelativePath(next, &path);
   return virtual_root_path_.Append(path);
 }
 
+int64 LocalFileSystemFileEnumerator::Size() {
+  return file_util::FileEnumerator::GetFilesize(file_util_info_);
+}
+
 bool LocalFileSystemFileEnumerator::IsDirectory() {
-  file_util::FileEnumerator::FindInfo file_util_info;
-  file_enum_.GetFindInfo(&file_util_info);
-  return file_util::FileEnumerator::IsDirectory(file_util_info);
+  return file_util::FileEnumerator::IsDirectory(file_util_info_);
 }
 
 FileSystemFileUtil::AbstractFileEnumerator*
diff --git a/webkit/fileapi/obfuscated_file_system_file_util.cc b/webkit/fileapi/obfuscated_file_system_file_util.cc
index ee284e2..0b1cb90 100644
--- a/webkit/fileapi/obfuscated_file_system_file_util.cc
+++ b/webkit/fileapi/obfuscated_file_system_file_util.cc
@@ -5,6 +5,7 @@
 #include "webkit/fileapi/obfuscated_file_system_file_util.h"
 
 #include <queue>
+#include <string>
 #include <vector>
 
 #include "base/file_util.h"
@@ -50,8 +51,8 @@ bool IsRootDirectory(const FilePath& virtual_path) {
 // which FilePath uses wide chars [since they're converted to UTF-8 for storage
 // anyway], but as long as the cost is high enough that one can't cheat on quota
 // by storing data in paths, it doesn't need to be all that accurate.
-const int64 kPathCreationQuotaCost = 146; // Bytes per inode, basically.
-const int64 kPathByteQuotaCost = 2; // Bytes per byte of path length in UTF-8.
+const int64 kPathCreationQuotaCost = 146;  // Bytes per inode, basically.
+const int64 kPathByteQuotaCost = 2;  // Bytes per byte of path length in UTF-8.
 
 int64 GetPathQuotaUsage(
     int growth_in_number_of_paths,
@@ -79,7 +80,7 @@ void UpdatePathQuotaUsage(
     fileapi::FileSystemOperationContext* context,
     const GURL& origin_url,
     fileapi::FileSystemType type,
-    int growth_in_number_of_paths, // -1, 0, or 1
+    int growth_in_number_of_paths,  // -1, 0, or 1
     int64 growth_in_bytes_of_path_length) {
   int64 growth = GetPathQuotaUsage(growth_in_number_of_paths,
       growth_in_bytes_of_path_length);
@@ -646,8 +647,19 @@ class ObfuscatedFileSystemFileEnumerator
     : public FileSystemFileUtil::AbstractFileEnumerator {
  public:
   ObfuscatedFileSystemFileEnumerator(
-      FileSystemDirectoryDatabase* db, const FilePath& virtual_root_path)
-      : db_(db) {
+      const FilePath& base_path,
+      FileSystemDirectoryDatabase* db,
+      FileSystemOperationContext* context,
+      FileSystemFileUtil* underlying_file_util,
+      const FilePath& virtual_root_path)
+      : base_path_(base_path),
+        db_(db),
+        context_(context),
+        underlying_file_util_(underlying_file_util) {
+    DCHECK(db_);
+    DCHECK(context_);
+    DCHECK(underlying_file_util_);
+
     FileId file_id;
     FileInfo file_info;
     if (!db_->GetFileWithPath(virtual_root_path, &file_id))
@@ -663,7 +675,7 @@ class ObfuscatedFileSystemFileEnumerator
 
   ~ObfuscatedFileSystemFileEnumerator() {}
 
-  virtual FilePath Next() {
+  virtual FilePath Next() OVERRIDE {
     ProcessRecurseQueue();
     if (display_queue_.empty())
       return FilePath();
@@ -674,7 +686,24 @@ class ObfuscatedFileSystemFileEnumerator
     return current_.file_path;
   }
 
-  virtual bool IsDirectory() {
+  virtual int64 Size() OVERRIDE {
+    if (IsDirectory())
+      return 0;
+
+    base::PlatformFileInfo file_info;
+    FilePath platform_file_path;
+
+    FilePath local_path = base_path_.Append(current_.file_info.data_path);
+    base::PlatformFileError error = underlying_file_util_->GetFileInfo(
+        context_, local_path, &file_info, &platform_file_path);
+    if (error != base::PLATFORM_FILE_OK) {
+      LOG(WARNING) << "Lost a backing file.";
+      return 0;
+    }
+    return file_info.size;
+  }
+
+  virtual bool IsDirectory() OVERRIDE {
     return current_.file_info.is_directory();
   }
 
@@ -710,7 +739,10 @@ class ObfuscatedFileSystemFileEnumerator
   std::queue<FileRecord> display_queue_;
   std::queue<FileRecord> recurse_queue_;
   FileRecord current_;
+  FilePath base_path_;
   FileSystemDirectoryDatabase* db_;
+  FileSystemOperationContext* context_;
+  FileSystemFileUtil* underlying_file_util_;
 };
 
 class ObfuscatedFileSystemOriginEnumerator
@@ -775,7 +807,13 @@ ObfuscatedFileSystemFileUtil::CreateFileEnumerator(
       context->src_origin_url(), context->src_type(), false);
   if (!db)
     return new FileSystemFileUtil::EmptyFileEnumerator();
-  return new ObfuscatedFileSystemFileEnumerator(db, root_path);
+  return new ObfuscatedFileSystemFileEnumerator(
+      GetDirectoryForOriginAndType(context->src_origin_url(),
+                                   context->src_type(), false),
+      db,
+      context,
+      underlying_file_util_.get(),
+      root_path);
 }
 
 PlatformFileError ObfuscatedFileSystemFileUtil::GetFileInfoInternal(
diff --git a/webkit/fileapi/obfuscated_file_system_file_util.h b/webkit/fileapi/obfuscated_file_system_file_util.h
index a3ad15e..782fc07 100644
--- a/webkit/fileapi/obfuscated_file_system_file_util.h
+++ b/webkit/fileapi/obfuscated_file_system_file_util.h
@@ -6,6 +6,7 @@
 #define WEBKIT_FILEAPI_OBFUSCATED_FILE_SYSTEM_FILE_UTIL_H_
 
 #include <map>
+#include <string>
 #include <vector>
 
 #include "base/file_path.h"
diff --git a/webkit/fileapi/sandbox_mount_point_provider.cc b/webkit/fileapi/sandbox_mount_point_provider.cc
index dbef6c3..3c0f1d3 100644
--- a/webkit/fileapi/sandbox_mount_point_provider.cc
+++ b/webkit/fileapi/sandbox_mount_point_provider.cc
@@ -23,7 +23,6 @@
 #include "webkit/fileapi/local_file_system_file_util.h"
 #include "webkit/fileapi/obfuscated_file_system_file_util.h"
 #include "webkit/fileapi/quota_file_util.h"
-#include "webkit/fileapi/sandbox_mount_point_provider.h"
 #include "webkit/glue/webkit_glue.h"
 #include "webkit/quota/quota_manager.h"
 
@@ -546,22 +545,10 @@ int64 SandboxMountPointProvider::GetOriginUsageOnFileThread(
   FilePath file_path_each;
   int64 usage = 0;
 
-  // TODO(ericu): This could be made much more efficient if the
-  // AbstractFileEnumerator also had an interface to tell you the size of the
-  // file.  ObfuscatedFileSystemFileEnumerator has already looked up the data,
-  // and it's a big waste to look it up again.  The other implementers could
-  // easily add it on-demand, so as not to waste time when it's not needed.
   while (!(file_path_each = enumerator->Next()).empty()) {
     base::PlatformFileInfo file_info;
     FilePath platform_file_path;
-    if (!enumerator->IsDirectory()) {
-      base::PlatformFileError error = sandbox_file_util_->GetFileInfo(
-          &context, file_path_each, &file_info, &platform_file_path);
-      if (error != base::PLATFORM_FILE_OK)
-        NOTREACHED();
-      else
-        usage += file_info.size;
-    }
+    usage += enumerator->Size();
     usage += ObfuscatedFileSystemFileUtil::ComputeFilePathCost(file_path_each);
   }
   // This clears the dirty flag too.