Remove stl_util's deletion functions from storage/.

storage/browser/fileapi/copy_or_move_operation_delegate.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "storage/browser/fileapi/copy_or_move_operation_delegate.h"
 
 #include <stdint.h>
 
 #include <memory>
 #include <tuple>
 #include <utility>
 
 #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"
 #include "storage/browser/fileapi/copy_or_move_file_validator.h"
 #include "storage/browser/fileapi/file_observers.h"
 #include "storage/browser/fileapi/file_stream_reader.h"
 #include "storage/browser/fileapi/file_stream_writer.h"
 #include "storage/browser/fileapi/file_system_context.h"
 #include "storage/browser/fileapi/file_system_operation_runner.h"
 #include "storage/browser/fileapi/file_system_url.h"
@@ skipping 721 matching lines @@
       operation_type_(operation_type),
       option_(option),
       error_behavior_(error_behavior),
       progress_callback_(progress_callback),
       callback_(callback),
       weak_factory_(this) {
   same_file_system_ = src_root_.IsInSameFileSystem(dest_root_);
 }
 
 CopyOrMoveOperationDelegate::~CopyOrMoveOperationDelegate() {
-  base::STLDeleteElements(&running_copy_set_);
 }
 
 void CopyOrMoveOperationDelegate::Run() {
   // Not supported; this should never be called.
   NOTREACHED();
 }
 
 void CopyOrMoveOperationDelegate::RunRecursively() {
   // Perform light-weight checks first.
 
@@ skipping 20 matching lines @@
 
 void CopyOrMoveOperationDelegate::ProcessFile(
     const FileSystemURL& src_url,
     const StatusCallback& callback) {
   if (!progress_callback_.is_null()) {
     progress_callback_.Run(
         FileSystemOperation::BEGIN_COPY_ENTRY, src_url, FileSystemURL(), 0);
   }
 
   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));
   } else {
     // Cross filesystem case.
     base::File::Error error = base::File::FILE_ERROR_FAILED;
     CopyOrMoveFileValidatorFactory* validator_factory =
         file_system_context()->GetCopyOrMoveFileValidatorFactory(
             dest_root_.type(), &error);
     if (error != base::File::FILE_OK) {
       if (!progress_callback_.is_null())
         progress_callback_.Run(FileSystemOperation::ERROR_COPY_ENTRY, src_url,
                                dest_url, 0);
 
       callback.Run(error);
       return;
     }
 
     if (!validator_factory) {
       std::unique_ptr<storage::FileStreamReader> reader =
           file_system_context()->CreateFileStreamReader(
               src_url, 0 /* offset */, storage::kMaximumLength, base::Time());
       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,
                        weak_factory_.GetWeakPtr(), src_url));
       }
     }
 
     if (!impl) {
-      impl = new SnapshotCopyOrMoveImpl(
+      impl = base::MakeUnique<SnapshotCopyOrMoveImpl>(
           operation_runner(), operation_type_, src_url, dest_url, option_,
           validator_factory,
           base::Bind(&CopyOrMoveOperationDelegate::OnCopyFileProgress,
                      weak_factory_.GetWeakPtr(), src_url));
     }
   }
 
   // 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();
+  ;

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

extra;

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

Done.

+  running_copy_set_.insert(std::move(impl));
+  impl_ptr->Run(base::Bind(&CopyOrMoveOperationDelegate::DidCopyOrMoveFile,
+                           weak_factory_.GetWeakPtr(), src_url, dest_url,
+                           callback, impl_ptr));
 }
 
 void CopyOrMoveOperationDelegate::ProcessDirectory(
     const FileSystemURL& src_url,
     const StatusCallback& callback) {
   if (src_url == src_root_) {
     // The src_root_ looks to be a directory.
     // Try removing the dest_root_ to see if it exists and/or it is an
     // empty directory.
     // We do not invoke |progress_callback_| for source root, because it is
@@ skipping 24 matching lines @@
 
   operation_runner()->GetMetadata(
       src_url, FileSystemOperation::GET_METADATA_FIELD_LAST_MODIFIED,
       base::Bind(
           &CopyOrMoveOperationDelegate::PostProcessDirectoryAfterGetMetadata,
           weak_factory_.GetWeakPtr(), src_url, callback));
 }
 
 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->Cancel();
 }
 
 void CopyOrMoveOperationDelegate::DidCopyOrMoveFile(
     const FileSystemURL& src_url,
     const FileSystemURL& dest_url,
     const StatusCallback& callback,
     CopyOrMoveImpl* impl,
     base::File::Error error) {
-  running_copy_set_.erase(impl);
-  delete impl;
+  running_copy_set_.erase(

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

I assume this set is small and not being called much and thats why we're doing
std::find_if instead of a map where we could use better than O(n) map::find?
Maybe you can say this in a comment.

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

Switching to map.

+      std::find_if(running_copy_set_.begin(), running_copy_set_.end(),
+                   [impl](const std::unique_ptr<CopyOrMoveImpl>& ptr) {
+                     return ptr.get() == impl;
+                   }));
 
   if (!progress_callback_.is_null() && error != base::File::FILE_OK &&
       error != base::File::FILE_ERROR_NOT_A_FILE)
     progress_callback_.Run(FileSystemOperation::ERROR_COPY_ENTRY, src_url,
                            dest_url, 0);
 
   if (!progress_callback_.is_null() && error == base::File::FILE_OK) {
     progress_callback_.Run(
         FileSystemOperation::END_COPY_ENTRY, src_url, dest_url, 0);
   }
@@ skipping 110 matching lines @@
   base::FilePath relative = dest_root_.virtual_path();
   src_root_.virtual_path().AppendRelativePath(src_url.virtual_path(),
                                               &relative);
   return file_system_context()->CreateCrackedFileSystemURL(
       dest_root_.origin(),
       dest_root_.mount_type(),
       relative);
 }
 
 }  // namespace storage