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

content/browser/download/base_file_unittest.cc

Index: content/browser/download/base_file_unittest.cc
diff --git a/content/browser/download/base_file_unittest.cc b/content/browser/download/base_file_unittest.cc
index 6175afe86f9279abd8a04ef7a969deddb919b143..37a112e239309cea58b08dbb65676a8ab28b9099 100644
--- a/content/browser/download/base_file_unittest.cc
+++ b/content/browser/download/base_file_unittest.cc
@@ -57,7 +57,7 @@ class BaseFileTest : public testing::Test {
                                   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()));
   }
 
@@ -108,37 +108,12 @@ class BaseFileTest : public testing::Test {
                                   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_)
@@ -159,14 +134,13 @@ class BaseFileTest : public testing::Test {
   // 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()));
@@ -184,14 +158,13 @@ class BaseFileTest : public testing::Test {
   // 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.
@@ -210,8 +183,11 @@ class BaseFileTest : public testing::Test {
     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.
@@ -405,14 +381,13 @@ TEST_F(BaseFileTest, MultipleWritesInterruptedWithHash) {
   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);
@@ -488,19 +463,36 @@ TEST_F(BaseFileTest, RenameWithError) {
 
 // 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));
@@ -540,7 +532,7 @@ TEST_F(BaseFileTest, AppendToBaseFile) {
                                 kTestDataLength4,
                                 false,
                                 "",
-                                file_stream_,
+                                scoped_ptr<net::FileStream>().Pass(),
                                 net::BoundNetLog()));
 
   EXPECT_EQ(net::OK, base_file_->Initialize(temp_dir_.path()));
@@ -574,7 +566,7 @@ TEST_F(BaseFileTest, ReadonlyBaseFile) {
                                 0,
                                 false,
                                 "",
-                                file_stream_,
+                                scoped_ptr<net::FileStream>().Pass(),
                                 net::BoundNetLog()));
 
   expect_in_progress_ = false;