Remove stl_util's deletion functions from storage/.

BUG=555865

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

storage/browser/blob/blob_reader.cc

@@ -15,7 +15,6 @@
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/sequenced_task_runner.h"
-#include "base/stl_util.h"
 #include "base/time/time.h"
 #include "base/trace_event/trace_event.h"
 #include "net/base/io_buffer.h"
@@ -76,7 +75,6 @@ BlobReader::BlobReader(
 }
 
 BlobReader::~BlobReader() {
-  base::STLDeleteValues(&index_to_reader_);
 }
 
 BlobReader::Status BlobReader::CalculateSize(
@@ -620,13 +618,13 @@ FileStreamReader* BlobReader::GetOrCreateFileReaderAtIndex(size_t index) {
   auto it = index_to_reader_.find(index);
   if (it != index_to_reader_.end()) {
     DCHECK(it->second);
-    return it->second;
+    return it->second.get();
   }
   std::unique_ptr<FileStreamReader> reader = CreateFileStreamReader(item, 0);
   FileStreamReader* ret_value = reader.get();
   if (!ret_value)
     return nullptr;
-  index_to_reader_[index] = reader.release();
+  index_to_reader_[index] = std::move(reader);
   return ret_value;
 }
 
@@ -662,19 +660,10 @@ std::unique_ptr<FileStreamReader> BlobReader::CreateFileStreamReader(
 void BlobReader::SetFileReaderAtIndex(
     size_t index,
     std::unique_ptr<FileStreamReader> reader) {
-  auto found = index_to_reader_.find(current_item_index_);
-  if (found != index_to_reader_.end()) {
-    if (found->second) {
-      delete found->second;
-    }
-    if (!reader.get()) {
-      index_to_reader_.erase(found);
-      return;
-    }
-    found->second = reader.release();
-  } else if (reader.get()) {
-    index_to_reader_[current_item_index_] = reader.release();
-  }
+  if (reader)
+    index_to_reader_[index] = std::move(reader);
+  else
+    index_to_reader_.erase(index);
 }
 
 }  // namespace storage

storage/browser/blob/blob_reader.h

@@ -214,7 +214,7 @@ class STORAGE_EXPORT BlobReader {
   uint64_t total_size_ = 0;
   uint64_t remaining_bytes_ = 0;
   size_t pending_get_file_info_count_ = 0;
-  std::map<size_t, FileStreamReader*> index_to_reader_;
+  std::map<size_t, std::unique_ptr<FileStreamReader>> index_to_reader_;
   size_t current_item_index_ = 0;
   uint64_t current_item_offset_ = 0;
 

storage/browser/fileapi/copy_or_move_operation_delegate.cc

@@ -13,6 +13,7 @@
 #include "base/bind.h"
 #include "base/files/file_path.h"
 #include "base/macros.h"
+#include "base/memory/ptr_util.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
 #include "storage/browser/blob/shareable_file_reference.h"
@@ -754,7 +755,6 @@ CopyOrMoveOperationDelegate::CopyOrMoveOperationDelegate(
 }
 
 CopyOrMoveOperationDelegate::~CopyOrMoveOperationDelegate() {
-  base::STLDeleteElements(&running_copy_set_);
 }
 
 void CopyOrMoveOperationDelegate::Run() {
@@ -795,13 +795,13 @@ void CopyOrMoveOperationDelegate::ProcessFile(
   }
 
   FileSystemURL dest_url = CreateDestURL(src_url);
-  CopyOrMoveImpl* impl = NULL;
+  std::unique_ptr<CopyOrMoveImpl> impl;
   if (same_file_system_ &&
       (file_system_context()
            ->GetFileSystemBackend(src_url.type())
            ->HasInplaceCopyImplementation(src_url.type()) ||
        operation_type_ == OPERATION_MOVE)) {
-    impl = new CopyOrMoveOnSameFileSystemImpl(
+    impl = base::MakeUnique<CopyOrMoveOnSameFileSystemImpl>(
         operation_runner(), operation_type_, src_url, dest_url, option_,
         base::Bind(&CopyOrMoveOperationDelegate::OnCopyFileProgress,
                    weak_factory_.GetWeakPtr(), src_url));
@@ -827,7 +827,7 @@ void CopyOrMoveOperationDelegate::ProcessFile(
       std::unique_ptr<FileStreamWriter> writer =
           file_system_context()->CreateFileStreamWriter(dest_url, 0);
       if (reader && writer) {
-        impl = new StreamCopyOrMoveImpl(
+        impl = base::MakeUnique<StreamCopyOrMoveImpl>(
             operation_runner(), file_system_context(), operation_type_, src_url,
             dest_url, option_, std::move(reader), std::move(writer),
             base::Bind(&CopyOrMoveOperationDelegate::OnCopyFileProgress,
@@ -836,7 +836,7 @@ void CopyOrMoveOperationDelegate::ProcessFile(
     }
 
     if (!impl) {
-      impl = new SnapshotCopyOrMoveImpl(
+      impl = base::MakeUnique<SnapshotCopyOrMoveImpl>(
           operation_runner(), operation_type_, src_url, dest_url, option_,
           validator_factory,
           base::Bind(&CopyOrMoveOperationDelegate::OnCopyFileProgress,
@@ -845,10 +845,12 @@ void CopyOrMoveOperationDelegate::ProcessFile(
   }
 
   // Register the running task.
-  running_copy_set_.insert(impl);
-  impl->Run(base::Bind(
-      &CopyOrMoveOperationDelegate::DidCopyOrMoveFile,
-      weak_factory_.GetWeakPtr(), src_url, dest_url, callback, impl));
+
+  CopyOrMoveImpl* impl_ptr = impl.get();
+  running_copy_set_[impl_ptr] = std::move(impl);
+  impl_ptr->Run(base::Bind(&CopyOrMoveOperationDelegate::DidCopyOrMoveFile,
+                           weak_factory_.GetWeakPtr(), src_url, dest_url,
+                           callback, impl_ptr));
 }
 
 void CopyOrMoveOperationDelegate::ProcessDirectory(
@@ -893,9 +895,8 @@ void CopyOrMoveOperationDelegate::PostProcessDirectory(
 
 void CopyOrMoveOperationDelegate::OnCancel() {
   // Request to cancel all running Copy/Move file.
-  for (std::set<CopyOrMoveImpl*>::iterator iter = running_copy_set_.begin();
-       iter != running_copy_set_.end(); ++iter)
-    (*iter)->Cancel();
+  for (auto& job : running_copy_set_)
+    job.first->Cancel();
 }
 
 void CopyOrMoveOperationDelegate::DidCopyOrMoveFile(
@@ -905,7 +906,6 @@ void CopyOrMoveOperationDelegate::DidCopyOrMoveFile(
     CopyOrMoveImpl* impl,
     base::File::Error error) {
   running_copy_set_.erase(impl);
-  delete impl;
 
   if (!progress_callback_.is_null() && error != base::File::FILE_OK &&
       error != base::File::FILE_ERROR_NOT_A_FILE)

storage/browser/fileapi/copy_or_move_operation_delegate.h

@@ -7,8 +7,8 @@
 
 #include <stdint.h>
 
+#include <map>
 #include <memory>
-#include <set>
 #include <stack>
 
 #include "base/macros.h"
@@ -156,7 +156,7 @@ class CopyOrMoveOperationDelegate
   CopyProgressCallback progress_callback_;
   StatusCallback callback_;
 
-  std::set<CopyOrMoveImpl*> running_copy_set_;
+  std::map<CopyOrMoveImpl*, std::unique_ptr<CopyOrMoveImpl>> running_copy_set_;
   base::WeakPtrFactory<CopyOrMoveOperationDelegate> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(CopyOrMoveOperationDelegate);

storage/browser/fileapi/file_system_usage_cache.cc

@@ -11,6 +11,7 @@
 
 #include "base/bind.h"
 #include "base/files/file_util.h"
+#include "base/memory/ptr_util.h"
 #include "base/pickle.h"
 #include "base/stl_util.h"
 #include "base/trace_event/trace_event.h"
@@ -161,7 +162,7 @@ bool FileSystemUsageCache::Delete(const base::FilePath& usage_file_path) {
 void FileSystemUsageCache::CloseCacheFiles() {
   TRACE_EVENT0("FileSystem", "UsageCache::CloseCacheFiles");
   DCHECK(CalledOnValidThread());
-  base::STLDeleteValues(&cache_files_);
+  cache_files_.clear();
   timer_.reset();
 }
 
@@ -228,24 +229,22 @@ base::File* FileSystemUsageCache::GetFile(const base::FilePath& file_path) {
     CloseCacheFiles();
   ScheduleCloseTimer();
 
-  base::File* new_file = NULL;
-  std::pair<CacheFiles::iterator, bool> inserted =
-      cache_files_.insert(std::make_pair(file_path, new_file));
-  if (!inserted.second)
-    return inserted.first->second;
+  auto& entry = cache_files_[file_path];
+  if (entry)
+    return entry.get();
 
-  new_file = new base::File(file_path,
-                            base::File::FLAG_OPEN_ALWAYS |
+  // Because there are no null entries in cache_files_, the [] inserted a blank
+  // pointer, so let's populate the cache.
+  entry = base::MakeUnique<base::File>(file_path, base::File::FLAG_OPEN_ALWAYS |
                                                       base::File::FLAG_READ |
                                                       base::File::FLAG_WRITE);
-  if (!new_file->IsValid()) {
-    cache_files_.erase(inserted.first);
-    delete new_file;
-    return NULL;
+
+  if (!entry->IsValid()) {
+    cache_files_.erase(file_path);
+    return nullptr;
   }
 
-  inserted.first->second = new_file;
-  return new_file;
+  return entry.get();
 }
 
 bool FileSystemUsageCache::ReadBytes(const base::FilePath& file_path,

storage/browser/fileapi/file_system_usage_cache.h

@@ -63,8 +63,6 @@ class STORAGE_EXPORT FileSystemUsageCache {
   static const int kUsageFileHeaderSize;
 
  private:
-  typedef std::map<base::FilePath, base::File*> CacheFiles;
-
   // Read the size, validity and the "dirty" entry described in the .usage file.
   // Returns less than zero if no .usage file is available.
   bool Read(const base::FilePath& usage_file_path,
@@ -93,7 +91,7 @@ class STORAGE_EXPORT FileSystemUsageCache {
   bool CalledOnValidThread();
 
   std::unique_ptr<TimedTaskHelper> timer_;
-  CacheFiles cache_files_;
+  std::map<base::FilePath, std::unique_ptr<base::File>> cache_files_;
 
   scoped_refptr<base::SequencedTaskRunner> task_runner_;
 

storage/browser/quota/storage_monitor.cc

@@ -8,7 +8,7 @@
 
 #include <algorithm>
 
-#include "base/stl_util.h"
+#include "base/memory/ptr_util.h"
 #include "base/trace_event/trace_event.h"
 #include "net/base/url_util.h"
 #include "storage/browser/quota/quota_manager.h"
@@ -247,7 +247,6 @@ StorageTypeObservers::StorageTypeObservers(QuotaManager* quota_manager)
 }
 
 StorageTypeObservers::~StorageTypeObservers() {
-  base::STLDeleteValues(&host_observers_map_);
 }
 
 void StorageTypeObservers::AddObserver(
@@ -256,25 +255,22 @@ void StorageTypeObservers::AddObserver(
   if (host.empty())
     return;
 
-  HostStorageObservers* host_observers = NULL;
-  HostObserversMap::iterator it = host_observers_map_.find(host);
-  if (it == host_observers_map_.end()) {
-    host_observers = new HostStorageObservers(quota_manager_);
-    host_observers_map_[host] = host_observers;
-  } else {
-    host_observers = it->second;
+  auto& host_observers = host_observers_map_[host];
+  if (!host_observers) {
+    // Because there are no null entries in host_observers_map_, the [] inserted
+    // a blank pointer, so let's populate it.
+    host_observers = base::MakeUnique<HostStorageObservers>(quota_manager_);
   }
 
   host_observers->AddObserver(observer, params);
 }
 
 void StorageTypeObservers::RemoveObserver(StorageObserver* observer) {
-  for (HostObserversMap::iterator it = host_observers_map_.begin();
-       it != host_observers_map_.end(); ) {
+  for (auto it = host_observers_map_.begin();
+       it != host_observers_map_.end();) {
     it->second->RemoveObserver(observer);
     if (!it->second->ContainsObservers()) {
-      delete it->second;
-      host_observers_map_.erase(it++);
+      it = host_observers_map_.erase(it);
     } else {
       ++it;
     }
@@ -284,31 +280,29 @@ void StorageTypeObservers::RemoveObserver(StorageObserver* observer) {
 void StorageTypeObservers::RemoveObserverForFilter(
     StorageObserver* observer, const StorageObserver::Filter& filter) {
   std::string host = net::GetHostOrSpecFromURL(filter.origin);
-  HostObserversMap::iterator it = host_observers_map_.find(host);
+  auto it = host_observers_map_.find(host);
   if (it == host_observers_map_.end())
     return;
 
   it->second->RemoveObserver(observer);
-  if (!it->second->ContainsObservers()) {
-    delete it->second;
+  if (!it->second->ContainsObservers())
     host_observers_map_.erase(it);
-  }
 }
 
 const HostStorageObservers* StorageTypeObservers::GetHostObservers(
     const std::string& host) const {
-  HostObserversMap::const_iterator it = host_observers_map_.find(host);
+  auto it = host_observers_map_.find(host);
   if (it != host_observers_map_.end())
-    return it->second;
+    return it->second.get();
 
-  return NULL;
+  return nullptr;
 }
 
 void StorageTypeObservers::NotifyUsageChange(
     const StorageObserver::Filter& filter,
     int64_t delta) {
   std::string host = net::GetHostOrSpecFromURL(filter.origin);
-  HostObserversMap::iterator it = host_observers_map_.find(host);
+  auto it = host_observers_map_.find(host);
   if (it == host_observers_map_.end())
     return;
 
@@ -323,7 +317,6 @@ StorageMonitor::StorageMonitor(QuotaManager* quota_manager)
 }
 
 StorageMonitor::~StorageMonitor() {
-  base::STLDeleteValues(&storage_type_observers_map_);
 }
 
 void StorageMonitor::AddObserver(
@@ -338,22 +331,16 @@ void StorageMonitor::AddObserver(
     return;
   }
 
-  StorageTypeObservers* type_observers = NULL;
-  StorageTypeObserversMap::iterator it =
-      storage_type_observers_map_.find(params.filter.storage_type);
-  if (it == storage_type_observers_map_.end()) {
-    type_observers = new StorageTypeObservers(quota_manager_);
-    storage_type_observers_map_[params.filter.storage_type] = type_observers;
-  } else {
-    type_observers = it->second;
-  }
+  auto& type_observers =
+      storage_type_observers_map_[params.filter.storage_type];
+  if (!type_observers)
+    type_observers = base::MakeUnique<StorageTypeObservers>(quota_manager_);
 
   type_observers->AddObserver(observer, params);
 }
 
 void StorageMonitor::RemoveObserver(StorageObserver* observer) {
-  for (StorageTypeObserversMap::iterator it =
-           storage_type_observers_map_.begin();
+  for (auto it = storage_type_observers_map_.begin();
        it != storage_type_observers_map_.end(); ++it) {
     it->second->RemoveObserver(observer);
   }
@@ -361,8 +348,7 @@ void StorageMonitor::RemoveObserver(StorageObserver* observer) {
 
 void StorageMonitor::RemoveObserverForFilter(
     StorageObserver* observer, const StorageObserver::Filter& filter) {
-  StorageTypeObserversMap::iterator it =
-      storage_type_observers_map_.find(filter.storage_type);
+  auto it = storage_type_observers_map_.find(filter.storage_type);
   if (it == storage_type_observers_map_.end())
     return;
 
@@ -371,12 +357,11 @@ void StorageMonitor::RemoveObserverForFilter(
 
 const StorageTypeObservers* StorageMonitor::GetStorageTypeObservers(
     StorageType storage_type) const {
-  StorageTypeObserversMap::const_iterator it =
-      storage_type_observers_map_.find(storage_type);
+  auto it = storage_type_observers_map_.find(storage_type);
   if (it != storage_type_observers_map_.end())
-    return it->second;
+    return it->second.get();
 
-  return NULL;
+  return nullptr;
 }
 
 void StorageMonitor::NotifyUsageChange(const StorageObserver::Filter& filter,
@@ -389,8 +374,7 @@ void StorageMonitor::NotifyUsageChange(const StorageObserver::Filter& filter,
     return;
   }
 
-  StorageTypeObserversMap::iterator it =
-      storage_type_observers_map_.find(filter.storage_type);
+  auto it = storage_type_observers_map_.find(filter.storage_type);
   if (it == storage_type_observers_map_.end())
     return;
 

storage/browser/quota/storage_monitor.h

@@ -8,6 +8,7 @@
 #include <stdint.h>
 
 #include <map>
+#include <memory>
 
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
@@ -140,10 +141,9 @@ class STORAGE_EXPORT StorageTypeObservers {
   void NotifyUsageChange(const StorageObserver::Filter& filter, int64_t delta);
 
  private:
-  typedef std::map<std::string, HostStorageObservers*> HostObserversMap;
-
   QuotaManager* quota_manager_;
-  HostObserversMap host_observers_map_;
+  std::map<std::string, std::unique_ptr<HostStorageObservers>>
+      host_observers_map_;
 
   DISALLOW_COPY_AND_ASSIGN(StorageTypeObservers);
 };
@@ -170,10 +170,9 @@ class STORAGE_EXPORT StorageMonitor {
   void NotifyUsageChange(const StorageObserver::Filter& filter, int64_t delta);
 
  private:
-  typedef std::map<StorageType, StorageTypeObservers*> StorageTypeObserversMap;
-
   QuotaManager* quota_manager_;
-  StorageTypeObserversMap storage_type_observers_map_;
+  std::map<StorageType, std::unique_ptr<StorageTypeObservers>>
+      storage_type_observers_map_;
 
   DISALLOW_COPY_AND_ASSIGN(StorageMonitor);
 };