Replace the DownloadFileManager with direct ownership of DownloadFileImpl

content/browser/download/download_browsertest.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.
 
 // This file contains download browser tests that are known to be runnable
 // in a pure content context.  Over time tests should be migrated here.
 
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/scoped_temp_dir.h"
 #include "content/browser/download/download_file_factory.h"
 #include "content/browser/download/download_file_impl.h"
-#include "content/browser/download/download_file_manager.h"
 #include "content/browser/download/download_item_impl.h"
 #include "content/browser/download/download_manager_impl.h"
 #include "content/browser/power_save_blocker.h"
-#include "content/browser/renderer_host/resource_dispatcher_host_impl.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/public/test/download_test_observer.h"
 #include "content/public/test/test_utils.h"
 #include "content/shell/shell.h"
 #include "content/shell/shell_browser_context.h"
 #include "content/shell/shell_download_manager_delegate.h"
 #include "content/test/content_browser_test.h"
 #include "content/test/content_browser_test_utils.h"
 #include "content/test/net/url_request_mock_http_job.h"
 #include "content/test/net/url_request_slow_download_job.h"
 #include "googleurl/src/gurl.h"
 
 namespace content {
 
 namespace {
 
 class DownloadFileWithDelayFactory;
 
 static DownloadManagerImpl* DownloadManagerForShell(Shell* shell) {
   // We're in a content_browsertest; we know that the DownloadManager
   // is a DownloadManagerImpl.
   return static_cast<DownloadManagerImpl*>(
       BrowserContext::GetDownloadManager(
           shell->web_contents()->GetBrowserContext()));
 }
 
 class DownloadFileWithDelay : public DownloadFileImpl {
  public:
   DownloadFileWithDelay(
-      const DownloadCreateInfo* info,
-      scoped_ptr<content::ByteStreamReader> stream,
-      DownloadRequestHandleInterface* request_handle,
-      scoped_refptr<content::DownloadManager> download_manager,
-      bool calculate_hash,
-      scoped_ptr<content::PowerSaveBlocker> power_save_blocker,
-      const net::BoundNetLog& bound_net_log,
-      // |owner| is required to outlive the DownloadFileWithDelay.
-      DownloadFileWithDelayFactory* owner);
+    const content::DownloadSaveInfo& save_info,
+    const FilePath& default_download_directory,
+    const GURL& url,
+    const GURL& referrer_url,
+    int64 received_bytes,
+    bool calculate_hash,
+    scoped_ptr<content::ByteStreamReader> stream,
+    const net::BoundNetLog& bound_net_log,
+    scoped_ptr<content::PowerSaveBlocker> power_save_blocker,
+    base::WeakPtr<content::DownloadDestinationObserver> observer,
+    base::WeakPtr<DownloadFileWithDelayFactory> owner);
 
   virtual ~DownloadFileWithDelay();
 
   // Wraps DownloadFileImpl::Rename and intercepts the return callback,
   // storing it in the factory that produced this object for later
   // retrieval.
   virtual void Rename(const FilePath& full_path,
                       bool overwrite_existing_file,
                       const RenameCompletionCallback& callback) OVERRIDE;
 
   // Wraps DownloadFileImpl::Detach and intercepts the return callback,
   // storing it in the factory that produced this object for later
   // retrieval.
   virtual void Detach(base::Closure callback) OVERRIDE;
 
  private:
   static void RenameCallbackWrapper(
       DownloadFileWithDelayFactory* factory,
       const RenameCompletionCallback& original_callback,
       content::DownloadInterruptReason reason,
       const FilePath& path);
 
   static void DetachCallbackWrapper(
       DownloadFileWithDelayFactory* factory,
       const base::Closure& original_callback);
 
-  // May only be used on the UI thread.
-  DownloadFileWithDelayFactory* owner_;
+  // This variable may only be read on the FILE thread, and may only be
+  // indirected through (e.g. methods on DownloadFileWithDelayFactory called)
+  // on the UI thread.  This is because after construction,
+  // DownloadFileWithDelay lives on the file thread, but
+  // DownloadFileWithDelayFactory is purely a UI thread object.
+  base::WeakPtr<DownloadFileWithDelayFactory> owner_;
 
   DISALLOW_COPY_AND_ASSIGN(DownloadFileWithDelay);
 };
 
+// All routines on this class must be called on the UI thread.
 class DownloadFileWithDelayFactory : public DownloadFileFactory {
  public:
   DownloadFileWithDelayFactory();
   virtual ~DownloadFileWithDelayFactory();
 
   // DownloadFileFactory interface.
-  virtual content::DownloadFile* CreateFile(
-      DownloadCreateInfo* info,
+  virtual DownloadFile* CreateFile(
+      const content::DownloadSaveInfo& save_info,
+      const FilePath& default_download_directory,
+      GURL url,
+      GURL referrer_url,
+      int64 received_bytes,
+      bool calculate_hash,
       scoped_ptr<content::ByteStreamReader> stream,
-      DownloadManager* download_manager,
-      bool calculate_hash,
-      const net::BoundNetLog& bound_net_log) OVERRIDE;
+      const net::BoundNetLog& bound_net_log,
+      base::WeakPtr<content::DownloadDestinationObserver> observer) OVERRIDE;
 
-  // Must all be called on the UI thread.
   void AddRenameCallback(base::Closure callback);
   void AddDetachCallback(base::Closure callback);
   void GetAllRenameCallbacks(std::vector<base::Closure>* results);
   void GetAllDetachCallbacks(std::vector<base::Closure>* results);
 
   // Do not return until either GetAllRenameCallbacks() or
   // GetAllDetachCallbacks() will return a non-empty list.
   void WaitForSomeCallback();
 
  private:
+  base::WeakPtrFactory<DownloadFileWithDelayFactory> weak_ptr_factory_;
   std::vector<base::Closure> rename_callbacks_;
   std::vector<base::Closure> detach_callbacks_;
   bool waiting_;
 
   DISALLOW_COPY_AND_ASSIGN(DownloadFileWithDelayFactory);
 };
 
 DownloadFileWithDelay::DownloadFileWithDelay(
-      const DownloadCreateInfo* info,
-      scoped_ptr<content::ByteStreamReader> stream,
-      DownloadRequestHandleInterface* request_handle,
-      scoped_refptr<content::DownloadManager> download_manager,
-      bool calculate_hash,
-      scoped_ptr<content::PowerSaveBlocker> power_save_blocker,
-      const net::BoundNetLog& bound_net_log,
-      DownloadFileWithDelayFactory* owner)
-    : DownloadFileImpl(info, stream.Pass(), request_handle, download_manager,
-                       calculate_hash, power_save_blocker.Pass(),
-                       bound_net_log),
+    const content::DownloadSaveInfo& save_info,
+    const FilePath& default_download_directory,
+    const GURL& url,
+    const GURL& referrer_url,
+    int64 received_bytes,
+    bool calculate_hash,
+    scoped_ptr<content::ByteStreamReader> stream,
+    const net::BoundNetLog& bound_net_log,
+    scoped_ptr<content::PowerSaveBlocker> power_save_blocker,
+    base::WeakPtr<content::DownloadDestinationObserver> observer,
+    base::WeakPtr<DownloadFileWithDelayFactory> owner)
+    : DownloadFileImpl(
+        save_info, default_download_directory, url, referrer_url,
+        received_bytes, calculate_hash, stream.Pass(), bound_net_log,
+        power_save_blocker.Pass(), observer),
       owner_(owner) {}
 
 DownloadFileWithDelay::~DownloadFileWithDelay() {}
 
 void DownloadFileWithDelay::Rename(const FilePath& full_path,
                       bool overwrite_existing_file,
                       const RenameCompletionCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   DownloadFileImpl::Rename(
       full_path, overwrite_existing_file,
       base::Bind(DownloadFileWithDelay::RenameCallbackWrapper,
-                 base::Unretained(owner_), callback));
+                 owner_, callback));
 }
 
 void DownloadFileWithDelay::Detach(base::Closure callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   DownloadFileImpl::Detach(
       base::Bind(DownloadFileWithDelay::DetachCallbackWrapper,
-                 base::Unretained(owner_), callback));
+                 owner_, callback));
 }
 
 // static
 void DownloadFileWithDelay::RenameCallbackWrapper(
     DownloadFileWithDelayFactory* factory,
     const RenameCompletionCallback& original_callback,
     content::DownloadInterruptReason reason,
     const FilePath& path) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   factory->AddRenameCallback(base::Bind(original_callback, reason, path));
 }
 
 // static
 void DownloadFileWithDelay::DetachCallbackWrapper(
     DownloadFileWithDelayFactory* factory,
     const base::Closure& original_callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   factory->AddDetachCallback(original_callback);
 }
 
 DownloadFileWithDelayFactory::DownloadFileWithDelayFactory()
