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

content/browser/download/base_file_unittest.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/browser/download/base_file.h"
 
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
 #include "base/scoped_temp_dir.h"
@@ skipping 39 matching lines @@
 
   virtual void SetUp() {
     ResetHash();
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
     base_file_.reset(new BaseFile(FilePath(),
                                   GURL(),
                                   GURL(),
                                   0,
                                   false,
                                   "",
-                                  file_stream_,
+                                  scoped_ptr<net::FileStream>().Pass(),

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

Do you need the .Pass()?

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

Apparently not.  Weird.  Done.

                                   net::BoundNetLog()));
   }
 
   virtual void TearDown() {
     EXPECT_FALSE(base_file_->in_progress());
     if (!expected_error_) {
       EXPECT_EQ(static_cast<int64>(expected_data_.size()),
                 base_file_->bytes_so_far());
     }
 
@@ skipping 30 matching lines @@
     return hash;
   }
 
   void MakeFileWithHash() {
     base_file_.reset(new BaseFile(FilePath(),
                                   GURL(),
                                   GURL(),
                                   0,
                                   true,
                                   "",
-                                  file_stream_,
+                                  scoped_ptr<net::FileStream>().Pass(),
                                   net::BoundNetLog()));
   }
 
-  bool OpenMockFileStream() {
-    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
-
-    FilePath path;
-    if (!file_util::CreateTemporaryFile(&path))
-      return false;
-
-    // Create a new file stream.
-    mock_file_stream_.reset(new net::testing::MockFileStream(NULL));
-    if (mock_file_stream_->OpenSync(
-        path,
-        base::PLATFORM_FILE_OPEN_ALWAYS | base::PLATFORM_FILE_WRITE) != 0) {
-      mock_file_stream_.reset();
-      return false;
-    }
-
-    return true;
-  }
-
-  void ForceError(net::Error error) {
-    mock_file_stream_->set_forced_error(error);
-  }
-
   int AppendDataToFile(const std::string& data) {
     EXPECT_EQ(expect_in_progress_, base_file_->in_progress());
-    expected_error_ = mock_file_stream_.get() &&
-                      (mock_file_stream_->forced_error() != net::OK);
     int appended = base_file_->AppendDataToFile(data.data(), data.size());
     if (appended == net::OK)
       EXPECT_TRUE(expect_in_progress_)
           << " appended = " << appended;
     if (base_file_->in_progress()) {
       expected_data_ += data;
       if (!expected_error_) {
         EXPECT_EQ(static_cast<int64>(expected_data_.size()),
                   base_file_->bytes_so_far());
       }
     }
     return appended;
   }
 
   void set_expected_data(const std::string& data) { expected_data_ = data; }
 
   // Helper functions.
   // Create a file.  Returns the complete file path.
   FilePath CreateTestFile() {
     FilePath file_name;
-    linked_ptr<net::FileStream> dummy_file_stream;
     BaseFile file(FilePath(),
                   GURL(),
                   GURL(),
                   0,
                   false,
                   "",
-                  dummy_file_stream,
+                  scoped_ptr<net::FileStream>(),
                   net::BoundNetLog());
 
     EXPECT_EQ(net::OK, file.Initialize(temp_dir_.path()));
     file_name = file.full_path();
     EXPECT_NE(FilePath::StringType(), file_name.value());
 
     EXPECT_EQ(net::OK, file.AppendDataToFile(kTestData4, kTestDataLength4));
 
     // Keep the file from getting deleted when existing_file_name is deleted.
     file.Detach();
 
     return file_name;
   }
 
   // Create a file with the specified file name.
   void CreateFileWithName(const FilePath& file_name) {
     EXPECT_NE(FilePath::StringType(), file_name.value());
-    linked_ptr<net::FileStream> dummy_file_stream;
     BaseFile duplicate_file(file_name,
                             GURL(),
                             GURL(),
                             0,
                             false,
                             "",
-                            dummy_file_stream,
+                            scoped_ptr<net::FileStream>(),
                             net::BoundNetLog());
     EXPECT_EQ(net::OK, duplicate_file.Initialize(temp_dir_.path()));
     // Write something into it.
     duplicate_file.AppendDataToFile(kTestData4, kTestDataLength4);
     // Detach the file so it isn't deleted on destruction of |duplicate_file|.
     duplicate_file.Detach();
   }
 
   int64 CurrentSpeedAtTime(base::TimeTicks current_time) {
     EXPECT_TRUE(base_file_.get());
     return base_file_->CurrentSpeedAtTime(current_time);
   }
 
   base::TimeTicks StartTick() {
     EXPECT_TRUE(base_file_.get());
     return base_file_->start_tick_;
   }
 
+  void set_expected_error(net::Error err) {
+    expected_error_ = err;
+  }
+
  protected:
-  linked_ptr<net::FileStream> file_stream_;
   linked_ptr<net::testing::MockFileStream> mock_file_stream_;
 
   // BaseClass instance we are testing.
   scoped_ptr<BaseFile> base_file_;
 
   // Temporary directory for renamed downloads.
   ScopedTempDir temp_dir_;
 
   // Expect the file to survive deletion of the BaseFile instance.
   bool expect_file_survives_;
@@ skipping 173 matching lines @@
   // Write some data
   ASSERT_EQ(net::OK, AppendDataToFile(kTestData1));
   ASSERT_EQ(net::OK, AppendDataToFile(kTestData2));
   // Get the hash state and file name.
   std::string hash_state;
   hash_state = base_file_->GetHashState();
   // Finish the file.
   base_file_->Finish();
 
   // Create another file
