FileSystem: Modify ObfucatedFileUtil to delete contents of the plugin private filesystem

Database destruction functions haven't correctly worked for plugin private
filesystems since the filesystem has a different directory structure
from other filesystems as follows:

- PluginPrivate FS: /path/to/File System/Plugins/<origins>/<plugin_id>/<paths>/<file>
- Other FS: /path/to/File System/<origins>/<storage type>/<paths>/<file>

This patch tweaks those functions correctly to delete contents and
metadata of the plugin private filesystem.

NOTE: A part of tests is contributed by xhwang@chromium.org:
https://codereview.chromium.org/580363003/

BUG=326429
TEST=content_unittests --gtest_filter=PluginPrivateFileSystemBackendTest.*
TEST=content_unittests --gtest_filter=ObfuscatedFileUtilTest.*

Review URL: https://codereview.chromium.org/579083004

Cr-Commit-Position: refs/heads/master@{#303213}

4 files changed, 295 insertions, 54 deletions

diff --git a/content/browser/fileapi/obfuscated_file_util_unittest.cc b/content/browser/fileapi/obfuscated_file_util_unittest.cc
index 2bfdf87..cef3990 100644
--- a/content/browser/fileapi/obfuscated_file_util_unittest.cc
+++ b/content/browser/fileapi/obfuscated_file_util_unittest.cc
@@ -709,8 +709,8 @@ class ObfuscatedFileUtilTest : public testing::Test {
     ASSERT_TRUE(db != NULL);
 
     // Destory it.
-    ASSERT_TRUE(file_util->DestroyDirectoryDatabase(
-        url.origin(), GetTypeString(url.type())));
+    file_util->DestroyDirectoryDatabase(
+        url.origin(), GetTypeString(url.type()));
     ASSERT_TRUE(file_util->directories_.empty());
   }
 
@@ -2449,4 +2449,123 @@ TEST_F(ObfuscatedFileUtilTest, CreateDirectory_NotADirectoryInRecursive) {
                                    true /* recursive */));
 }
 
