Remove stl_util's deletion functions from storage/.

storage/browser/fileapi/file_system_usage_cache.cc

Index: storage/browser/fileapi/file_system_usage_cache.cc
diff --git a/storage/browser/fileapi/file_system_usage_cache.cc b/storage/browser/fileapi/file_system_usage_cache.cc
index 44d291fc60aa48378beb2df5eca427bc3c751a62..bf96d0e334ed3741f364895d8021d41fef12b2fb 100644
--- a/storage/browser/fileapi/file_system_usage_cache.cc
+++ b/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,20 @@ 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;
-
-  new_file = new 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;
+  auto& entry = cache_files_[file_path];
+  if (entry)

[danakj, 2016/10/03 22:51:49]

This is slightly different but it looks like we never have entries in the map
with a null. Can you say that in a comment?

[Avi (use Gerrit), 2016/10/04 15:56:22]

The old code is weird. It does an insert with a null just to see if an insert
succeeds, and if the insert succeeds then it constructs a file.

Because the old code is weird, I rewrote it when I switched containers. I don't
see why we need to bring over the weirdness to the new code; the default
assumption that I see is that of course the entry in the map wouldn't be null.

[danakj, 2016/10/04 19:55:52]

I just think it would be helpful when reading this code to see a comment about
the assumption its making about the rest of the class, that null is not a valid
thing in the container. I think it's a fine assumption, but documenting that
it's being made.

[Avi (use Gerrit), 2016/10/04 22:06:07]

Done.

+    return entry.get();
+
+  entry = base::MakeUnique<base::File>(file_path, base::File::FLAG_OPEN_ALWAYS |
+                                                      base::File::FLAG_READ |
+                                                      base::File::FLAG_WRITE);
+
+  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,