CRX FileSystem Pepper private API

ppapi/proxy/ext_crx_file_system_private_resource.cc

Index: ppapi/proxy/ext_crx_file_system_private_resource.cc
diff --git a/ppapi/proxy/ext_crx_file_system_private_resource.cc b/ppapi/proxy/ext_crx_file_system_private_resource.cc
new file mode 100644
index 0000000000000000000000000000000000000000..492a83bcc02a13d72d5f7ca2a58b26397ea154a7
--- /dev/null
+++ b/ppapi/proxy/ext_crx_file_system_private_resource.cc
@@ -0,0 +1,70 @@
+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ppapi/proxy/ext_crx_file_system_private_resource.h"
+
+#include "base/bind.h"
+#include "ppapi/c/pp_errors.h"
+#include "ppapi/proxy/ppapi_messages.h"
+#include "ppapi/proxy/resource_message_params.h"
+#include "ppapi/shared_impl/host_resource.h"
+#include "ppapi/shared_impl/tracked_callback.h"
+#include "ppapi/thunk/enter.h"
+
+namespace ppapi {
+namespace proxy {
+
+ExtCrxFileSystemPrivateResource::ExtCrxFileSystemPrivateResource(
+    Connection connection, PP_Instance instance)
+    : PluginResource(connection, instance) {
+  SendCreate(BROWSER, PpapiHostMsg_Ext_CrxFileSystem_Create());
+}
+
+ExtCrxFileSystemPrivateResource::~ExtCrxFileSystemPrivateResource() {
+}
+
+thunk::PPB_Ext_CrxFileSystem_Private_API*
+ExtCrxFileSystemPrivateResource::AsPPB_Ext_CrxFileSystem_Private_API() {
+  return this;
+}
+
+int32_t ExtCrxFileSystemPrivateResource::Open(
+    PP_Resource* file_system_resource,
+    scoped_refptr<TrackedCallback> callback) {
+  DCHECK(file_system_resource);

[yzshen1, 2013/05/02 23:22:01]

Please return failure if necessary instead of DCHECK. (Because this is used by a
public API.)

[victorhsieh, 2013/05/03 17:26:58]

Done.

+
+  if (!TrackedCallback::IsPending(callback))

[yzshen1, 2013/05/02 23:22:01]

If you want to check whether it is in progress, you should check the callback
you received in a previous call, or have a boolean flag.

[victorhsieh, 2013/05/03 17:26:58]

Done.

+    return PP_ERROR_INPROGRESS;
+
+  Call<PpapiPluginMsg_Ext_CrxFileSystem_BrowserOpenReply>(BROWSER,
+      PpapiHostMsg_Ext_CrxFileSystem_BrowserOpen(),
+      base::Bind(&ExtCrxFileSystemPrivateResource::OnBrowserOpenComplete, this,
+                 file_system_resource,
+                 callback));
+  return PP_OK_COMPLETIONPENDING;
+}
+
+void ExtCrxFileSystemPrivateResource::OnBrowserOpenComplete(
+    PP_Resource* file_system_resource,
+    scoped_refptr<TrackedCallback> callback,
+    const ResourceMessageReplyParams& params,
+    const std::string& fsid) {

[yzshen1, 2013/05/02 23:22:01]

Test whether callback is still pending before doing other things. It may have
been aborted and in that case |file_system_resource| may have been released by
the plugin.

[victorhsieh, 2013/05/03 17:26:58]

Done.

+  if (params.result() != PP_OK) {
+    callback->Run(params.result());
+    return;
+  }
+
+  thunk::EnterResourceCreationNoLock enter(pp_instance());
+  if (enter.failed()) {
+    callback->Run(enter.retval());
+    return;

[yzshen1, 2013/05/02 23:22:01]

[I don't know the answer but would like to make sure you have considered that]
do we need to somehow notify the browser if we never pass this fsid to the
renderer. Will the file system stay open forever?

[victorhsieh, 2013/05/03 17:26:58]

Isolated filesystem will live until the end of renderer process (see
crbug.com/142533).

+  }
+
+  *file_system_resource = enter.functions()->CreateIsolatedFileSystem(
+      pp_instance(), fsid.c_str());
+  callback->Run(*file_system_resource > 0 ? PP_OK : PP_ERROR_FAILED);
+}
+
+}  // namespace proxy
+}  // namespace ppapi