More filesystem cleanup: convert URL-encoded-as-FilePath to actual URL, where

webkit/plugins/ppapi/ppb_file_ref_impl.cc

Index: webkit/plugins/ppapi/ppb_file_ref_impl.cc
===================================================================
--- webkit/plugins/ppapi/ppb_file_ref_impl.cc	(revision 79631)
+++ webkit/plugins/ppapi/ppb_file_ref_impl.cc	(working copy)
@@ -6,6 +6,7 @@
 
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
+#include "googleurl/src/gurl.h"
 #include "ppapi/c/pp_errors.h"
 #include "webkit/plugins/ppapi/common.h"
 #include "webkit/plugins/ppapi/file_callbacks.h"
@@ -126,7 +127,7 @@
 
   PluginInstance* instance = file_system->instance();
   if (!instance->delegate()->MakeDirectory(
-          directory_ref->GetSystemPath(), PPBoolToBool(make_ancestors),
+          directory_ref->GetFileSystemURL(), PPBoolToBool(make_ancestors),
           new FileCallbacks(instance->module()->AsWeakPtr(), directory_ref_id,
                             callback, NULL, NULL, NULL)))
     return PP_ERROR_FAILED;
@@ -150,7 +151,8 @@
 
   PluginInstance* instance = file_system->instance();
   if (!instance->delegate()->Touch(
-          file_ref->GetSystemPath(), base::Time::FromDoubleT(last_access_time),
+          file_ref->GetFileSystemURL(),
+          base::Time::FromDoubleT(last_access_time),
           base::Time::FromDoubleT(last_modified_time),
           new FileCallbacks(instance->module()->AsWeakPtr(), file_ref_id,
                             callback, NULL, NULL, NULL)))
@@ -173,7 +175,7 @@
 
   PluginInstance* instance = file_system->instance();
   if (!instance->delegate()->Delete(
-          file_ref->GetSystemPath(),
+          file_ref->GetFileSystemURL(),
           new FileCallbacks(instance->module()->AsWeakPtr(), file_ref_id,
                             callback, NULL, NULL, NULL)))
     return PP_ERROR_FAILED;
@@ -204,7 +206,7 @@
   // http://crbug.com/67624
   PluginInstance* instance = file_system->instance();
   if (!instance->delegate()->Rename(
-          file_ref->GetSystemPath(), new_file_ref->GetSystemPath(),
+          file_ref->GetFileSystemURL(), new_file_ref->GetFileSystemURL(),
           new FileCallbacks(instance->module()->AsWeakPtr(), file_ref_id,
                             callback, NULL, NULL, NULL)))
     return PP_ERROR_FAILED;
@@ -320,22 +322,23 @@
 }
 
 FilePath PPB_FileRef_Impl::GetSystemPath() const {
-  if (GetFileSystemType() == PP_FILESYSTEMTYPE_EXTERNAL)
-    return system_path_;
+  if (GetFileSystemType() != PP_FILESYSTEMTYPE_EXTERNAL) {
+    NOTREACHED();
+    return FilePath();
+  }
+  return system_path_;
+}
 
-  // Since |virtual_path_| starts with a '/', it is considered an absolute path
-  // on POSIX systems. We need to remove the '/' before calling Append() or we
-  // will run into a DCHECK.
-  FilePath virtual_file_path(
-#if defined(OS_WIN)
-      UTF8ToWide(virtual_path_.substr(1))
-#elif defined(OS_POSIX)
-      virtual_path_.substr(1)
-#else
-#error "Unsupported platform."
-#endif
-  );
-  return file_system_->root_path().Append(virtual_file_path);
+GURL PPB_FileRef_Impl::GetFileSystemURL() const {
+  if (GetFileSystemType() != PP_FILESYSTEMTYPE_LOCALPERSISTENT &&
+      GetFileSystemType() != PP_FILESYSTEMTYPE_LOCALTEMPORARY) {
+    NOTREACHED();
+    return GURL();
+  }
+  // Since |virtual_path_| starts with a '/', it looks like an absolute path.
+  // We need to trim off the '/' before calling Resolve, as FileSystem URLs
+  // start with a storage type identifier that looks like a path segment.
+  return file_system_->root_url().Resolve(virtual_path_.substr(1));

[viettrungluu, 2011/04/06 22:38:25]

Should you check that virtual_path_ isn't empty (maybe in the above if)? (I
suppose the old version had this potential issue too.)

[ericu, 2011/04/06 23:30:36]

Fixed, thanks.

 }
 
 }  // namespace ppapi