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

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',

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));

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);

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();
+}
+

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_
+

webkit/plugins/ppapi/ppb_file_system_impl.cc

@@ -56,8 +56,10 @@ int32_t Open(PP_Resource file_system_id,
   if (!file_system)
     return PP_ERROR_BADRESOURCE;
 
-  if (file_system->opened())
-    return PP_OK;
+  // Should not allow multiple opens.
+  if (file_system->called_open())
+    return PP_ERROR_FAILED;
+  file_system->set_called_open();
 
   if ((file_system->type() != PP_FILESYSTEMTYPE_LOCALPERSISTENT) &&
       (file_system->type() != PP_FILESYSTEMTYPE_LOCALTEMPORARY))
@@ -100,7 +102,8 @@ PPB_FileSystem_Impl::PPB_FileSystem_Impl(PluginInstance* instance,
     : Resource(instance),
       instance_(instance),
       type_(type),
-      opened_(false) {
+      opened_(false),
+      called_open_(false) {
   DCHECK(type_ != PP_FILESYSTEMTYPE_INVALID);
 }
 

webkit/plugins/ppapi/ppb_file_system_impl.h

@@ -32,12 +32,15 @@ class PPB_FileSystem_Impl : public Resource {
   void set_root_path(const FilePath& root_path) { root_path_ = root_path; }
   bool opened() const { return opened_; }
   void set_opened(bool opened) { opened_ = opened; }
+  bool called_open() const { return called_open_; }
+  void set_called_open() { called_open_ = true; }
 
  private:
   PluginInstance* instance_;
   PP_FileSystemType_Dev type_;
   FilePath root_path_;
   bool opened_;
+  bool called_open_;
 
   DISALLOW_COPY_AND_ASSIGN(PPB_FileSystem_Impl);
 };