Shift passage of FileStream in downloads system to be by scoped_ptr<>.

content/public/test/test_file_error_injector.cc

 // Copyright (c) 2012 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 "content/public/test/test_file_error_injector.h"
 
 #include <vector>
 
 #include "base/compiler_specific.h"
 #include "base/logging.h"
@@ skipping 21 matching lines @@
 }
 
 // A class that performs file operations and injects errors.
 class DownloadFileWithErrors: public DownloadFileImpl {
  public:
   typedef base::Callback<void(const GURL& url, content::DownloadId id)>
       ConstructionCallback;
   typedef base::Callback<void(const GURL& url)> DestructionCallback;
 
   DownloadFileWithErrors(
-      const DownloadCreateInfo* info,
+      scoped_ptr<DownloadCreateInfo> info,
       scoped_ptr<content::ByteStreamReader> stream,
       DownloadRequestHandleInterface* request_handle,
       content::DownloadManager* download_manager,
       bool calculate_hash,
       const net::BoundNetLog& bound_net_log,
+      const GURL& source_url,
       const content::TestFileErrorInjector::FileErrorInfo& error_info,
       const ConstructionCallback& ctor_callback,
       const DestructionCallback& dtor_callback);
 
   ~DownloadFileWithErrors();
 
   // DownloadFile interface.
   virtual content::DownloadInterruptReason Initialize() OVERRIDE;
   virtual content::DownloadInterruptReason AppendDataToFile(
       const char* data, size_t data_len) OVERRIDE;
@@ skipping 37 matching lines @@
     content::DownloadInterruptReason original_error,
     const FilePath& path_result) {
   original_callback.Run(
       overwrite_error,
       overwrite_error == content::DOWNLOAD_INTERRUPT_REASON_NONE ?
       path_result : FilePath());
 }
 
 
 DownloadFileWithErrors::DownloadFileWithErrors(
-    const DownloadCreateInfo* info,
+    scoped_ptr<DownloadCreateInfo> info,
     scoped_ptr<content::ByteStreamReader> stream,
     DownloadRequestHandleInterface* request_handle,
     content::DownloadManager* download_manager,
     bool calculate_hash,
     const net::BoundNetLog& bound_net_log,
+    const GURL& source_url,
     const content::TestFileErrorInjector::FileErrorInfo& error_info,
     const ConstructionCallback& ctor_callback,
     const DestructionCallback& dtor_callback)
-        : DownloadFileImpl(info,
-                           stream.Pass(),
-                           request_handle,
-                           download_manager,
-                           calculate_hash,
-                           scoped_ptr<content::PowerSaveBlocker>(NULL).Pass(),
-                           bound_net_log),
-          source_url_(info->url()),
-          error_info_(error_info),
-          destruction_callback_(dtor_callback) {
+    : DownloadFileImpl(info.Pass(),
+                       stream.Pass(),
+                       request_handle,
+                       download_manager,
+                       calculate_hash,
+                       scoped_ptr<content::PowerSaveBlocker>(NULL).Pass(),
+                       bound_net_log),
+      source_url_(source_url),
+      error_info_(error_info),
+      destruction_callback_(dtor_callback) {
   ctor_callback.Run(source_url_, info->download_id);
 }
 
 DownloadFileWithErrors::~DownloadFileWithErrors() {
   destruction_callback_.Run(source_url_);
 }
 
 content::DownloadInterruptReason DownloadFileWithErrors::Initialize() {
   return ShouldReturnError(
       content::TestFileErrorInjector::FILE_OPERATION_INITIALIZE,
@@ skipping 57 matching lines @@
 class DownloadFileWithErrorsFactory : public content::DownloadFileFactory {
  public:
 
   DownloadFileWithErrorsFactory(
       const DownloadFileWithErrors::ConstructionCallback& ctor_callback,
       const DownloadFileWithErrors::DestructionCallback& dtor_callback);
   virtual ~DownloadFileWithErrorsFactory();
 
   // DownloadFileFactory interface.
   virtual DownloadFile* CreateFile(
-      DownloadCreateInfo* info,
+      scoped_ptr<DownloadCreateInfo> info,
       scoped_ptr<content::ByteStreamReader> stream,
       content::DownloadManager* download_manager,
       bool calculate_hash,
       const net::BoundNetLog& bound_net_log);
 
   bool AddError(
       const TestFileErrorInjector::FileErrorInfo& error_info);
 
   void ClearErrors();
 
@@ skipping 10 matching lines @@
     const DownloadFileWithErrors::ConstructionCallback& ctor_callback,
     const DownloadFileWithErrors::DestructionCallback& dtor_callback)
         : construction_callback_(ctor_callback),
           destruction_callback_(dtor_callback) {
 }
 
 DownloadFileWithErrorsFactory::~DownloadFileWithErrorsFactory() {
 }
 
 content::DownloadFile* DownloadFileWithErrorsFactory::CreateFile(
-    DownloadCreateInfo* info,
+    scoped_ptr<DownloadCreateInfo> info,
     scoped_ptr<content::ByteStreamReader> stream,
     content::DownloadManager* download_manager,
     bool calculate_hash,
     const net::BoundNetLog& bound_net_log) {
-  std::string url = info->url().spec();
+  GURL url = info->url();
 
-  if (injected_errors_.find(url) == injected_errors_.end()) {
+  if (injected_errors_.find(url.spec()) == injected_errors_.end()) {
     // Have to create entry, because FileErrorInfo is not a POD type.
     TestFileErrorInjector::FileErrorInfo err_info = {
-      url,
+      url.spec(),
       TestFileErrorInjector::FILE_OPERATION_INITIALIZE,
       -1,
       content::DOWNLOAD_INTERRUPT_REASON_NONE
     };
-    injected_errors_[url] = err_info;
+    injected_errors_[url.spec()] = err_info;
   }
 
+  DownloadRequestHandle* request_handle(
+      new DownloadRequestHandle(info->request_handle));
+
   return new DownloadFileWithErrors(
-      info,
+      info.Pass(),
       stream.Pass(),
-      new DownloadRequestHandle(info->request_handle),
+      request_handle,                   // Takes ownership.

[benjhayden, 2012/10/11 19:41:12]

Again, this comment belongs in DFWE, not here.

[Randy Smith (Not in Mondays), 2012/10/15 18:56:32]

Moot because of your other suggestion, but if I were leaving the code the way it
was: The purpose of this comment is to warn someone looking at this code away
from doing something they'll regret.  I would claim that's a reasonable worry. 
The comment should certainly be in DFWE.h, but I think it's appropriate for it
to be here as well; it's saying "don't do anything with this pointer after this
point".

       download_manager,
       calculate_hash,
       bound_net_log,
-      injected_errors_[url],
+      url,
+      injected_errors_[url.spec()],
       construction_callback_,
       destruction_callback_);
 }
 
 bool DownloadFileWithErrorsFactory::AddError(
     const TestFileErrorInjector::FileErrorInfo& error_info) {
   // Creates an empty entry if necessary.  Duplicate entries overwrite.
   injected_errors_[error_info.url] = error_info;
 
   return true;
@@ skipping 188 matching lines @@
     case FILE_OPERATION_RENAME:
       return "RENAME";
     default:
       break;
   }
 
   return "Unknown";
 }
 
 }  // namespace content