Don't allow multiple opens for Pepper FileSystem.

BUG=73667
TEST=test_file_system.{h,cc}

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

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

5 files changed, 113 insertions, 18 deletions

diff --git a/ppapi/ppapi_tests.gypi b/ppapi/ppapi_tests.gypi
index a8f4bae..a3de9bb 100644
--- a/ppapi/ppapi_tests.gypi
+++ b/ppapi/ppapi_tests.gypi
@@ -190,6 +190,8 @@
         'tests/test_file_io.h',
         'tests/test_file_ref.cc',
         'tests/test_file_ref.h',
+        'tests/test_file_system.cc',
+        'tests/test_file_system.h',
         'tests/test_graphics_2d.cc',
         'tests/test_graphics_2d.h',
         'tests/test_image_data.cc',
diff --git a/ppapi/proxy/ppb_file_system_proxy.cc b/ppapi/proxy/ppb_file_system_proxy.cc
index 36a36f1..74c8bcc 100644
--- a/ppapi/proxy/ppb_file_system_proxy.cc
+++ b/ppapi/proxy/ppb_file_system_proxy.cc
@@ -28,7 +28,7 @@ class FileSystem : public PluginResource {
   virtual FileSystem* AsFileSystem();
 
   PP_FileSystemType_Dev type_;
-  bool opened_;
+  bool called_open_;
   PP_CompletionCallback current_open_callback_;
 
  private:
@@ -39,7 +39,7 @@ FileSystem::FileSystem(const HostResource& host_resource,
                        PP_FileSystemType_Dev type)
     : PluginResource(host_resource),
       type_(type),
-      opened_(false),
+      called_open_(false),
       current_open_callback_(PP_MakeCompletionCallback(NULL, NULL)) {
 }
 
@@ -89,8 +89,6 @@ int32_t Open(PP_Resource file_system,
   FileSystem* object = PluginResource::GetAs<FileSystem>(file_system);
   if (!object)
     return PP_ERROR_BADRESOURCE;
-  if (object->opened_)
-    return PP_OK;
 
   Dispatcher* dispatcher = PluginDispatcher::GetForInstance(object->instance());
   if (!dispatcher)
@@ -98,7 +96,11 @@ int32_t Open(PP_Resource file_system,
 
   if (object->current_open_callback_.func)
     return PP_ERROR_INPROGRESS;
+  else if (object->called_open_)
+    return PP_ERROR_FAILED;
+
   object->current_open_callback_ = callback;
+  object->called_open_ = true;
 
   dispatcher->Send(new PpapiHostMsg_PPBFileSystem_Open(
       INTERFACE_ID_PPB_FILE_SYSTEM, object->host_resource(), expected_size));
diff --git a/ppapi/tests/test_file_ref.cc b/ppapi/tests/test_file_ref.cc
index 4403646..1596333 100644
--- a/ppapi/tests/test_file_ref.cc
+++ b/ppapi/tests/test_file_ref.cc
@@ -254,20 +254,6 @@ std::string TestFileRef::TestMakeDirectory() {
   if (rv != PP_OK)
     return ReportError("FileSystem::Open", rv);
 
-  // Open aborted (see the DirectoryReader test for comments).
-  callback.reset_run_count();
-  rv = pp::FileSystem_Dev(instance_, PP_FILESYSTEMTYPE_LOCALTEMPORARY)
-      .Open(1024, callback);
-  if (callback.run_count() > 0)
-    return "FileSystem::Open ran callback synchronously.";
-  if (rv == PP_ERROR_WOULDBLOCK) {
-    rv = callback.WaitForResult();
-    if (rv != PP_ERROR_ABORTED)
-      return "FileSystem::Open not aborted.";
-  } else if (rv != PP_OK) {
-    return ReportError("FileSystem::Open", rv);
-  }
-
   // MakeDirectory.
   pp::FileRef_Dev dir_ref(file_system, "/test_dir_make_directory");
   rv = dir_ref.MakeDirectory(callback);
diff --git a/ppapi/tests/test_file_system.cc b/ppapi/tests/test_file_system.cc
new file mode 100644
index 0000000..958f896
--- /dev/null
+++ b/ppapi/tests/test_file_system.cc
@@ -0,0 +1,79 @@
+// Copyright (c) 2011 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/tests/test_file_system.h"
+
+#include <string.h>
+
+#include "ppapi/c/pp_errors.h"
+#include "ppapi/cpp/dev/file_system_dev.h"
+#include "ppapi/tests/test_utils.h"
+#include "ppapi/tests/testing_instance.h"
+
+REGISTER_TEST_CASE(FileSystem);
+
+bool TestFileSystem::Init() {
+  return InitTestingInterface() && EnsureRunningOverHTTP();
+}
+
+void TestFileSystem::RunTest() {
+  RUN_TEST(Open);
+  RUN_TEST(MultipleOpens);
+}
+
+std::string TestFileSystem::TestOpen() {
+  TestCompletionCallback callback(instance_->pp_instance());
+
+  // Open.
+  pp::FileSystem_Dev file_system(instance_, PP_FILESYSTEMTYPE_LOCALTEMPORARY);
+  int32_t rv = file_system.Open(1024, callback);
+  if (rv == PP_ERROR_WOULDBLOCK)
+    rv = callback.WaitForResult();
+  if (rv != PP_OK)
+    return ReportError("FileSystem::Open", rv);
+
+  // Open aborted (see the DirectoryReader test for comments).
+  callback.reset_run_count();
+  rv = pp::FileSystem_Dev(instance_, PP_FILESYSTEMTYPE_LOCALTEMPORARY)
+      .Open(1024, callback);
+  if (callback.run_count() > 0)
+    return "FileSystem::Open ran callback synchronously.";
+  if (rv == PP_ERROR_WOULDBLOCK) {
+    rv = callback.WaitForResult();
+    if (rv != PP_ERROR_ABORTED)
+      return "FileSystem::Open not aborted.";
+  } else if (rv != PP_OK) {
+    return ReportError("FileSystem::Open", rv);
+  }
+
+  PASS();
+}
+
+std::string TestFileSystem::TestMultipleOpens() {
+  // Should not allow multiple opens, no matter the first open has completed or
+  // not.
+  TestCompletionCallback callback_1(instance_->pp_instance());
+  pp::FileSystem_Dev file_system(instance_, PP_FILESYSTEMTYPE_LOCALTEMPORARY);
+  int32_t rv_1 = file_system.Open(1024, callback_1);
+  if (callback_1.run_count() > 0)
+    return "FileSystem::Open ran callback synchronously.";
+
+  TestCompletionCallback callback_2(instance_->pp_instance());
+  int32_t rv_2 = file_system.Open(1024, callback_2);
+  if (rv_2 == PP_ERROR_WOULDBLOCK || rv_2 == PP_OK)
+    return "FileSystem::Open should not allow multiple opens.";
+
+  if (rv_1 == PP_ERROR_WOULDBLOCK)
+    rv_1 = callback_1.WaitForResult();
+  if (rv_1 != PP_OK)
+    return ReportError("FileSystem::Open", rv_1);
+
+  TestCompletionCallback callback_3(instance_->pp_instance());
+  int32_t rv_3 = file_system.Open(1024, callback_3);
+  if (rv_3 == PP_ERROR_WOULDBLOCK || rv_3 == PP_OK)
+    return "FileSystem::Open should not allow multiple opens.";
+
+  PASS();
+}
+
diff --git a/ppapi/tests/test_file_system.h b/ppapi/tests/test_file_system.h
new file mode 100644
index 0000000..3a200f2
--- /dev/null
+++ b/ppapi/tests/test_file_system.h
@@ -0,0 +1,26 @@
+// Copyright (c) 2011 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.
+
+#ifndef PAPPI_TESTS_TEST_FILE_SYSTEM_H_
+#define PAPPI_TESTS_TEST_FILE_SYSTEM_H_
+
+#include <string>
+
+#include "ppapi/tests/test_case.h"
+
+class TestFileSystem : public TestCase {
+ public:
+  explicit TestFileSystem(TestingInstance* instance) : TestCase(instance) {}
+
+  // TestCase implementation.
+  virtual bool Init();
+  virtual void RunTest();
+
+ private:
+  std::string TestOpen();
+  std::string TestMultipleOpens();
+};
+
+#endif  // PAPPI_TESTS_TEST_FILE_SYSTEM_H_
+