+TEST_F(ObfuscatedFileUtilTest, DeleteDirectoryForOriginAndType) {
+  const GURL origin1("http://www.example.com:12");
+  const GURL origin2("http://www.example.com:1234");
+
+  // Create origin directories.
+  scoped_ptr<SandboxFileSystemTestHelper> fs1(
+      NewFileSystem(origin1, kFileSystemTypeTemporary));
+  scoped_ptr<SandboxFileSystemTestHelper> fs2(
+      NewFileSystem(origin1, kFileSystemTypePersistent));
+  scoped_ptr<SandboxFileSystemTestHelper> fs3(
+      NewFileSystem(origin2, kFileSystemTypeTemporary));
+  scoped_ptr<SandboxFileSystemTestHelper> fs4(
+      NewFileSystem(origin2, kFileSystemTypePersistent));
+
+  // Make sure directories for origin1 exist.
+  base::File::Error error = base::File::FILE_ERROR_FAILED;
+  ofu()->GetDirectoryForOriginAndType(
+      origin1, GetTypeString(kFileSystemTypeTemporary), false, &error);
+  ASSERT_EQ(base::File::FILE_OK, error);
+  error = base::File::FILE_ERROR_FAILED;
+  ofu()->GetDirectoryForOriginAndType(
+      origin1, GetTypeString(kFileSystemTypePersistent), false, &error);
+  ASSERT_EQ(base::File::FILE_OK, error);
+
+  // Make sure directories for origin2 exist.
+  error = base::File::FILE_ERROR_FAILED;
+  ofu()->GetDirectoryForOriginAndType(
+      origin2, GetTypeString(kFileSystemTypeTemporary), false, &error);
+  ASSERT_EQ(base::File::FILE_OK, error);
+  error = base::File::FILE_ERROR_FAILED;
+  ofu()->GetDirectoryForOriginAndType(
+      origin2, GetTypeString(kFileSystemTypePersistent), false, &error);
+  ASSERT_EQ(base::File::FILE_OK, error);
+
+  // Delete a directory for origin1's persistent filesystem.
+  ofu()->DeleteDirectoryForOriginAndType(
+      origin1, GetTypeString(kFileSystemTypePersistent));
+
+  // The directory for origin1's temporary filesystem should not be removed.
+  error = base::File::FILE_ERROR_FAILED;
+  ofu()->GetDirectoryForOriginAndType(
+      origin1, GetTypeString(kFileSystemTypeTemporary), false, &error);
+  ASSERT_EQ(base::File::FILE_OK, error);
+
+  // The directory for origin1's persistent filesystem should be removed.
+  error = base::File::FILE_ERROR_FAILED;
+  ofu()->GetDirectoryForOriginAndType(
+      origin1, GetTypeString(kFileSystemTypePersistent), false, &error);
+  ASSERT_EQ(base::File::FILE_ERROR_NOT_FOUND, error);
+
+  // The directories for origin2 should not be removed.
+  error = base::File::FILE_ERROR_FAILED;
+  ofu()->GetDirectoryForOriginAndType(
+      origin2, GetTypeString(kFileSystemTypeTemporary), false, &error);
+  ASSERT_EQ(base::File::FILE_OK, error);
+  error = base::File::FILE_ERROR_FAILED;
+  ofu()->GetDirectoryForOriginAndType(
+      origin2, GetTypeString(kFileSystemTypePersistent), false, &error);
+  ASSERT_EQ(base::File::FILE_OK, error);
+}
+
+TEST_F(ObfuscatedFileUtilTest, DeleteDirectoryForOriginAndType_DeleteAll) {
+  const GURL origin1("http://www.example.com:12");
+  const GURL origin2("http://www.example.com:1234");
+
+  // Create origin directories.
+  scoped_ptr<SandboxFileSystemTestHelper> fs1(
+      NewFileSystem(origin1, kFileSystemTypeTemporary));
+  scoped_ptr<SandboxFileSystemTestHelper> fs2(
+      NewFileSystem(origin1, kFileSystemTypePersistent));
+  scoped_ptr<SandboxFileSystemTestHelper> fs3(
+      NewFileSystem(origin2, kFileSystemTypeTemporary));
+  scoped_ptr<SandboxFileSystemTestHelper> fs4(
+      NewFileSystem(origin2, kFileSystemTypePersistent));
+
+  // Make sure directories for origin1 exist.
+  base::File::Error error = base::File::FILE_ERROR_FAILED;
+  ofu()->GetDirectoryForOriginAndType(
+      origin1, GetTypeString(kFileSystemTypeTemporary), false, &error);
+  ASSERT_EQ(base::File::FILE_OK, error);
+  error = base::File::FILE_ERROR_FAILED;
+  ofu()->GetDirectoryForOriginAndType(
+      origin1, GetTypeString(kFileSystemTypePersistent), false, &error);
+  ASSERT_EQ(base::File::FILE_OK, error);
+
+  // Make sure directories for origin2 exist.
+  error = base::File::FILE_ERROR_FAILED;
+  ofu()->GetDirectoryForOriginAndType(
+      origin2, GetTypeString(kFileSystemTypeTemporary), false, &error);
+  ASSERT_EQ(base::File::FILE_OK, error);
+  error = base::File::FILE_ERROR_FAILED;
+  ofu()->GetDirectoryForOriginAndType(
+      origin2, GetTypeString(kFileSystemTypePersistent), false, &error);
+  ASSERT_EQ(base::File::FILE_OK, error);
+
+  // Delete all directories for origin1.
+  ofu()->DeleteDirectoryForOriginAndType(origin1, std::string());
+
+  // The directories for origin1 should be removed.
+  error = base::File::FILE_ERROR_FAILED;
+  ofu()->GetDirectoryForOriginAndType(
+      origin1, GetTypeString(kFileSystemTypeTemporary), false, &error);
+  ASSERT_EQ(base::File::FILE_ERROR_NOT_FOUND, error);
+  error = base::File::FILE_ERROR_FAILED;
+  ofu()->GetDirectoryForOriginAndType(
+      origin1, GetTypeString(kFileSystemTypePersistent), false, &error);
+  ASSERT_EQ(base::File::FILE_ERROR_NOT_FOUND, error);
+
+  // The directories for origin2 should not be removed.
+  error = base::File::FILE_ERROR_FAILED;
+  ofu()->GetDirectoryForOriginAndType(
+      origin2, GetTypeString(kFileSystemTypeTemporary), false, &error);
+  ASSERT_EQ(base::File::FILE_OK, error);
+  error = base::File::FILE_ERROR_FAILED;
+  ofu()->GetDirectoryForOriginAndType(
+      origin2, GetTypeString(kFileSystemTypePersistent), false, &error);
+  ASSERT_EQ(base::File::FILE_OK, error);
+}
+
 }  // namespace content
