Add Query() support to FileRef

webkit/plugins/ppapi/ppb_file_ref_impl.cc

Index: webkit/plugins/ppapi/ppb_file_ref_impl.cc
diff --git a/webkit/plugins/ppapi/ppb_file_ref_impl.cc b/webkit/plugins/ppapi/ppb_file_ref_impl.cc
index aacc016a8860384b75056349f637a36ab5d0187d..b6d549f07f9c123761ee12c276df49855993f62e 100644
--- a/webkit/plugins/ppapi/ppb_file_ref_impl.cc
+++ b/webkit/plugins/ppapi/ppb_file_ref_impl.cc
@@ -4,15 +4,17 @@
 
 #include "webkit/plugins/ppapi/ppb_file_ref_impl.h"
 
+#include "base/platform_file.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "googleurl/src/gurl.h"
 #include "net/base/escape.h"
 #include "ppapi/c/pp_errors.h"
-#include "ppapi/thunk/enter.h"
-#include "ppapi/thunk/ppb_file_system_api.h"
+#include "ppapi/shared_impl/file_type_conversion.h"
 #include "ppapi/shared_impl/time_conversion.h"
 #include "ppapi/shared_impl/var.h"
+#include "ppapi/thunk/enter.h"
+#include "ppapi/thunk/ppb_file_system_api.h"
 #include "webkit/plugins/ppapi/common.h"
 #include "webkit/plugins/ppapi/file_callbacks.h"
 #include "webkit/plugins/ppapi/plugin_delegate.h"
@@ -92,14 +94,16 @@ PPB_FileRef_Impl::PPB_FileRef_Impl(const PPB_FileRef_CreateInfo& info,
                                    PPB_FileSystem_Impl* file_system)
     : PPB_FileRef_Shared(::ppapi::OBJECT_IS_IMPL, info),
       file_system_(file_system),
-      external_file_system_path_() {
+      external_file_system_path_(),
+      weak_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
 }
 
 PPB_FileRef_Impl::PPB_FileRef_Impl(const PPB_FileRef_CreateInfo& info,
                                    const base::FilePath& external_file_path)
     : PPB_FileRef_Shared(::ppapi::OBJECT_IS_IMPL, info),
       file_system_(),
-      external_file_system_path_(external_file_path) {
+      external_file_system_path_(external_file_path),
+      weak_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
 }
 
 PPB_FileRef_Impl::~PPB_FileRef_Impl() {
@@ -293,5 +297,78 @@ bool PPB_FileRef_Impl::IsValidNonExternalFileSystem() const {
       file_system_->type() != PP_FILESYSTEMTYPE_EXTERNAL;
 }
 
+int32_t PPB_FileRef_Impl::Query(PP_FileInfo* info,
+                                scoped_refptr<TrackedCallback> callback) {
+  PluginInstance* plugin_instance = ResourceHelper::GetPluginInstance(this);
+  if (!plugin_instance)
+    return PP_ERROR_FAILED;
+
+  if (!file_system_) {
+    // External filesystem
+    // We have to do something totally different for external filesystems.

[yzshen1, 2013/03/26 23:32:42]

nit: file systems.

+    if (!plugin_instance->delegate()->AsyncOpenFile(
+        GetSystemPath(), base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_READ,
+        base::Bind(&PPB_FileRef_Impl::QueryCallback,
+                   weak_factory_.GetWeakPtr(), info, callback)))
+      return PP_ERROR_FAILED;
+  } else {
+    // Non-external filesystem

[yzshen1, 2013/03/26 23:32:42]

Just out of curiosity: I wonder why we have IsValidNonExternalFileSystem(), if
(file_system.get() != NULL) ==> non-external filesystem.
(That method has a check about file_system_->type().)

+    if (!HasValidFileSystem())
+      return PP_ERROR_NOACCESS;
+
+    if (!plugin_instance->delegate()->Query(
+            GetFileSystemURL(),
+            new FileCallbacks(this, callback, info, file_system_)))
+      return PP_ERROR_FAILED;
+
+  }
+  return PP_OK_COMPLETIONPENDING;
+}
+
+void PPB_FileRef_Impl::QueryCallback(PP_FileInfo* info,
+                                     scoped_refptr<TrackedCallback> callback,
+                                     base::PlatformFileError error_code,
+                                     base::PassPlatformFile passed_file) {
+  int32_t pp_error = ::ppapi::PlatformFileErrorToPepperError(error_code);

[yzshen1, 2013/03/26 23:32:42]

Please test that whether |callback| is still pending. If not you could return
early.

+  if (pp_error != PP_OK)
+    callback->Run(pp_error);
+  base::PlatformFile file = passed_file.ReleaseValue();

[yzshen1, 2013/03/26 23:32:42]

It seems the file handle is leaked?
(Besides, because you are using weakptr factory, it is not reliable to close the
file handle in GetFileInfoCallback().)

+
+  PluginInstance* plugin_instance = ResourceHelper::GetPluginInstance(this);
+  if (!plugin_instance)
+    callback->Run(PP_ERROR_FAILED);
+
+  if (!base::FileUtilProxy::GetFileInfoFromPlatformFile(
+      plugin_instance->delegate()->GetFileThreadMessageLoopProxy(), file,
+      base::Bind(&PPB_FileRef_Impl::GetFileInfoCallback,
+                 weak_factory_.GetWeakPtr(), info, callback)))
+    callback->Run(PP_ERROR_FAILED);
+}
+
+void PPB_FileRef_Impl::GetFileInfoCallback(
+    PP_FileInfo* info,
+    scoped_refptr<TrackedCallback> callback,
+    base::PlatformFileError error_code,
+    const base::PlatformFileInfo& file_info) {
+  int32_t pp_error = ::ppapi::PlatformFileErrorToPepperError(error_code);

[yzshen1, 2013/03/26 23:32:42]

Same here, please test whether |callback| is still pending, if not, you
shouldn't touch |info|. It may be invalid.

+  if (pp_error != PP_OK)
+    callback->Run(pp_error);
+
+  info->size = file_info.size;
+  if (file_info.is_symbolic_link)
+    info->type = PP_FILETYPE_OTHER;
+  else if (file_info.is_directory)
+    info->type = PP_FILETYPE_DIRECTORY;
+  else
+    info->type = PP_FILETYPE_REGULAR;
+
+  info->system_type = PP_FILESYSTEMTYPE_EXTERNAL;
+  info->creation_time = file_info.creation_time.ToDoubleT();
+  info->last_access_time = file_info.last_accessed.ToDoubleT();
+  info->last_modified_time = file_info.last_modified.ToDoubleT();
+  callback->Run(PP_OK);
+}
+
+
 }  // namespace ppapi
 }  // namespace webkit