-    : waiting_(false) {}
+    : weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)),
+      waiting_(false) {}
 DownloadFileWithDelayFactory::~DownloadFileWithDelayFactory() {}
 
 DownloadFile* DownloadFileWithDelayFactory::CreateFile(
-    DownloadCreateInfo* info,
+    const content::DownloadSaveInfo& save_info,
+    const FilePath& default_download_directory,
+    GURL url,
+    GURL referrer_url,
+    int64 received_bytes,
+    bool calculate_hash,
     scoped_ptr<content::ByteStreamReader> stream,
-    DownloadManager* download_manager,
-    bool calculate_hash,
-    const net::BoundNetLog& bound_net_log) {
-
+    const net::BoundNetLog& bound_net_log,
+    base::WeakPtr<content::DownloadDestinationObserver> observer) {
+  scoped_ptr<content::PowerSaveBlocker> psb(
+      new content::PowerSaveBlocker(
+          content::PowerSaveBlocker::kPowerSaveBlockPreventAppSuspension,
+          "Download in progress"));
   return new DownloadFileWithDelay(
-      info, stream.Pass(), new DownloadRequestHandle(info->request_handle),
-      download_manager, calculate_hash,
-      scoped_ptr<content::PowerSaveBlocker>(
-          new content::PowerSaveBlocker(
-              content::PowerSaveBlocker::kPowerSaveBlockPreventAppSuspension,
-              "Download in progress")).Pass(),
-      bound_net_log, this);
+      save_info, default_download_directory, url, referrer_url,
+      received_bytes, calculate_hash, stream.Pass(), bound_net_log,
+      psb.Pass(), observer, weak_ptr_factory_.GetWeakPtr());
 }
 
 void DownloadFileWithDelayFactory::AddRenameCallback(base::Closure callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   rename_callbacks_.push_back(callback);
   if (waiting_)
     MessageLoopForUI::current()->Quit();
 }
 
 void DownloadFileWithDelayFactory::AddDetachCallback(base::Closure callback) {
@@ skipping 102 matching lines @@
       return false;
 
     // Check the contents.
     EXPECT_EQ(value, file_contents);
     if (memcmp(file_contents.c_str(), value.c_str(), expected_size) != 0)
       return false;
 
     return true;
   }
 