diff --git a/content/browser/fileapi/plugin_private_file_system_backend_unittest.cc b/content/browser/fileapi/plugin_private_file_system_backend_unittest.cc
index 61de643..ef88fed 100644
--- a/content/browser/fileapi/plugin_private_file_system_backend_unittest.cc
+++ b/content/browser/fileapi/plugin_private_file_system_backend_unittest.cc
@@ -28,7 +28,8 @@ namespace content {
 
 namespace {
 
-const GURL kOrigin("http://www.example.com");
+const GURL kOrigin1("http://www.example.com");
+const GURL kOrigin2("https://www.example.com");
 const std::string kPlugin1("plugin1");
 const std::string kPlugin2("plugin2");
 const storage::FileSystemType kType = storage::kFileSystemTypePluginPrivate;
@@ -72,14 +73,16 @@ class PluginPrivateFileSystemBackendTest : public testing::Test {
   base::ScopedTempDir data_dir_;
   base::MessageLoop message_loop_;
   scoped_refptr<FileSystemContext> context_;
-  std::string filesystem_id_;
 };
 
+// TODO(kinuko,nhiroki): There are a lot of duplicate code in these tests. Write
+// helper functions to simplify the tests.
+
 TEST_F(PluginPrivateFileSystemBackendTest, OpenFileSystemBasic) {
   const std::string filesystem_id1 = RegisterFileSystem();
   base::File::Error error = base::File::FILE_ERROR_FAILED;
   backend()->OpenPrivateFileSystem(
-      kOrigin,
+      kOrigin1,
       kType,
       filesystem_id1,
       kPlugin1,
@@ -92,7 +95,7 @@ TEST_F(PluginPrivateFileSystemBackendTest, OpenFileSystemBasic) {
   const std::string filesystem_id2 = RegisterFileSystem();
   error = base::File::FILE_ERROR_FAILED;
   backend()->OpenPrivateFileSystem(
-      kOrigin,
+      kOrigin1,
       kType,
       filesystem_id2,
       kPlugin1,
@@ -102,7 +105,7 @@ TEST_F(PluginPrivateFileSystemBackendTest, OpenFileSystemBasic) {
   ASSERT_EQ(base::File::FILE_OK, error);
 
   const GURL root_url(storage::GetIsolatedFileSystemRootURIString(
-      kOrigin, filesystem_id1, kRootName));
+      kOrigin1, filesystem_id1, kRootName));
   FileSystemURL file = CreateURL(root_url, "foo");
   base::FilePath platform_path;
   EXPECT_EQ(base::File::FILE_OK,
@@ -119,7 +122,7 @@ TEST_F(PluginPrivateFileSystemBackendTest, PluginIsolation) {
   const std::string filesystem_id1 = RegisterFileSystem();
   base::File::Error error = base::File::FILE_ERROR_FAILED;
   backend()->OpenPrivateFileSystem(
-      kOrigin,
+      kOrigin1,
       kType,
       filesystem_id1,
       kPlugin1,
@@ -131,7 +134,7 @@ TEST_F(PluginPrivateFileSystemBackendTest, PluginIsolation) {
   const std::string filesystem_id2 = RegisterFileSystem();
   error = base::File::FILE_ERROR_FAILED;
   backend()->OpenPrivateFileSystem(
-      kOrigin,
+      kOrigin1,
       kType,
       filesystem_id2,
       kPlugin2,
@@ -142,9 +145,8 @@ TEST_F(PluginPrivateFileSystemBackendTest, PluginIsolation) {
 
   // Create 'foo' in kPlugin1.
   const GURL root_url1(storage::GetIsolatedFileSystemRootURIString(
-      kOrigin, filesystem_id1, kRootName));
+      kOrigin1, filesystem_id1, kRootName));
   FileSystemURL file1 = CreateURL(root_url1, "foo");
-  base::FilePath platform_path;
   EXPECT_EQ(base::File::FILE_OK,
             AsyncFileTestHelper::CreateFile(context_.get(), file1));
   EXPECT_TRUE(AsyncFileTestHelper::FileExists(
@@ -152,13 +154,137 @@ TEST_F(PluginPrivateFileSystemBackendTest, PluginIsolation) {
 
   // See the same path is not available in kPlugin2.
   const GURL root_url2(storage::GetIsolatedFileSystemRootURIString(
-      kOrigin, filesystem_id2, kRootName));
+      kOrigin1, filesystem_id2, kRootName));
   FileSystemURL file2 = CreateURL(root_url2, "foo");
   EXPECT_FALSE(AsyncFileTestHelper::FileExists(
       context_.get(), file2, AsyncFileTestHelper::kDontCheckSize));
 }
 
-// TODO(kinuko,nhiroki): also test if DeleteOriginDataOnFileThread
-// works fine when there's multiple plugin partitions.
+TEST_F(PluginPrivateFileSystemBackendTest, OriginIsolation) {
+  // Open filesystem for kOrigin1 and kOrigin2.
+  const std::string filesystem_id1 = RegisterFileSystem();
+  base::File::Error error = base::File::FILE_ERROR_FAILED;
+  backend()->OpenPrivateFileSystem(
+      kOrigin1,
+      kType,
+      filesystem_id1,
+      kPlugin1,
+      storage::OPEN_FILE_SYSTEM_CREATE_IF_NONEXISTENT,
+      base::Bind(&DidOpenFileSystem, &error));
+  base::RunLoop().RunUntilIdle();
+  ASSERT_EQ(base::File::FILE_OK, error);
+
+  const std::string filesystem_id2 = RegisterFileSystem();
+  error = base::File::FILE_ERROR_FAILED;
+  backend()->OpenPrivateFileSystem(
+      kOrigin2,
+      kType,
+      filesystem_id2,
+      kPlugin1,
+      storage::OPEN_FILE_SYSTEM_CREATE_IF_NONEXISTENT,
+      base::Bind(&DidOpenFileSystem, &error));
+  base::RunLoop().RunUntilIdle();
+  ASSERT_EQ(base::File::FILE_OK, error);
+
+  // Create 'foo' in kOrigin1.
+  const GURL root_url1(storage::GetIsolatedFileSystemRootURIString(
+      kOrigin1, filesystem_id1, kRootName));
+  FileSystemURL file1 = CreateURL(root_url1, "foo");
+  EXPECT_EQ(base::File::FILE_OK,
+            AsyncFileTestHelper::CreateFile(context_.get(), file1));
+  EXPECT_TRUE(AsyncFileTestHelper::FileExists(
+      context_.get(), file1, AsyncFileTestHelper::kDontCheckSize));
+
+  // See the same path is not available in kOrigin2.
+  const GURL root_url2(storage::GetIsolatedFileSystemRootURIString(
+      kOrigin2, filesystem_id2, kRootName));
+  FileSystemURL file2 = CreateURL(root_url2, "foo");
+  EXPECT_FALSE(AsyncFileTestHelper::FileExists(
+      context_.get(), file2, AsyncFileTestHelper::kDontCheckSize));
+}
+
+TEST_F(PluginPrivateFileSystemBackendTest, DeleteOriginDirectory) {
+  // Open filesystem for kOrigin1 and kOrigin2.
+  const std::string filesystem_id1 = RegisterFileSystem();
+  base::File::Error error = base::File::FILE_ERROR_FAILED;
+  backend()->OpenPrivateFileSystem(
+      kOrigin1,
+      kType,
+      filesystem_id1,
+      kPlugin1,
+      storage::OPEN_FILE_SYSTEM_CREATE_IF_NONEXISTENT,
+      base::Bind(&DidOpenFileSystem, &error));
+  base::RunLoop().RunUntilIdle();
+  ASSERT_EQ(base::File::FILE_OK, error);
+
+  const std::string filesystem_id2 = RegisterFileSystem();
+  error = base::File::FILE_ERROR_FAILED;
+  backend()->OpenPrivateFileSystem(
+      kOrigin2,
+      kType,
+      filesystem_id2,
+      kPlugin1,
+      storage::OPEN_FILE_SYSTEM_CREATE_IF_NONEXISTENT,
+      base::Bind(&DidOpenFileSystem, &error));
+  base::RunLoop().RunUntilIdle();
+  ASSERT_EQ(base::File::FILE_OK, error);
+
+  // Create 'foo' in kOrigin1.
+  const GURL root_url1(storage::GetIsolatedFileSystemRootURIString(
+      kOrigin1, filesystem_id1, kRootName));
+  FileSystemURL file1 = CreateURL(root_url1, "foo");
+  EXPECT_EQ(base::File::FILE_OK,
+            AsyncFileTestHelper::CreateFile(context_.get(), file1));
+  EXPECT_TRUE(AsyncFileTestHelper::FileExists(
+      context_.get(), file1, AsyncFileTestHelper::kDontCheckSize));
+
+  // Create 'foo' in kOrigin2.
+  const GURL root_url2(storage::GetIsolatedFileSystemRootURIString(
+      kOrigin2, filesystem_id2, kRootName));
+  FileSystemURL file2 = CreateURL(root_url2, "foo");
+  EXPECT_EQ(base::File::FILE_OK,
+            AsyncFileTestHelper::CreateFile(context_.get(), file2));
+  EXPECT_TRUE(AsyncFileTestHelper::FileExists(
+      context_.get(), file2, AsyncFileTestHelper::kDontCheckSize));
+
+  // Delete data for kOrigin1.
+  error = backend()->DeleteOriginDataOnFileTaskRunner(
+      context_.get(), NULL, kOrigin1, kType);
+  EXPECT_EQ(base::File::FILE_OK, error);
+
+  // Confirm 'foo' in kOrigin1 is deleted.
+  EXPECT_FALSE(AsyncFileTestHelper::FileExists(
+      context_.get(), file1, AsyncFileTestHelper::kDontCheckSize));
+
+  // Confirm 'foo' in kOrigin2 is NOT deleted.
+  EXPECT_TRUE(AsyncFileTestHelper::FileExists(
+      context_.get(), file2, AsyncFileTestHelper::kDontCheckSize));
+
+  // Re-open filesystem for kOrigin1.
+  const std::string filesystem_id3 = RegisterFileSystem();
+  error = base::File::FILE_ERROR_FAILED;
+  backend()->OpenPrivateFileSystem(
+      kOrigin1,
+      kType,
+      filesystem_id3,
+      kPlugin1,
+      storage::OPEN_FILE_SYSTEM_CREATE_IF_NONEXISTENT,
+      base::Bind(&DidOpenFileSystem, &error));
+  base::RunLoop().RunUntilIdle();
+  ASSERT_EQ(base::File::FILE_OK, error);
+
+  // Re-create 'foo' in kOrigin1.
+  const GURL root_url3(storage::GetIsolatedFileSystemRootURIString(
+      kOrigin1, filesystem_id3, kRootName));
+  FileSystemURL file3 = CreateURL(root_url3, "foo");
+  EXPECT_EQ(base::File::FILE_OK,
+            AsyncFileTestHelper::CreateFile(context_.get(), file3));
+  EXPECT_TRUE(AsyncFileTestHelper::FileExists(
+      context_.get(), file3, AsyncFileTestHelper::kDontCheckSize));
+
+  // Confirm 'foo' in kOrigin1 is re-created.
+  EXPECT_TRUE(AsyncFileTestHelper::FileExists(
+      context_.get(), file3, AsyncFileTestHelper::kDontCheckSize));
+}
 
 }  // namespace content
diff --git a/storage/browser/fileapi/obfuscated_file_util.cc b/storage/browser/fileapi/obfuscated_file_util.cc
index fb671e7..8f696c0 100644
--- a/storage/browser/fileapi/obfuscated_file_util.cc
+++ b/storage/browser/fileapi/obfuscated_file_util.cc
@@ -15,6 +15,7 @@
 #include "base/metrics/histogram.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/strings/utf_string_conversions.h"
@@ -67,6 +68,8 @@ void InitFileInfo(
 const int64 kPathCreationQuotaCost = 146;  // Bytes per inode, basically.
 const int64 kPathByteQuotaCost = 2;  // Bytes per byte of path length in UTF-8.
 
+const char kDirectoryDatabaseKeySeparator = ' ';
+
 int64 UsageForPath(size_t length) {
   return kPathCreationQuotaCost +
       static_cast<int64>(length) * kPathByteQuotaCost;
@@ -855,28 +858,22 @@ base::FilePath ObfuscatedFileUtil::GetDirectoryForOriginAndType(
 bool ObfuscatedFileUtil::DeleteDirectoryForOriginAndType(
     const GURL& origin,
     const std::string& type_string) {
-  base::File::Error error = base::File::FILE_OK;
-  base::FilePath origin_type_path = GetDirectoryForOriginAndType(
-      origin, type_string, false, &error);
-  if (origin_type_path.empty())
-    return true;
-  if (error != base::File::FILE_ERROR_NOT_FOUND) {
-    // TODO(dmikurube): Consider the return value of DestroyDirectoryDatabase.
-    // We ignore its error now since 1) it doesn't matter the final result, and
-    // 2) it always returns false in Windows because of LevelDB's
-    // implementation.
-    // Information about failure would be useful for debugging.
-    if (!type_string.empty())
-      DestroyDirectoryDatabase(origin, type_string);
-    if (!base::DeleteFile(origin_type_path, true /* recursive */))
-      return false;
-  }
-
-  base::FilePath origin_path = VirtualPath::DirName(origin_type_path);
-  DCHECK_EQ(origin_path.value(),
-            GetDirectoryForOrigin(origin, false, NULL).value());
+  DestroyDirectoryDatabase(origin, type_string);
 
+  const base::FilePath origin_path = GetDirectoryForOrigin(origin, false, NULL);
   if (!type_string.empty()) {
+    // Delete the filesystem type directory.
+    base::File::Error error = base::File::FILE_OK;
+    const base::FilePath origin_type_path =
+        GetDirectoryForOriginAndType(origin, type_string, false, &error);
+    if (error == base::File::FILE_ERROR_FAILED)
+      return false;
+    if (error == base::File::FILE_OK &&
+        !origin_type_path.empty() &&
+        !base::DeleteFile(origin_type_path, true /* recursive */)) {
+      return false;
+    }
+
     // At this point we are sure we had successfully deleted the origin/type
     // directory (i.e. we're ready to just return true).
     // See if we have other directories in this origin directory.
@@ -898,10 +895,7 @@ bool ObfuscatedFileUtil::DeleteDirectoryForOriginAndType(
     origin_database_->RemovePathForOrigin(
         storage::GetIdentifierFromOrigin(origin));
   }
-  if (!base::DeleteFile(origin_path, true /* recursive */))
-    return false;
-
-  return true;
+  return base::DeleteFile(origin_path, true /* recursive */);
 }
 
 ObfuscatedFileUtil::AbstractOriginEnumerator*
@@ -913,18 +907,23 @@ ObfuscatedFileUtil::CreateOriginEnumerator() {
       origin_database_.get(), file_system_directory_);
 }
 
-bool ObfuscatedFileUtil::DestroyDirectoryDatabase(
+void ObfuscatedFileUtil::DestroyDirectoryDatabase(
     const GURL& origin,
     const std::string& type_string) {
-  std::string key = GetDirectoryDatabaseKey(origin, type_string);
-  if (key.empty())
-    return true;
-  DirectoryMap::iterator iter = directories_.find(key);
-  if (iter == directories_.end())
-    return true;
-  scoped_ptr<SandboxDirectoryDatabase> database(iter->second);
-  directories_.erase(iter);
-  return database->DestroyDatabase();
+  // If |type_string| is empty, delete all filesystem types under |origin|.
+  const std::string key_prefix = GetDirectoryDatabaseKey(origin, type_string);
+  for (DirectoryMap::iterator iter = directories_.lower_bound(key_prefix);
+       iter != directories_.end();) {
+    if (!StartsWithASCII(iter->first, key_prefix, true))
+      break;
+    DCHECK(type_string.empty() || iter->first == key_prefix);
+    scoped_ptr<SandboxDirectoryDatabase> database(iter->second);
+    directories_.erase(iter++);
+
+    // Continue to destroy databases even if it failed because it doesn't affect
+    // the final result.
+    database->DestroyDatabase();
+  }
 }
 
 // static
@@ -1136,12 +1135,9 @@ base::FilePath ObfuscatedFileUtil::DataPathToLocalPath(
 
 std::string ObfuscatedFileUtil::GetDirectoryDatabaseKey(
     const GURL& origin, const std::string& type_string) {
-  if (type_string.empty()) {
-    LOG(WARNING) << "Unknown filesystem type requested:" << type_string;
-    return std::string();
-  }
   // For isolated origin we just use a type string as a key.
-  return storage::GetIdentifierFromOrigin(origin) + type_string;
+  return storage::GetIdentifierFromOrigin(origin) +
+      kDirectoryDatabaseKeySeparator + type_string;
 }
 
 // TODO(ericu): How to do the whole validation-without-creation thing?
diff --git a/storage/browser/fileapi/obfuscated_file_util.h b/storage/browser/fileapi/obfuscated_file_util.h
index d22d2e2..090e55e 100644
--- a/storage/browser/fileapi/obfuscated_file_util.h
+++ b/storage/browser/fileapi/obfuscated_file_util.h
@@ -197,7 +197,7 @@ class STORAGE_EXPORT_PRIVATE ObfuscatedFileUtil
 
   // Deletes a directory database from the database list in the ObfuscatedFSFU
   // and destroys the database on the disk.
-  bool DestroyDirectoryDatabase(const GURL& origin,
+  void DestroyDirectoryDatabase(const GURL& origin,
                                 const std::string& type_string);
 
   // Computes a cost for storing a given file in the obfuscated FSFU.