1 files changed, 83 insertions, 66 deletions

diff --git a/content/browser/download/download_file_unittest.cc b/content/browser/download/download_file_unittest.cc
index 78da487..24eaade 100644
--- a/content/browser/download/download_file_unittest.cc
+++ b/content/browser/download/download_file_unittest.cc
@@ -12,7 +12,6 @@
 #include "content/browser/download/download_file_impl.h"
 #include "content/browser/download/download_request_handle.h"
 #include "content/browser/power_save_blocker.h"
-#include "content/public/browser/download_destination_observer.h"
 #include "content/public/browser/download_interrupt_reasons.h"
 #include "content/public/browser/download_manager.h"
 #include "content/public/test/mock_download_manager.h"
@@ -26,6 +25,7 @@ using content::BrowserThread;
 using content::BrowserThreadImpl;
 using content::DownloadFile;
 using content::DownloadId;
+using content::DownloadManager;
 using ::testing::_;
 using ::testing::AnyNumber;
 using ::testing::DoAll;
@@ -48,21 +48,14 @@ class MockByteStreamReader : public content::ByteStreamReader {
   MOCK_METHOD1(RegisterCallback, void(const base::Closure&));
 };
 
-class MockDownloadDestinationObserver
-    : public content::DownloadDestinationObserver {
+class LocalMockDownloadManager : public content::MockDownloadManager {
  public:
-  MOCK_METHOD3(DestinationUpdate, void(int64, int64, const std::string&));
-  MOCK_METHOD1(DestinationError, void(content::DownloadInterruptReason));
-  MOCK_METHOD1(DestinationCompleted, void(const std::string&));
-
-  // Doesn't override any methods in the base class.  Used to make sure
-  // that the last DestinationUpdate before a Destination{Completed,Error}
-  // had the right values.
-  MOCK_METHOD3(CurrentUpdateStatus,
-               void(int64, int64, const std::string&));
+  MOCK_METHOD4(CurrentUpdateStatus,
+               void(int32, int64, int64, const std::string&));
+ protected:
+  virtual ~LocalMockDownloadManager() {}
 };
 
-
 MATCHER(IsNullCallback, "") { return (arg.is_null()); }
 
 }  // namespace
@@ -80,9 +73,12 @@ class DownloadFileTest : public testing::Test {
   static const int kDummyChildId;
   static const int kDummyRequestId;
 
+  // We need a UI |BrowserThread| in order to destruct |download_manager_|,
+  // which has trait |BrowserThread::DeleteOnUIThread|.  Without this,
+  // calling Release() on |download_manager_| won't ever result in its
+  // destructor being called and we get a leak.
   DownloadFileTest() :
-      observer_(new StrictMock<MockDownloadDestinationObserver>),
-      observer_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(observer_.get())),
+      update_download_id_(-1),
       bytes_(-1),
       bytes_per_sec_(-1),
       hash_state_("xyzzy"),
