Remove stl_util's STLDeleteContainerPairSecondPointers from storage/.

BUG=555865

Review-Url: https://codereview.chromium.org/2347253002
Cr-Commit-Position: refs/heads/master@{#419280}

storage/browser/fileapi/external_mount_points.cc

@@ -7,7 +7,7 @@
 #include "base/files/file_path.h"
 #include "base/lazy_instance.h"
 #include "base/macros.h"
-#include "base/stl_util.h"
+#include "base/memory/ptr_util.h"
 #include "storage/browser/fileapi/file_system_url.h"
 
 namespace {
@@ -103,7 +103,8 @@ bool ExternalMountPoints::RegisterFileSystem(
   if (!ValidateNewMountPoint(mount_name, type, path))
     return false;
 
-  instance_map_[mount_name] = new Instance(type, path, mount_option);
+  instance_map_[mount_name] =
+      base::MakeUnique<Instance>(type, path, mount_option);
   if (!path.empty() && IsOverlappingMountPathForbidden(type))
     path_to_name_map_.insert(std::make_pair(path, mount_name));
   return true;
@@ -117,13 +118,12 @@ bool ExternalMountPoints::HandlesFileSystemMountType(
 
 bool ExternalMountPoints::RevokeFileSystem(const std::string& mount_name) {
   base::AutoLock locker(lock_);
-  NameToInstance::iterator found = instance_map_.find(mount_name);
+  auto found = instance_map_.find(mount_name);
   if (found == instance_map_.end())
     return false;
-  Instance* instance = found->second;
+  Instance* instance = found->second.get();
   if (IsOverlappingMountPathForbidden(instance->type()))
     path_to_name_map_.erase(NormalizeFilePath(instance->path()));
-  delete found->second;
   instance_map_.erase(found);
   return true;
 }
@@ -132,7 +132,7 @@ bool ExternalMountPoints::GetRegisteredPath(
     const std::string& filesystem_id, base::FilePath* path) const {
   DCHECK(path);
   base::AutoLock locker(lock_);
-  NameToInstance::const_iterator found = instance_map_.find(filesystem_id);
+  auto found = instance_map_.find(filesystem_id);
   if (found == instance_map_.end())
     return false;
   *path = found->second->path();
@@ -167,13 +167,12 @@ bool ExternalMountPoints::CrackVirtualPath(
   base::FilePath cracked_path;
   {
     base::AutoLock locker(lock_);
-    NameToInstance::const_iterator found_instance =
-        instance_map_.find(maybe_mount_name);
+    auto found_instance = instance_map_.find(maybe_mount_name);
     if (found_instance == instance_map_.end())
       return false;
 
     *mount_name = maybe_mount_name;
-    const Instance* instance = found_instance->second;
+    const Instance* instance = found_instance->second.get();
     if (type)
       *type = instance->type();
     cracked_path = instance->path();
@@ -204,9 +203,8 @@ void ExternalMountPoints::AddMountPointInfosTo(
     std::vector<MountPointInfo>* mount_points) const {
   base::AutoLock locker(lock_);
   DCHECK(mount_points);
-  for (NameToInstance::const_iterator iter = instance_map_.begin();
-       iter != instance_map_.end(); ++iter) {
-    mount_points->push_back(MountPointInfo(iter->first, iter->second->path()));
+  for (const auto& pair : instance_map_) {
+    mount_points->push_back(MountPointInfo(pair.first, pair.second->path()));
   }
 }
 
@@ -249,20 +247,16 @@ void ExternalMountPoints::RevokeAllFileSystems() {
   NameToInstance instance_map_copy;
   {
     base::AutoLock locker(lock_);
-    instance_map_copy = instance_map_;
-    instance_map_.clear();
+    // This swap moves the contents of instance_map_ to the local variable so
+    // they can be freed outside the lock.
+    instance_map_copy.swap(instance_map_);
     path_to_name_map_.clear();
   }
-  base::STLDeleteContainerPairSecondPointers(instance_map_copy.begin(),
-                                             instance_map_copy.end());
 }
 
-ExternalMountPoints::ExternalMountPoints() {}
+ExternalMountPoints::ExternalMountPoints() = default;
 
-ExternalMountPoints::~ExternalMountPoints() {
-  base::STLDeleteContainerPairSecondPointers(instance_map_.begin(),
-                                             instance_map_.end());
-}
+ExternalMountPoints::~ExternalMountPoints() = default;
 
 FileSystemURL ExternalMountPoints::CrackFileSystemURL(
     const FileSystemURL& url) const {
@@ -312,7 +306,7 @@ bool ExternalMountPoints::ValidateNewMountPoint(const std::string& mount_name,
     return false;
 
   // Verify there is no registered mount point with the same name.
-  NameToInstance::iterator found = instance_map_.find(mount_name);
+  auto found = instance_map_.find(mount_name);
   if (found != instance_map_.end())
     return false;
 

storage/browser/fileapi/external_mount_points.h

@@ -6,6 +6,7 @@
 #define STORAGE_BROWSER_FILEAPI_EXTERNAL_MOUNT_POINTS_H_
 
 #include <map>
+#include <memory>
 #include <string>
 #include <vector>
 
@@ -119,7 +120,7 @@ class STORAGE_EXPORT ExternalMountPoints
   // Represents each file system instance (defined in the .cc).
   class Instance;
 
-  typedef std::map<std::string, Instance*> NameToInstance;
+  typedef std::map<std::string, std::unique_ptr<Instance>> NameToInstance;
 
   // Reverse map from registered path to its corresponding mount name.
   typedef std::map<base::FilePath, std::string> PathToName;

storage/browser/fileapi/isolated_context.cc

@@ -9,8 +9,8 @@
 
 #include "base/logging.h"
 #include "base/macros.h"
+#include "base/memory/ptr_util.h"
 #include "base/rand_util.h"
-#include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
@@ -219,8 +219,8 @@ std::string IsolatedContext::RegisterDraggedFileSystem(
     const FileInfoSet& files) {
   base::AutoLock locker(lock_);
   std::string filesystem_id = GetNewFileSystemId();
-  instance_map_[filesystem_id] = new Instance(
-      kFileSystemTypeDragged, files.fileset());
+  instance_map_[filesystem_id] =
+      base::MakeUnique<Instance>(kFileSystemTypeDragged, files.fileset());
   return filesystem_id;
 }
 
@@ -243,9 +243,8 @@ std::string IsolatedContext::RegisterFileSystemForPath(
 
   base::AutoLock locker(lock_);
   std::string new_id = GetNewFileSystemId();
-  instance_map_[new_id] = new Instance(type, filesystem_id,
-                                       MountPointInfo(name, path),
-                                       Instance::PLATFORM_PATH);
+  instance_map_[new_id] = base::MakeUnique<Instance>(
+      type, filesystem_id, MountPointInfo(name, path), Instance::PLATFORM_PATH);
   path_to_id_map_[path].insert(new_id);
   return new_id;
 }
@@ -259,7 +258,7 @@ std::string IsolatedContext::RegisterFileSystemForVirtualPath(
   if (path.ReferencesParent())
     return std::string();
   std::string filesystem_id = GetNewFileSystemId();
-  instance_map_[filesystem_id] = new Instance(
+  instance_map_[filesystem_id] = base::MakeUnique<Instance>(
       type,
       std::string(),  // filesystem_id
       MountPointInfo(register_name, cracked_path_prefix),
@@ -281,7 +280,7 @@ bool IsolatedContext::GetRegisteredPath(
     const std::string& filesystem_id, base::FilePath* path) const {
   DCHECK(path);
   base::AutoLock locker(lock_);
-  IDToInstance::const_iterator found = instance_map_.find(filesystem_id);
+  auto found = instance_map_.find(filesystem_id);
   if (found == instance_map_.end() || !found->second->IsSinglePathInstance())
     return false;
   *path = found->second->file_info().path;
@@ -319,11 +318,11 @@ bool IsolatedContext::CrackVirtualPath(
   base::FilePath cracked_path;
   {
     base::AutoLock locker(lock_);
-    IDToInstance::const_iterator found_instance = instance_map_.find(fsid);
+    auto found_instance = instance_map_.find(fsid);
     if (found_instance == instance_map_.end())
       return false;
     *id_or_name = fsid;
-    const Instance* instance = found_instance->second;
+    const Instance* instance = found_instance->second.get();
     if (type)
       *type = instance->type();
     if (cracked_id)
@@ -364,18 +363,12 @@ FileSystemURL IsolatedContext::CreateCrackedFileSystemURL(
 void IsolatedContext::RevokeFileSystemByPath(const base::FilePath& path_in) {
   base::AutoLock locker(lock_);
   base::FilePath path(path_in.NormalizePathSeparators());
-  PathToID::iterator ids_iter = path_to_id_map_.find(path);
+  auto ids_iter = path_to_id_map_.find(path);
   if (ids_iter == path_to_id_map_.end())
     return;
   std::set<std::string>& ids = ids_iter->second;
-  for (std::set<std::string>::iterator iter = ids.begin();
-       iter != ids.end(); ++iter) {
-    IDToInstance::iterator found = instance_map_.find(*iter);
-    if (found != instance_map_.end()) {
-      delete found->second;
-      instance_map_.erase(found);
-    }
-  }
+  for (auto iter = ids.begin(); iter != ids.end(); ++iter)
+    instance_map_.erase(*iter);
   path_to_id_map_.erase(ids_iter);
 }
 
@@ -389,10 +382,10 @@ void IsolatedContext::RemoveReference(const std::string& filesystem_id) {
   base::AutoLock locker(lock_);
   // This could get called for non-existent filesystem if it has been
   // already deleted by RevokeFileSystemByPath.
-  IDToInstance::iterator found = instance_map_.find(filesystem_id);
+  auto found = instance_map_.find(filesystem_id);
   if (found == instance_map_.end())
     return;
-  Instance* instance = found->second;
+  Instance* instance = found->second.get();
   DCHECK_GT(instance->ref_counts(), 0);
   instance->RemoveRef();
   if (instance->ref_counts() == 0) {
@@ -406,7 +399,7 @@ bool IsolatedContext::GetDraggedFileInfo(
     std::vector<MountPointInfo>* files) const {
   DCHECK(files);
   base::AutoLock locker(lock_);
-  IDToInstance::const_iterator found = instance_map_.find(filesystem_id);
+  auto found = instance_map_.find(filesystem_id);
   if (found == instance_map_.end() ||
       found->second->type() != kFileSystemTypeDragged)
     return false;
@@ -420,13 +413,9 @@ base::FilePath IsolatedContext::CreateVirtualRootPath(
   return base::FilePath().AppendASCII(filesystem_id);
 }
 
-IsolatedContext::IsolatedContext() {
-}
+IsolatedContext::IsolatedContext() = default;
 
-IsolatedContext::~IsolatedContext() {
-  base::STLDeleteContainerPairSecondPointers(instance_map_.begin(),
-                                             instance_map_.end());
-}
+IsolatedContext::~IsolatedContext() = default;
 
 FileSystemURL IsolatedContext::CrackFileSystemURL(
     const FileSystemURL& url) const {
@@ -454,19 +443,17 @@ FileSystemURL IsolatedContext::CrackFileSystemURL(
 
 bool IsolatedContext::UnregisterFileSystem(const std::string& filesystem_id) {
   lock_.AssertAcquired();
-  IDToInstance::iterator found = instance_map_.find(filesystem_id);
+  auto found = instance_map_.find(filesystem_id);
   if (found == instance_map_.end())
     return false;
-  Instance* instance = found->second;
+  Instance* instance = found->second.get();
   if (instance->IsSinglePathInstance()) {
-    PathToID::iterator ids_iter = path_to_id_map_.find(
-        instance->file_info().path);
+    auto 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;
 }

storage/browser/fileapi/isolated_context.h

@@ -166,11 +166,6 @@ class STORAGE_EXPORT IsolatedContext : public MountPoints {
   // Represents each file system instance (defined in the .cc).
   class Instance;
 
-  typedef std::map<std::string, Instance*> IDToInstance;
-
-  // Reverse map from registered path to IDs.
-  typedef std::map<base::FilePath, std::set<std::string> > PathToID;
-
   // Obtain an instance of this class via GetInstance().
   IsolatedContext();
   ~IsolatedContext() override;
@@ -188,8 +183,10 @@ class STORAGE_EXPORT IsolatedContext : public MountPoints {
   // This lock needs to be obtained when accessing the instance_map_.
   mutable base::Lock lock_;
 
-  IDToInstance instance_map_;
-  PathToID path_to_id_map_;
+  std::map<std::string, std::unique_ptr<Instance>> instance_map_;
+
+  // Reverse map from registered path to IDs.
+  std::map<base::FilePath, std::set<std::string>> path_to_id_map_;
 
   DISALLOW_COPY_AND_ASSIGN(IsolatedContext);
 };

storage/browser/fileapi/obfuscated_file_util.cc

@@ -14,9 +14,9 @@
 #include "base/files/file_util.h"
 #include "base/format_macros.h"
 #include "base/logging.h"
+#include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
 #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"
@@ -903,13 +903,12 @@ void ObfuscatedFileUtil::CloseFileSystemForOriginAndType(
     const GURL& origin,
     const std::string& type_string) {
   const std::string key_prefix = GetDirectoryDatabaseKey(origin, type_string);
-  for (DirectoryMap::iterator iter = directories_.lower_bound(key_prefix);
+  for (auto iter = directories_.lower_bound(key_prefix);
        iter != directories_.end();) {
     if (!base::StartsWith(iter->first, key_prefix,
                           base::CompareCase::SENSITIVE))
       break;
     DCHECK(type_string.empty() || iter->first == key_prefix);
-    std::unique_ptr<SandboxDirectoryDatabase> database(iter->second);
     directories_.erase(iter++);
   }
 }
@@ -928,13 +927,14 @@ void ObfuscatedFileUtil::DestroyDirectoryDatabase(
     const std::string& type_string) {
   // 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);
+  for (auto iter = directories_.lower_bound(key_prefix);
        iter != directories_.end();) {
     if (!base::StartsWith(iter->first, key_prefix,
                           base::CompareCase::SENSITIVE))
       break;
     DCHECK(type_string.empty() || iter->first == key_prefix);
-    std::unique_ptr<SandboxDirectoryDatabase> database(iter->second);
+    std::unique_ptr<SandboxDirectoryDatabase> database =
+        std::move(iter->second);
     directories_.erase(iter++);
 
     // Continue to destroy databases even if it failed because it doesn't affect
@@ -969,10 +969,11 @@ void ObfuscatedFileUtil::MaybePrepopulateDatabase(
         origin, type_string, false, &error);
     if (error != base::File::FILE_OK)
       continue;
-    std::unique_ptr<SandboxDirectoryDatabase> db(
-        new SandboxDirectoryDatabase(path, env_override_));
+    std::unique_ptr<SandboxDirectoryDatabase> db =
+        base::MakeUnique<SandboxDirectoryDatabase>(path, env_override_);
     if (db->Init(SandboxDirectoryDatabase::FAIL_ON_CORRUPTION)) {
-      directories_[GetDirectoryDatabaseKey(origin, type_string)] = db.release();
+      directories_[GetDirectoryDatabaseKey(origin, type_string)] =
+          std::move(db);
       MarkUsed();
       // Don't populate more than one database, as it may rather hurt
       // performance.
@@ -1168,10 +1169,10 @@ SandboxDirectoryDatabase* ObfuscatedFileUtil::GetDirectoryDatabase(
   if (key.empty())
     return NULL;
 
-  DirectoryMap::iterator iter = directories_.find(key);
+  auto iter = directories_.find(key);
   if (iter != directories_.end()) {
     MarkUsed();
-    return iter->second;
+    return iter->second.get();
   }
 
   base::File::Error error = base::File::FILE_OK;
@@ -1182,10 +1183,9 @@ SandboxDirectoryDatabase* ObfuscatedFileUtil::GetDirectoryDatabase(
     return NULL;
   }
   MarkUsed();
-  SandboxDirectoryDatabase* database =
-      new SandboxDirectoryDatabase(path, env_override_);
-  directories_[key] = database;
-  return database;
+  directories_[key] =
+      base::MakeUnique<SandboxDirectoryDatabase>(path, env_override_);
+  return directories_[key].get();
 }
 
 base::FilePath ObfuscatedFileUtil::GetDirectoryForOrigin(
@@ -1264,8 +1264,6 @@ void ObfuscatedFileUtil::MarkUsed() {
 
 void ObfuscatedFileUtil::DropDatabases() {
   origin_database_.reset();
-  base::STLDeleteContainerPairSecondPointers(directories_.begin(),
-                                             directories_.end());
   directories_.clear();
   timer_.reset();
 }

storage/browser/fileapi/obfuscated_file_util.h

@@ -327,8 +327,7 @@ class STORAGE_EXPORT ObfuscatedFileUtil : public FileSystemFileUtil {
 
   bool HasIsolatedStorage(const GURL& origin);
 
-  typedef std::map<std::string, SandboxDirectoryDatabase*> DirectoryMap;
-  DirectoryMap directories_;
+  std::map<std::string, std::unique_ptr<SandboxDirectoryDatabase>> directories_;
   std::unique_ptr<SandboxOriginDatabaseInterface> origin_database_;
   scoped_refptr<storage::SpecialStoragePolicy> special_storage_policy_;
   base::FilePath file_system_directory_;