-  DownloadFileManager* GetDownloadFileManager() {
-    ResourceDispatcherHostImpl* rdh(ResourceDispatcherHostImpl::Get());
-    return rdh->download_file_manager();
-  }
-
  private:
   static void EnsureNoPendingDownloadJobsOnIO(bool* result) {
     if (URLRequestSlowDownloadJob::NumberOutstandingRequests())
       *result = false;
     BrowserThread::PostTask(
         BrowserThread::UI, FROM_HERE, MessageLoop::QuitClosure());
   }
 
   // Location of the downloads directory for these tests
   ScopedTempDir downloads_directory_;
@@ skipping 92 matching lines @@
   ASSERT_TRUE(file_util::ContentsEqual(
       file2, GetTestFilePath("download", "download-test.lib")));
 }
 
 // Try to cancel just before we release the download file, by delaying final
 // rename callback.
 IN_PROC_BROWSER_TEST_F(DownloadContentTest, CancelAtFinalRename) {
   // Setup new factory.
   DownloadFileWithDelayFactory* file_factory =
       new DownloadFileWithDelayFactory();
-  GetDownloadFileManager()->SetFileFactoryForTesting(
+  DownloadManagerImpl* download_manager(DownloadManagerForShell(shell()));
+  download_manager->SetDownloadFileFactoryForTesting(
       scoped_ptr<content::DownloadFileFactory>(file_factory).Pass());
 
   // Create a download
   FilePath file(FILE_PATH_LITERAL("download-test.lib"));
   NavigateToURL(shell(), URLRequestMockHTTPJob::GetMockUrl(file));
 
   // Wait until the first (intermediate file) rename and execute the callback.
   file_factory->WaitForSomeCallback();
   std::vector<base::Closure> callbacks;
   file_factory->GetAllDetachCallbacks(&callbacks);
   ASSERT_TRUE(callbacks.empty());
   file_factory->GetAllRenameCallbacks(&callbacks);
   ASSERT_EQ(1u, callbacks.size());
   callbacks[0].Run();
   callbacks.clear();
 
   // Wait until the second (final) rename callback is posted.
   file_factory->WaitForSomeCallback();
   file_factory->GetAllDetachCallbacks(&callbacks);
   ASSERT_TRUE(callbacks.empty());
   file_factory->GetAllRenameCallbacks(&callbacks);
   ASSERT_EQ(1u, callbacks.size());
 
   // Cancel it.
   std::vector<DownloadItem*> items;
-  DownloadManagerForShell(shell())->GetAllDownloads(&items);
+  download_manager->GetAllDownloads(&items);
   ASSERT_EQ(1u, items.size());
   items[0]->Cancel(true);
   RunAllPendingInMessageLoop();
 
   // Check state.
   EXPECT_EQ(DownloadItem::CANCELLED, items[0]->GetState());
 
   // Run final rename callback.
   callbacks[0].Run();
   callbacks.clear();
 
   // Check state.
   EXPECT_EQ(DownloadItem::CANCELLED, items[0]->GetState());
 }
 
 // Try to cancel just after we release the download file, by delaying
 // release.
 IN_PROC_BROWSER_TEST_F(DownloadContentTest, CancelAtRelease) {
   // Setup new factory.
-  // Setup new factory.
   DownloadFileWithDelayFactory* file_factory =
       new DownloadFileWithDelayFactory();
-  GetDownloadFileManager()->SetFileFactoryForTesting(
+  DownloadManagerImpl* download_manager(DownloadManagerForShell(shell()));
+  download_manager->SetDownloadFileFactoryForTesting(
       scoped_ptr<content::DownloadFileFactory>(file_factory).Pass());
 
   // Create a download
   FilePath file(FILE_PATH_LITERAL("download-test.lib"));
   NavigateToURL(shell(), URLRequestMockHTTPJob::GetMockUrl(file));
 
   // Wait until the first (intermediate file) rename and execute the callback.
   file_factory->WaitForSomeCallback();
   std::vector<base::Closure> callbacks;
   file_factory->GetAllDetachCallbacks(&callbacks);
   ASSERT_TRUE(callbacks.empty());
   file_factory->GetAllRenameCallbacks(&callbacks);
   ASSERT_EQ(1u, callbacks.size());
   callbacks[0].Run();
   callbacks.clear();
 
   // Wait until the second (final) rename callback is posted.
   file_factory->WaitForSomeCallback();
   file_factory->GetAllDetachCallbacks(&callbacks);
   ASSERT_TRUE(callbacks.empty());
   file_factory->GetAllRenameCallbacks(&callbacks);
   ASSERT_EQ(1u, callbacks.size());
 
   // Call it.
   callbacks[0].Run();
   callbacks.clear();
 
-  // Confirm download isn't complete yet.
+  // Confirm download still IN_PROGRESS.
   std::vector<DownloadItem*> items;
-  DownloadManagerForShell(shell())->GetAllDownloads(&items);
+  download_manager->GetAllDownloads(&items);
   EXPECT_EQ(DownloadItem::IN_PROGRESS, items[0]->GetState());
 
-  // Cancel the download; confirm cancel fails anyway.
+  // Cancel the download; confirm cancel fails.
   ASSERT_EQ(1u, items.size());
   items[0]->Cancel(true);
   EXPECT_EQ(DownloadItem::IN_PROGRESS, items[0]->GetState());
-  RunAllPendingInMessageLoop();
-  EXPECT_EQ(DownloadItem::IN_PROGRESS, items[0]->GetState());
 
   // Confirm detach callback and run it.
   file_factory->WaitForSomeCallback();
   file_factory->GetAllRenameCallbacks(&callbacks);
   ASSERT_TRUE(callbacks.empty());
   file_factory->GetAllDetachCallbacks(&callbacks);
   ASSERT_EQ(1u, callbacks.size());
   callbacks[0].Run();
   callbacks.clear();
+
+  // *Now* the download should be complete.
   EXPECT_EQ(DownloadItem::COMPLETE, items[0]->GetState());
 }
 
 }  // namespace content