@@ -93,35 +89,44 @@ class DownloadFileTest : public testing::Test {
   ~DownloadFileTest() {
   }
 
-  void SetUpdateDownloadInfo(int64 bytes, int64 bytes_per_sec,
+  void SetUpdateDownloadInfo(int32 id, int64 bytes, int64 bytes_per_sec,
                              const std::string& hash_state) {
+    update_download_id_ = id;
     bytes_ = bytes;
     bytes_per_sec_ = bytes_per_sec;
     hash_state_ = hash_state;
   }
 
   void ConfirmUpdateDownloadInfo() {
-    observer_->CurrentUpdateStatus(bytes_, bytes_per_sec_, hash_state_);
+    download_manager_->CurrentUpdateStatus(
+        update_download_id_, bytes_, bytes_per_sec_, hash_state_);
   }
 
   virtual void SetUp() {
-    EXPECT_CALL(*(observer_.get()), DestinationUpdate(_, _, _))
+    download_manager_ = new StrictMock<LocalMockDownloadManager>;
+    EXPECT_CALL(*(download_manager_.get()),
+                UpdateDownload(
+                    DownloadId(kValidIdDomain, kDummyDownloadId + 0).local(),
+                    _, _, _))
         .Times(AnyNumber())
         .WillRepeatedly(Invoke(this, &DownloadFileTest::SetUpdateDownloadInfo));
   }
 
+  virtual void TearDown() {
+    // When a DownloadManager's reference count drops to 0, it is not
+    // deleted immediately. Instead, a task is posted to the UI thread's
+    // message loop to delete it.
+    // So, drop the reference count to 0 and run the message loop once
+    // to ensure that all resources are cleaned up before the test exits.
+    download_manager_ = NULL;
+    ui_thread_.message_loop()->RunAllPending();
+  }
+
   // Mock calls to this function are forwarded here.
   void RegisterCallback(base::Closure sink_callback) {
     sink_callback_ = sink_callback;
   }
 
-  void SetInterruptReasonCallback(bool* was_called,
-                                  content::DownloadInterruptReason* reason_p,
-                                  content::DownloadInterruptReason reason) {
-    *was_called = true;
-    *reason_p = reason;
-  }
-
   virtual bool CreateDownloadFile(int offset, bool calculate_hash) {
     // There can be only one.
     DCHECK(!download_file_.get());
@@ -134,36 +139,30 @@ class DownloadFileTest : public testing::Test {
         .WillOnce(Invoke(this, &DownloadFileTest::RegisterCallback))
         .RetiresOnSaturation();
 
+    DownloadCreateInfo info;
+    // info.request_handle default constructed to null.
+    info.download_id = DownloadId(kValidIdDomain, kDummyDownloadId + offset);
+    info.save_info.file_stream = file_stream_;
     download_file_.reset(
         new DownloadFileImpl(
-            content::DownloadSaveInfo(),
-            GURL(),                     // Source
-            GURL(),                     // Referrer
-            0,                          // Received bytes
-            calculate_hash,
-            scoped_ptr<content::ByteStreamReader>(input_stream_),
-            net::BoundNetLog(),
+            &info,
+            scoped_ptr<content::ByteStreamReader>(input_stream_).Pass(),
+            new DownloadRequestHandle(),
+            download_manager_, calculate_hash,
             scoped_ptr<content::PowerSaveBlocker>(NULL).Pass(),
-            observer_factory_.GetWeakPtr()));
+            net::BoundNetLog()));
 
     EXPECT_CALL(*input_stream_, Read(_, _))
         .WillOnce(Return(content::ByteStreamReader::STREAM_EMPTY))
         .RetiresOnSaturation();
-
-    base::WeakPtrFactory<DownloadFileTest> weak_ptr_factory(this);
-    bool called = false;
-    content::DownloadInterruptReason result;
-    download_file_->Initialize(base::Bind(
-        &DownloadFileTest::SetInterruptReasonCallback,
-        weak_ptr_factory.GetWeakPtr(), &called, &result));
-    loop_.RunAllPending();
-    EXPECT_TRUE(called);
-
+    content::DownloadInterruptReason result = download_file_->Initialize();
     ::testing::Mock::VerifyAndClearExpectations(input_stream_);
     return result == content::DOWNLOAD_INTERRUPT_REASON_NONE;
   }
 
   virtual void DestroyDownloadFile(int offset) {
+    EXPECT_EQ(kDummyDownloadId + offset, download_file_->Id());
+    EXPECT_EQ(download_manager_, download_file_->GetDownloadManager());
     EXPECT_FALSE(download_file_->InProgress());
     EXPECT_EQ(static_cast<int64>(expected_data_.size()),
               download_file_->BytesSoFar());
@@ -233,16 +232,19 @@ class DownloadFileTest : public testing::Test {
   }
 
   void FinishStream(content::DownloadInterruptReason interrupt_reason,
-                  bool check_observer) {
+                  bool check_download_manager) {
     ::testing::Sequence s1;
     SetupFinishStream(interrupt_reason, s1);
     sink_callback_.Run();
     VerifyStreamAndSize();
-    if (check_observer) {
-      EXPECT_CALL(*(observer_.get()), DestinationCompleted(_));
+    if (check_download_manager) {
+      EXPECT_CALL(*download_manager_, OnResponseCompleted(_, _, _));
       loop_.RunAllPending();
-      ::testing::Mock::VerifyAndClearExpectations(observer_.get());
-      EXPECT_CALL(*(observer_.get()), DestinationUpdate(_, _, _))
+      ::testing::Mock::VerifyAndClearExpectations(download_manager_.get());
+      EXPECT_CALL(*(download_manager_.get()),
+                  UpdateDownload(
+                      DownloadId(kValidIdDomain, kDummyDownloadId + 0).local(),
+                      _, _, _))
           .Times(AnyNumber())
           .WillRepeatedly(Invoke(this,
                                  &DownloadFileTest::SetUpdateDownloadInfo));
@@ -270,9 +272,7 @@ class DownloadFileTest : public testing::Test {
   }
 
  protected:
-  scoped_ptr<StrictMock<MockDownloadDestinationObserver> > observer_;
-  base::WeakPtrFactory<content::DownloadDestinationObserver>
-      observer_factory_;
+  scoped_refptr<StrictMock<LocalMockDownloadManager> > download_manager_;
 
   linked_ptr<net::FileStream> file_stream_;
 
@@ -286,7 +286,8 @@ class DownloadFileTest : public testing::Test {
   // Sink callback data for stream.
   base::Closure sink_callback_;
 
-  // Latest update sent to the observer.
+  // Latest update sent to the download manager.
+  int32 update_download_id_;
   int64 bytes_;
   int64 bytes_per_sec_;
   std::string hash_state_;
@@ -487,12 +488,21 @@ TEST_F(DownloadFileTest, StreamEmptySuccess) {
   // Test that calling the sink_callback_ on an empty stream shouldn't
   // do anything.
   AppendDataToFile(NULL, 0);
+  ::testing::Mock::VerifyAndClearExpectations(download_manager_.get());
+  EXPECT_CALL(*(download_manager_.get()),
+              UpdateDownload(
+                  DownloadId(kValidIdDomain, kDummyDownloadId + 0).local(),
+                  _, _, _))
+      .Times(AnyNumber())
+      .WillRepeatedly(Invoke(this, &DownloadFileTest::SetUpdateDownloadInfo));
 
   // Finish the download this way and make sure we see it on the
-  // observer.
-  EXPECT_CALL(*(observer_.get()), DestinationCompleted(_));
+  // DownloadManager.
+  EXPECT_CALL(*(download_manager_.get()),
+              OnResponseCompleted(DownloadId(kValidIdDomain,
+                                             kDummyDownloadId + 0).local(),
+                                  0, _));
   FinishStream(content::DOWNLOAD_INTERRUPT_REASON_NONE, false);
-  loop_.RunAllPending();
 
   DestroyDownloadFile(0);
 }
@@ -503,9 +513,10 @@ TEST_F(DownloadFileTest, StreamEmptyError) {
   EXPECT_TRUE(file_util::PathExists(initial_path));
 
   // Finish the download in error and make sure we see it on the
-  // observer.
-  EXPECT_CALL(*(observer_.get()),
-              DestinationError(
+  // DownloadManager.
+  EXPECT_CALL(*(download_manager_.get()),
+              OnDownloadInterrupted(
+                  DownloadId(kValidIdDomain, kDummyDownloadId + 0).local(),
                   content::DOWNLOAD_INTERRUPT_REASON_NETWORK_DISCONNECTED))
       .WillOnce(InvokeWithoutArgs(
           this, &DownloadFileTest::ConfirmUpdateDownloadInfo));
@@ -515,7 +526,8 @@ TEST_F(DownloadFileTest, StreamEmptyError) {
   // the last one may have the correct information even if the failure
   // doesn't produce an update, as the timer update may have triggered at the
   // same time.
-  EXPECT_CALL(*(observer_.get()), CurrentUpdateStatus(0, _, _));
+  EXPECT_CALL(*(download_manager_.get()),
+              CurrentUpdateStatus(kDummyDownloadId + 0, 0, _, _));
 
   FinishStream(content::DOWNLOAD_INTERRUPT_REASON_NETWORK_DISCONNECTED, false);
 
@@ -533,10 +545,13 @@ TEST_F(DownloadFileTest, StreamNonEmptySuccess) {
   ::testing::Sequence s1;
   SetupDataAppend(chunks1, 2, s1);
   SetupFinishStream(content::DOWNLOAD_INTERRUPT_REASON_NONE, s1);
-  EXPECT_CALL(*(observer_.get()), DestinationCompleted(_));
+  EXPECT_CALL(*(download_manager_.get()),
+              OnResponseCompleted(DownloadId(kValidIdDomain,
+                                             kDummyDownloadId + 0).local(),
+                                  strlen(kTestData1) + strlen(kTestData2),
+                                  _));
   sink_callback_.Run();
   VerifyStreamAndSize();
-  loop_.RunAllPending();
   DestroyDownloadFile(0);
 }
 
@@ -551,8 +566,9 @@ TEST_F(DownloadFileTest, StreamNonEmptyError) {
   SetupFinishStream(content::DOWNLOAD_INTERRUPT_REASON_NETWORK_DISCONNECTED,
                     s1);
 
-  EXPECT_CALL(*(observer_.get()),
-              DestinationError(
+  EXPECT_CALL(*(download_manager_.get()),
+              OnDownloadInterrupted(
+                  DownloadId(kValidIdDomain, kDummyDownloadId + 0).local(),
                   content::DOWNLOAD_INTERRUPT_REASON_NETWORK_DISCONNECTED))
       .WillOnce(InvokeWithoutArgs(
           this, &DownloadFileTest::ConfirmUpdateDownloadInfo));
@@ -562,8 +578,9 @@ TEST_F(DownloadFileTest, StreamNonEmptyError) {
   // the last one may have the correct information even if the failure
   // doesn't produce an update, as the timer update may have triggered at the
   // same time.
-  EXPECT_CALL(*(observer_.get()),
-              CurrentUpdateStatus(strlen(kTestData1) + strlen(kTestData2),
+  EXPECT_CALL(*(download_manager_.get()),
+              CurrentUpdateStatus(kDummyDownloadId + 0,
+                                  strlen(kTestData1) + strlen(kTestData2),
                                   _, _));
 
   sink_callback_.Run();
@@ -573,7 +590,7 @@ TEST_F(DownloadFileTest, StreamNonEmptyError) {
 }
 
 // Send some data, wait 3/4s of a second, run the message loop, and
-// confirm the values the observer received are correct.
+// confirm the values the DownloadManager received are correct.
 TEST_F(DownloadFileTest, ConfirmUpdate) {
   CreateDownloadFile(0, true);