[NaCl SDK] nacl_io: It is now possible to open and read Html5Fs directories.

MountNodeHtml5Fs now detects when a directory is being opened and each operation
now behaves accordingly (e.g., Read and Write set EISDIR, GetStat works
correctly).

Also fixed three bugs in MountNodeHtml5Fs::GetDents:
* The correct size is returned, instead of 0.
* All inodes are set to 1, not 0 (which is reserved).
* It correctly fails with ENOTDIR if called on a regular file.

BUG=244170

Review URL: https://chromiumcodereview.appspot.com/15712004

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

4 files changed, 175 insertions, 28 deletions

diff --git a/native_client_sdk/src/libraries/nacl_io/mount_node_html5fs.cc b/native_client_sdk/src/libraries/nacl_io/mount_node_html5fs.cc
index fb0d5ed..ce32e08 100644
--- a/native_client_sdk/src/libraries/nacl_io/mount_node_html5fs.cc
+++ b/native_client_sdk/src/libraries/nacl_io/mount_node_html5fs.cc
@@ -65,6 +65,10 @@ int32_t ModeToOpenFlags(int mode) {
 }  // namespace
 
 int MountNodeHtml5Fs::FSync() {
+  // Cannot call Flush on a directory; simply do nothing.
+  if (IsDirectory())
+    return 0;
+
   int32_t result = mount_->ppapi()->GetFileIoInterface()->Flush(
       fileio_resource_, PP_BlockUntilComplete());
   if (result != PP_OK) {
@@ -88,6 +92,12 @@ int MountNodeHtml5Fs::GetDents(size_t offs, struct dirent* pdir, size_t size) {
     return -1;
   }
 
+  // If this is not a directory, fail
+  if (!IsDirectory()) {
+    errno = ENOTDIR;
+    return -1;
+  }
+
   OutputBuffer output_buf = { NULL, 0 };
   PP_ArrayOutput output = { &GetOutputBuffer, &output_buf };
   int32_t result =
@@ -124,7 +134,7 @@ int MountNodeHtml5Fs::GetDents(size_t offs, struct dirent* pdir, size_t size) {
 
     dirents.push_back(dirent());
     struct dirent& direntry = dirents.back();
-    direntry.d_ino = 0;  // TODO(binji): Is this needed?
+    direntry.d_ino = 1;  // Must be > 0.
     direntry.d_off = sizeof(struct dirent);
     direntry.d_reclen = sizeof(struct dirent);
     strncpy(direntry.d_name, file_name, file_name_length);
@@ -142,15 +152,15 @@ int MountNodeHtml5Fs::GetDents(size_t offs, struct dirent* pdir, size_t size) {
   if (offs + size >= max) size = max - offs;
 
   memcpy(pdir, reinterpret_cast<char*>(dirents.data()) + offs, size);
-  return 0;
+  return size;
 }
 
 int MountNodeHtml5Fs::GetStat(struct stat* stat) {
   AutoLock lock(&lock_);
 
   PP_FileInfo info;
-  int32_t result = mount_->ppapi()->GetFileIoInterface()->Query(
-      fileio_resource_, &info, PP_BlockUntilComplete());
+  int32_t result = mount_->ppapi()->GetFileRefInterface()->Query(
+      fileref_resource_, &info, PP_BlockUntilComplete());
   if (result != PP_OK) {
     errno = PPErrorToErrno(result);
     return -1;
@@ -175,6 +185,11 @@ int MountNodeHtml5Fs::GetStat(struct stat* stat) {
 }
 
 int MountNodeHtml5Fs::Read(size_t offs, void* buf, size_t count) {
+  if (IsDirectory()) {
+    errno = EISDIR;
+    return -1;
+  }
+
   int32_t result = mount_->ppapi()->GetFileIoInterface()->Read(
       fileio_resource_, offs, static_cast<char*>(buf),
       static_cast<int32_t>(count),
@@ -188,6 +203,11 @@ int MountNodeHtml5Fs::Read(size_t offs, void* buf, size_t count) {
 }
 
 int MountNodeHtml5Fs::FTruncate(off_t size) {
+  if (IsDirectory()) {
+    errno = EISDIR;
+    return -1;
+  }
+
   int32_t result = mount_->ppapi()->GetFileIoInterface()->SetLength(
       fileio_resource_, size, PP_BlockUntilComplete());
   if (result != PP_OK) {
@@ -199,6 +219,11 @@ int MountNodeHtml5Fs::FTruncate(off_t size) {
 }
 
 int MountNodeHtml5Fs::Write(size_t offs, const void* buf, size_t count) {
+  if (IsDirectory()) {
+    errno = EISDIR;
+    return -1;
+  }
+
   int32_t result = mount_->ppapi()->GetFileIoInterface()->Write(
       fileio_resource_, offs, static_cast<const char*>(buf),
       static_cast<int32_t>(count), PP_BlockUntilComplete());
@@ -214,8 +239,8 @@ size_t MountNodeHtml5Fs::GetSize() {
   AutoLock lock(&lock_);
 
   PP_FileInfo info;
-  int32_t result = mount_->ppapi()->GetFileIoInterface()->Query(
-      fileio_resource_, &info, PP_BlockUntilComplete());
+  int32_t result = mount_->ppapi()->GetFileRefInterface()->Query(
+      fileref_resource_, &info, PP_BlockUntilComplete());
   if (result != PP_OK) {
     errno = PPErrorToErrno(result);
     return -1;
@@ -234,6 +259,14 @@ bool MountNodeHtml5Fs::Init(int perm) {
   if (!MountNode::Init(Mount::OpenModeToPermission(perm)))
     return false;
 
+  // First query the FileRef to see if it is a file or directory.
+  PP_FileInfo file_info;
+  mount_->ppapi()->GetFileRefInterface()->Query(fileref_resource_, &file_info,
+                                                PP_BlockUntilComplete());
+  // If this is a directory, do not get a FileIO.
+  if (file_info.type == PP_FILETYPE_DIRECTORY)
+    return true;
+
   fileio_resource_= mount_->ppapi()->GetFileIoInterface()->Create(
       mount_->ppapi()->GetInstance());
   if (!fileio_resource_)
diff --git a/native_client_sdk/src/libraries/nacl_io/mount_node_html5fs.h b/native_client_sdk/src/libraries/nacl_io/mount_node_html5fs.h
index 58b4d13..e472208 100644
--- a/native_client_sdk/src/libraries/nacl_io/mount_node_html5fs.h
+++ b/native_client_sdk/src/libraries/nacl_io/mount_node_html5fs.h
@@ -33,7 +33,12 @@ class MountNodeHtml5Fs : public MountNode {
 
  private:
   PP_Resource fileref_resource_;
-  PP_Resource fileio_resource_;
+  PP_Resource fileio_resource_;  // 0 if the file is a directory.
+
+  // Returns true if this node is a directory.
+  bool IsDirectory() const {
+    return !fileio_resource_;
+  }
 
   friend class MountHtml5Fs;
 };
diff --git a/native_client_sdk/src/libraries/nacl_io/pepper/all_interfaces.h b/native_client_sdk/src/libraries/nacl_io/pepper/all_interfaces.h
index 6795736..5bab3b2 100644
--- a/native_client_sdk/src/libraries/nacl_io/pepper/all_interfaces.h
+++ b/native_client_sdk/src/libraries/nacl_io/pepper/all_interfaces.h
@@ -39,12 +39,14 @@ BEGIN_INTERFACE(FileIoInterface, PPB_FileIO, PPB_FILEIO_INTERFACE_1_0)
           const char*, int32_t, PP_CompletionCallback)
 END_INTERFACE(FileIoInterface, PPB_FileIO)
 
-BEGIN_INTERFACE(FileRefInterface, PPB_FileRef, PPB_FILEREF_INTERFACE_1_0)
+BEGIN_INTERFACE(FileRefInterface, PPB_FileRef, PPB_FILEREF_INTERFACE_1_1)
   METHOD2(FileRefInterface, PP_Resource, Create, PP_Resource, const char*)
   METHOD2(FileRefInterface, int32_t, Delete, PP_Resource, PP_CompletionCallback)
   METHOD1(FileRefInterface, PP_Var, GetName, PP_Resource)
   METHOD3(FileRefInterface, int32_t, MakeDirectory, PP_Resource, PP_Bool,
           PP_CompletionCallback)
+  METHOD3(FileRefInterface, int32_t, Query, PP_Resource, PP_FileInfo*,
+          PP_CompletionCallback)
   METHOD3(FileRefInterface, int32_t, ReadDirectoryEntries, PP_Resource,
           const PP_ArrayOutput&, PP_CompletionCallback)
 END_INTERFACE(FileRefInterface, PPB_FileRef)
diff --git a/native_client_sdk/src/libraries/nacl_io_test/mount_html5fs_test.cc b/native_client_sdk/src/libraries/nacl_io_test/mount_html5fs_test.cc
index ce3ab2d..0ca210b 100644
--- a/native_client_sdk/src/libraries/nacl_io_test/mount_html5fs_test.cc
+++ b/native_client_sdk/src/libraries/nacl_io_test/mount_html5fs_test.cc
@@ -3,6 +3,7 @@
  * found in the LICENSE file.
  */
 
+#include <errno.h>
 #include <fcntl.h>
 #include <string.h>
 #include <gmock/gmock.h>
@@ -93,7 +94,7 @@ class MountHtml5FsNodeTest : public MountHtml5FsTest {
   virtual void SetUp();
   virtual void TearDown();
 
-  void SetUpNodeExpectations();
+  void SetUpNodeExpectations(PP_FileType file_type);
   void InitFilesystem();
   void InitNode();
 
@@ -131,22 +132,32 @@ void MountHtml5FsNodeTest::TearDown() {
   }
 }
 
-void MountHtml5FsNodeTest::SetUpNodeExpectations() {
+void MountHtml5FsNodeTest::SetUpNodeExpectations(PP_FileType file_type) {
   // Open.
   EXPECT_CALL(*fileref_, Create(filesystem_resource_, StrEq(&path_[0])))
       .WillOnce(Return(fileref_resource_));
-  EXPECT_CALL(*fileio_, Create(instance_)).WillOnce(Return(fileio_resource_));
-  int32_t open_flags = PP_FILEOPENFLAG_READ | PP_FILEOPENFLAG_WRITE |
-      PP_FILEOPENFLAG_CREATE;
-  EXPECT_CALL(*fileio_,
-              Open(fileio_resource_, fileref_resource_, open_flags, _))
-      .WillOnce(Return(int32_t(PP_OK)));
+  PP_FileInfo info;
+  memset(&info, 0, sizeof(PP_FileInfo));
+  info.type = file_type;
+  EXPECT_CALL(*fileref_, Query(fileref_resource_, _, _))
+      .WillOnce(DoAll(SetArgPointee<1>(info),
+                      Return(int32_t(PP_OK))));
+  if (file_type != PP_FILETYPE_DIRECTORY) {
+    EXPECT_CALL(*fileio_, Create(instance_)).WillOnce(Return(fileio_resource_));
+    int32_t open_flags = PP_FILEOPENFLAG_READ | PP_FILEOPENFLAG_WRITE |
+        PP_FILEOPENFLAG_CREATE;
+    EXPECT_CALL(*fileio_,
+                Open(fileio_resource_, fileref_resource_, open_flags, _))
+        .WillOnce(Return(int32_t(PP_OK)));
+
+    // Close.
+    EXPECT_CALL(*fileio_, Close(fileio_resource_));
+    EXPECT_CALL(*ppapi_, ReleaseResource(fileio_resource_));
+    EXPECT_CALL(*fileio_, Flush(fileio_resource_, _));
+  }
 
   // Close.
-  EXPECT_CALL(*fileio_, Close(fileio_resource_));
   EXPECT_CALL(*ppapi_, ReleaseResource(fileref_resource_));
-  EXPECT_CALL(*ppapi_, ReleaseResource(fileio_resource_));
-  EXPECT_CALL(*fileio_, Flush(fileio_resource_, _));
 }
 
 void MountHtml5FsNodeTest::InitFilesystem() {
@@ -163,17 +174,34 @@ void MountHtml5FsNodeTest::InitNode() {
 // creation of the mount blocks until the filesystem is ready.
 class MountHtml5FsNodeSyncTest : public MountHtml5FsNodeTest {
  public:
+  void SetUpForFileType(PP_FileType file_type);
+
   virtual void SetUp();
 };
 
-void MountHtml5FsNodeSyncTest::SetUp() {
+void MountHtml5FsNodeSyncTest::SetUpForFileType(PP_FileType file_type) {
   MountHtml5FsNodeTest::SetUp();
   SetUpFilesystemExpectations(PP_FILESYSTEMTYPE_LOCALPERSISTENT, 0);
   InitFilesystem();
-  SetUpNodeExpectations();
+  SetUpNodeExpectations(file_type);
   InitNode();
 }
 
+void MountHtml5FsNodeSyncTest::SetUp() {
+  SetUpForFileType(PP_FILETYPE_REGULAR);
+}
+
+// Node test where the filesystem is opened synchronously, and the node is a
+// directory.
+class MountHtml5FsNodeSyncDirTest : public MountHtml5FsNodeSyncTest {
+ public:
+  virtual void SetUp();
+};
+
+void MountHtml5FsNodeSyncDirTest::SetUp() {
+  SetUpForFileType(PP_FILETYPE_DIRECTORY);
+}
+
 void ReadDirectoryEntriesAction(const PP_ArrayOutput& output) {
   const int fileref_resource_1 = 238;
   const int fileref_resource_2 = 239;
@@ -224,7 +252,7 @@ void MountHtml5FsNodeAsyncTest::SetUp() {
   // true => asynchronous filesystem open.
   SetUpFilesystemExpectations(PP_FILESYSTEMTYPE_LOCALPERSISTENT, 0, true);
   InitFilesystem();
-  SetUpNodeExpectations();
+  SetUpNodeExpectations(PP_FILETYPE_REGULAR);
 
   // Signal the other thread to try opening a Node.
   pthread_mutex_lock(&mutex_);
@@ -380,7 +408,7 @@ TEST_F(MountHtml5FsNodeSyncTest, GetStat) {
   info.last_access_time = access_time;
   info.last_modified_time = modified_time;
 
-  EXPECT_CALL(*fileio_, Query(fileio_resource_, _, _))
+  EXPECT_CALL(*fileref_, Query(fileref_resource_, _, _))
       .WillOnce(DoAll(SetArgPointee<1>(info),
                       Return(int32_t(PP_OK))));
 
@@ -405,6 +433,77 @@ TEST_F(MountHtml5FsNodeSyncTest, FTruncate) {
 }
 
 TEST_F(MountHtml5FsNodeSyncTest, GetDents) {
+  struct dirent dirents[2];
+  memset(&dirents[0], 0, sizeof(dirents));
+
+  // Should fail for regular files.
+  int result = node_->GetDents(0, &dirents[0], sizeof(dirent) * 2);
+  ASSERT_EQ(-1, result);
+  ASSERT_EQ(ENOTDIR, errno);
+}
+
+TEST_F(MountHtml5FsNodeSyncDirTest, OpenAndClose) {
+}
+
+TEST_F(MountHtml5FsNodeSyncDirTest, Write) {
+  const int offset = 10;
+  const int count = 20;
+  const char buffer[30] = {0};
+
+  // Should fail for directories.
+  int result = node_->Write(offset, &buffer, count);
+  ASSERT_EQ(-1, result);
+  EXPECT_EQ(EISDIR, errno);
+}
+
+TEST_F(MountHtml5FsNodeSyncDirTest, Read) {
+  const int offset = 10;
+  const int count = 20;
+  char buffer[30] = {0};
+
+  // Should fail for directories.
+  int result = node_->Read(offset, &buffer, count);
+  ASSERT_EQ(-1, result);
+  EXPECT_EQ(EISDIR, errno);
+}
+
+TEST_F(MountHtml5FsNodeSyncDirTest, GetStat) {
+  const int creation_time = 1000;
+  const int access_time = 2000;
+  const int modified_time = 3000;
+
+  PP_FileInfo info;
+  info.size = 0;
+  info.type = PP_FILETYPE_DIRECTORY;
+  info.system_type = PP_FILESYSTEMTYPE_LOCALPERSISTENT;
+  info.creation_time = creation_time;
+  info.last_access_time = access_time;
+  info.last_modified_time = modified_time;
+
+  EXPECT_CALL(*fileref_, Query(fileref_resource_, _, _))
+      .WillOnce(DoAll(SetArgPointee<1>(info),
+                      Return(int32_t(PP_OK))));
+
+  struct stat statbuf;
+  int result = node_->GetStat(&statbuf);
+
+  EXPECT_EQ(0, result);
+  EXPECT_EQ(S_IFDIR | S_IWRITE | S_IREAD, statbuf.st_mode);
+  EXPECT_EQ(0, statbuf.st_size);
+  EXPECT_EQ(access_time, statbuf.st_atime);
+  EXPECT_EQ(modified_time, statbuf.st_mtime);
+  EXPECT_EQ(creation_time, statbuf.st_ctime);
+}
+
+TEST_F(MountHtml5FsNodeSyncDirTest, FTruncate) {
+  const int size = 123;
+  // Should fail for directories.
+  int result = node_->FTruncate(size);
+  ASSERT_EQ(-1, result);
+  EXPECT_EQ(EISDIR, errno);
+}
+
+TEST_F(MountHtml5FsNodeSyncDirTest, GetDents) {
   const int fileref_resource_1 = 238;
   const int fileref_resource_2 = 239;
 
@@ -441,9 +540,17 @@ TEST_F(MountHtml5FsNodeSyncTest, GetDents) {
 
   struct dirent dirents[2];
   memset(&dirents[0], 0, sizeof(dirents));
-  int result = node_->GetDents(0, &dirents[0], sizeof(dirent) * 2);
-
-  EXPECT_EQ(0, result);
-  EXPECT_STREQ(&fileref_name_cstr_1[0], &dirents[0].d_name[0]);
-  EXPECT_STREQ(&fileref_name_cstr_2[0], &dirents[1].d_name[0]);
+  // +2 to test a size that is not a multiple of sizeof(dirent).
+  // Expect it to round down.
+  int result = node_->GetDents(0, &dirents[0], sizeof(dirent) * 2 + 2);
+
+  ASSERT_EQ(sizeof(dirent) * 2, result);
+  EXPECT_LT(0, dirents[0].d_ino);  // 0 is an invalid inode number.
+  EXPECT_EQ(sizeof(dirent), dirents[0].d_off);
+  EXPECT_EQ(sizeof(dirent), dirents[0].d_reclen);
+  EXPECT_STREQ(fileref_name_cstr_1, dirents[0].d_name);
+  EXPECT_LT(0, dirents[1].d_ino);  // 0 is an invalid inode number.
+  EXPECT_EQ(sizeof(dirent), dirents[1].d_off);
+  EXPECT_EQ(sizeof(dirent), dirents[1].d_reclen);
+  EXPECT_STREQ(fileref_name_cstr_2, dirents[1].d_name);
 }