Add RevokeFileSystem back to IsolatedContext

We deprecated it when we added RevokeFileSystemByPath but it looks we still want it.

BUG=none
TEST=IsolatedContextTest.RegisterAndRevokeTest


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

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

3 files changed, 57 insertions, 13 deletions

diff --git a/webkit/fileapi/isolated_context.cc b/webkit/fileapi/isolated_context.cc
index 957680d..6398e42 100644
--- a/webkit/fileapi/isolated_context.cc
+++ b/webkit/fileapi/isolated_context.cc
@@ -166,8 +166,9 @@ std::string IsolatedContext::RegisterDraggedFileSystem(
 
 std::string IsolatedContext::RegisterFileSystemForPath(
     FileSystemType type,
-    const FilePath& path,
+    const FilePath& path_in,
     std::string* register_name) {
+  FilePath path(path_in.NormalizePathSeparators());
   DCHECK(!path.ReferencesParent() && path.IsAbsolute());
   std::string name;
   if (register_name && !register_name->empty()) {
@@ -185,8 +186,14 @@ std::string IsolatedContext::RegisterFileSystemForPath(
   return filesystem_id;
 }
 
-void IsolatedContext::RevokeFileSystemByPath(const FilePath& path) {
+bool IsolatedContext::RevokeFileSystem(const std::string& filesystem_id) {
   base::AutoLock locker(lock_);
+  return UnregisterFileSystem(filesystem_id);
+}
+
+void IsolatedContext::RevokeFileSystemByPath(const FilePath& path_in) {
+  base::AutoLock locker(lock_);
+  FilePath path(path_in.NormalizePathSeparators());
   PathToID::iterator ids_iter = path_to_id_map_.find(path);
   if (ids_iter == path_to_id_map_.end())
     return;
@@ -219,16 +226,8 @@ void IsolatedContext::RemoveReference(const std::string& filesystem_id) {
   DCHECK(instance->ref_counts() > 0);
   instance->RemoveRef();
   if (instance->ref_counts() == 0) {
-    if (instance->IsSinglePathInstance()) {
-      PathToID::iterator ids_iter = path_to_id_map_.find(
-          instance->file_info().path);
-      DCHECK(ids_iter != path_to_id_map_.end());
-      ids_iter->second.erase(filesystem_id);
-      if (ids_iter->second.empty())
-        path_to_id_map_.erase(ids_iter);
-    }
-    delete instance;
-    instance_map_.erase(found);
+    bool deleted = UnregisterFileSystem(filesystem_id);
+    DCHECK(deleted);
   }
 }
 
@@ -268,6 +267,7 @@ bool IsolatedContext::CrackIsolatedPath(const FilePath& virtual_path,
   std::string name = FilePath(components[1]).AsUTF8Unsafe();
   if (!found_instance->second->ResolvePathForName(name, &cracked_path))
     return false;
+
   for (size_t i = 2; i < components.size(); ++i)
     cracked_path = cracked_path.Append(components[i]);
   *path = cracked_path;
@@ -311,8 +311,28 @@ IsolatedContext::~IsolatedContext() {
                                        instance_map_.end());
 }
 
+bool IsolatedContext::UnregisterFileSystem(const std::string& filesystem_id) {
+  lock_.AssertAcquired();
+  IDToInstance::iterator found = instance_map_.find(filesystem_id);
+  if (found == instance_map_.end())
+    return false;
+  Instance* instance = found->second;
+  if (instance->IsSinglePathInstance()) {
+    PathToID::iterator ids_iter = path_to_id_map_.find(
+        instance->file_info().path);
+    DCHECK(ids_iter != path_to_id_map_.end());
+    ids_iter->second.erase(filesystem_id);
+    if (ids_iter->second.empty())
+      path_to_id_map_.erase(ids_iter);
+  }
+  delete found->second;
+  instance_map_.erase(found);
+  return true;
+}
+
 std::string IsolatedContext::GetNewFileSystemId() const {
   // Returns an arbitrary random string which must be unique in the map.
+  lock_.AssertAcquired();
   uint32 random_data[4];
   std::string id;
   do {
diff --git a/webkit/fileapi/isolated_context.h b/webkit/fileapi/isolated_context.h
index 5593c62..3888dfe 100644
--- a/webkit/fileapi/isolated_context.h
+++ b/webkit/fileapi/isolated_context.h
@@ -101,6 +101,10 @@ class FILEAPI_EXPORT IsolatedContext {
                                         const FilePath& path,
                                         std::string* register_name);
 
+  // Revokes the filesystem |filesystem_id|
+  // Returns false if the |filesystem_id| is not (no longer) registered.
+  bool RevokeFileSystem(const std::string& filesystem_id);
+
   // Revokes all filesystem(s) registered for the given path.
   // This is assumed to be called when the registered path becomes
   // globally invalid, e.g. when a device for the path is detached.
@@ -209,6 +213,10 @@ class FILEAPI_EXPORT IsolatedContext {
   IsolatedContext();
   ~IsolatedContext();
 
+  // Unregisters a file system of given |filesystem_id|. Must be called with
+  // lock_ held.  Returns true if the file system is unregistered.
+  bool UnregisterFileSystem(const std::string& filesystem_id);
+
   // Returns a new filesystem_id.  Called with lock.
   std::string GetNewFileSystemId() const;
 
diff --git a/webkit/fileapi/isolated_context_unittest.cc b/webkit/fileapi/isolated_context_unittest.cc
index ac71aec..a2e637d 100644
--- a/webkit/fileapi/isolated_context_unittest.cc
+++ b/webkit/fileapi/isolated_context_unittest.cc
@@ -124,11 +124,13 @@ TEST_F(IsolatedContextTest, RegisterAndRevokeTest) {
   ASSERT_FALSE(isolated_context()->GetRegisteredPath(id_, &path));
   ASSERT_TRUE(isolated_context()->GetRegisteredPath(id2, &path));
 
-  // Try registering two more file systems for the same path as id2.
+  // Try registering three more file systems for the same path as id2.
   std::string id3 = isolated_context()->RegisterFileSystemForPath(
       kFileSystemTypeIsolated, path, NULL);
   std::string id4 = isolated_context()->RegisterFileSystemForPath(
       kFileSystemTypeIsolated, path, NULL);
+  std::string id5 = isolated_context()->RegisterFileSystemForPath(
+      kFileSystemTypeIsolated, path, NULL);
 
   // Remove file system for id4.
   isolated_context()->AddReference(id4);
@@ -138,6 +140,19 @@ TEST_F(IsolatedContextTest, RegisterAndRevokeTest) {
   ASSERT_TRUE(isolated_context()->GetRegisteredPath(id2, &path));
   ASSERT_TRUE(isolated_context()->GetRegisteredPath(id3, &path));
   ASSERT_FALSE(isolated_context()->GetRegisteredPath(id4, &path));
+  ASSERT_TRUE(isolated_context()->GetRegisteredPath(id5, &path));
+
+  // Revoke file system id5, after adding multiple references.
+  isolated_context()->AddReference(id5);
+  isolated_context()->AddReference(id5);
+  isolated_context()->AddReference(id5);
+  isolated_context()->RevokeFileSystem(id5);
+
+  // No matter how many references we add id5 must be invalid now.
+  ASSERT_TRUE(isolated_context()->GetRegisteredPath(id2, &path));
+  ASSERT_TRUE(isolated_context()->GetRegisteredPath(id3, &path));
+  ASSERT_FALSE(isolated_context()->GetRegisteredPath(id4, &path));
+  ASSERT_FALSE(isolated_context()->GetRegisteredPath(id5, &path));
 
   // Revoke the file systems by path.
   isolated_context()->RevokeFileSystemByPath(path);
@@ -146,6 +161,7 @@ TEST_F(IsolatedContextTest, RegisterAndRevokeTest) {
   ASSERT_FALSE(isolated_context()->GetRegisteredPath(id2, &path));
   ASSERT_FALSE(isolated_context()->GetRegisteredPath(id3, &path));
   ASSERT_FALSE(isolated_context()->GetRegisteredPath(id4, &path));
+  ASSERT_FALSE(isolated_context()->GetRegisteredPath(id5, &path));
 }
 
 TEST_F(IsolatedContextTest, CrackWithRelativePaths) {