-  linked_ptr<net::FileStream> second_stream;
   BaseFile second_file(FilePath(),
                        GURL(),
                        GURL(),
                        base_file_->bytes_so_far(),
                        true,
                        hash_state,
-                       second_stream,
+                       scoped_ptr<net::FileStream>(),
                        net::BoundNetLog());
   ASSERT_EQ(net::OK, second_file.Initialize(temp_dir_.path()));
   std::string data(kTestData3);
   EXPECT_EQ(net::OK, second_file.AppendDataToFile(data.data(), data.size()));
   second_file.Finish();
 
   std::string hash;
   EXPECT_TRUE(second_file.GetHash(&hash));
   // This will fail until getting the hash state is supported in SecureHash.
   EXPECT_STREQ(expected_hash_hex.c_str(),
@@ skipping 55 matching lines @@
     file_util::PermissionRestorer restore_permissions_for(test_dir);
     ASSERT_TRUE(file_util::MakeFileUnwritable(test_dir));
     EXPECT_EQ(net::ERR_ACCESS_DENIED, base_file_->Rename(new_path));
   }
 
   base_file_->Finish();
 }
 
 // Write data to the file multiple times.
 TEST_F(BaseFileTest, MultipleWritesWithError) {
-  ASSERT_TRUE(OpenMockFileStream());
-  base_file_.reset(new BaseFile(mock_file_stream_->get_path(),
+  FilePath path;
+  ASSERT_TRUE(file_util::CreateTemporaryFile(&path));
+  // Create a new file stream.  scoped_ptr takes ownership and passes it to
+  // BaseFile; we use the pointer anyway and rely on the BaseFile not
+  // deleting the MockFileStream until the BaseFile is reset.
+  net::testing::MockFileStream* mock_file_stream_p(

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

What's the '_p' for?

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

"pointer" but I agree it's confusing.  I've tried a new naming scheme.  Let me
know what you think.

+      new net::testing::MockFileStream(NULL));
+  scoped_ptr<net::FileStream> mock_file_stream(mock_file_stream_p);
+
+  ASSERT_EQ(0,
+            mock_file_stream_p->OpenSync(
+                path,
+                base::PLATFORM_FILE_OPEN_ALWAYS | base::PLATFORM_FILE_WRITE));
+
+  // Copy of mock_file_stream; we pass ownership and rely on the BaseFile
+  // not deleting it until it is reset.
+
+  base_file_.reset(new BaseFile(mock_file_stream_p->get_path(),
                                 GURL(),
                                 GURL(),
                                 0,
                                 false,
                                 "",
-                                mock_file_stream_,
+                                mock_file_stream.Pass(),
                                 net::BoundNetLog()));
   EXPECT_EQ(net::OK, base_file_->Initialize(temp_dir_.path()));
   ASSERT_EQ(net::OK, AppendDataToFile(kTestData1));
   ASSERT_EQ(net::OK, AppendDataToFile(kTestData2));
-  ForceError(net::ERR_ACCESS_DENIED);
+  mock_file_stream_p->set_forced_error(net::ERR_ACCESS_DENIED);
+  set_expected_error(net::ERR_ACCESS_DENIED);
   ASSERT_NE(net::OK, AppendDataToFile(kTestData3));
   std::string hash;
   EXPECT_FALSE(base_file_->GetHash(&hash));
   base_file_->Finish();
 }
 
 // Try to write to uninitialized file.
 TEST_F(BaseFileTest, UninitializedFile) {
   expect_in_progress_ = false;
   EXPECT_EQ(net::ERR_INVALID_HANDLE, AppendDataToFile(kTestData1));
@@ skipping 19 matching lines @@
 
   set_expected_data(kTestData4);
 
   // Use the file we've just created.
   base_file_.reset(new BaseFile(existing_file_name,
                                 GURL(),
                                 GURL(),
                                 kTestDataLength4,
                                 false,
                                 "",
-                                file_stream_,
+                                scoped_ptr<net::FileStream>().Pass(),
                                 net::BoundNetLog()));
 
   EXPECT_EQ(net::OK, base_file_->Initialize(temp_dir_.path()));
 
   const FilePath file_name = base_file_->full_path();
   EXPECT_NE(FilePath::StringType(), file_name.value());
 
   // Write into the file.
   EXPECT_EQ(net::OK, AppendDataToFile(kTestData1));
 
@@ skipping 13 matching lines @@
   // Make it read-only.
   EXPECT_TRUE(file_util::MakeFileUnwritable(readonly_file_name));
 
   // Try to overwrite it.
   base_file_.reset(new BaseFile(readonly_file_name,
                                 GURL(),
                                 GURL(),
                                 0,
                                 false,
                                 "",
-                                file_stream_,
+                                scoped_ptr<net::FileStream>().Pass(),
                                 net::BoundNetLog()));
 
   expect_in_progress_ = false;
 
   int init_error = base_file_->Initialize(temp_dir_.path());
   DVLOG(1) << " init_error = " << init_error;
   EXPECT_NE(net::OK, init_error);
 
   const FilePath file_name = base_file_->full_path();
   EXPECT_NE(FilePath::StringType(), file_name.value());
@@ skipping 65 matching lines @@
   // be a string-wise match to base_file_->full_path().DirName() even though
   // they are in the same directory.
   FilePath temp_file;
   ASSERT_TRUE(file_util::CreateTemporaryFileInDir(temp_dir_.path(),
                                                   &temp_file));
   ASSERT_FALSE(temp_file.empty());
   EXPECT_STREQ(temp_file.DirName().value().c_str(),
                base_file_->full_path().DirName().value().c_str());
   base_file_->Finish();
 }