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

chrome/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.
 
 #include <sstream>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/file_path.h"
 #include "base/file_util.h"
@@ skipping 186 matching lines @@
   }
 
   DISALLOW_COPY_AND_ASSIGN(TestRenderViewContextMenu);
 };
 
 bool WasAutoOpened(DownloadItem* item) {
   return item->GetAutoOpened();
 }
 
 // Called when a download starts. Marks the download as hidden.
 void SetHiddenDownloadCallback(scoped_refptr<DownloadManager> download_manager,

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

This scoped_refptr appears unnecessary, even when considering that it appears to
be intended to keep the DM alive until it creates a DI.

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

Agreed, but I think the code would be uglier with the change:
* DownloadManagerForBrowser returns a scoped_refptr<DownloadManager>, which
would have to be dereferenced.
* We'd need a base::Unretained() in setting the callback.

Given that the change doesn't have anything to do with this CL, I'm inclined to
leave it, at least for now, and maybe until we get rid of the scoped_refptr<> on
the DM>

[awong, 2012/10/15 19:02:33]

drive-by nits: this should probably take "const scoped_refptr<DownloadManager>&"

But if you know the DownloadManager outlives this callback, couldn't you
actually completely remove this function and just do

base::Bind(&download_util::SetShouldShowInSelf);

in the params->set_callback() call below?

[Randy Smith (Not in Mondays), 2012/10/16 17:24:35]

So this may well be due to my lack of understanding of the callback system, but
I thought you could only do partial binding for leading arguments?  The callback
that params->set_callback() is expecting takes a DownloadItem and an error, and
SetShouldShowInShelf() takes a DownloadItem and a boolean.

Having said that, it looks to me as if I can eliminate the download_manager
argument completely, and I've done so.

[awong, 2012/10/16 19:14:33]

Oh, sorry. Your'e right. I misread the code. Yes, Bind can only partially bind
leading arguments.

                                DownloadItem* item,
                                net::Error error) {
   download_util::SetShouldShowInShelf(item, false);
 }
 
 }  // namespace
 
 // While an object of this class exists, it will mock out download
 // opening for all downloads created on the specified download manager.
 class MockDownloadOpeningObserver : public DownloadManager::Observer {
@@ skipping 430 matching lines @@
     ASSERT_TRUE(web_contents) << s.str();
 
     scoped_ptr<content::DownloadTestObserver> observer(
         new content::DownloadTestObserverTerminal(
             download_manager,
             1,
             content::DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_FAIL));
 
     if (download_info.download_method == DOWNLOAD_DIRECT) {
       // Go directly to download.  Don't wait for navigation.
-      content::DownloadSaveInfo save_info;
+      scoped_ptr<content::DownloadSaveInfo> save_info(
+          new content::DownloadSaveInfo());
       // NOTE: |prompt_for_save_location| may change during the download.
-      save_info.prompt_for_save_location = false;
+      save_info->prompt_for_save_location = false;
 
       scoped_refptr<content::DownloadTestItemCreationObserver>
           creation_observer(new content::DownloadTestItemCreationObserver);
 
       scoped_ptr<DownloadUrlParameters> params(
-          DownloadUrlParameters::FromWebContents(web_contents, url, save_info));
+          DownloadUrlParameters::FromWebContents(
+              web_contents, url, save_info.Pass()));
       params->set_callback(creation_observer->callback());
       DownloadManagerForBrowser(browser())->DownloadUrl(params.Pass());
 
       // Wait until the item is created, or we have determined that it
       // won't be.
       creation_observer->WaitForDownloadItemCreation();
 
       int32 invalid_id = content::DownloadId::Invalid().local();
       EXPECT_EQ(download_info.show_download_item,
                 creation_observer->succeeded());
@@ skipping 1048 matching lines @@
   // DownloadUrl always prompts; return acceptance of whatever it prompts.
   EnableFileChooser(true);
 
   WebContents* web_contents = chrome::GetActiveWebContents(browser());
   ASSERT_TRUE(web_contents);
 
   content::DownloadTestObserver* observer(
       new content::DownloadTestObserverTerminal(
           DownloadManagerForBrowser(browser()), 1,
           content::DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_FAIL));
-  content::DownloadSaveInfo save_info;
-  save_info.prompt_for_save_location = true;
+  scoped_ptr<content::DownloadSaveInfo> save_info(
+      new content::DownloadSaveInfo());
+  save_info->prompt_for_save_location = true;
   scoped_ptr<DownloadUrlParameters> params(
-      DownloadUrlParameters::FromWebContents(web_contents, url, save_info));
+      DownloadUrlParameters::FromWebContents(
+          web_contents, url, save_info.Pass()));
   DownloadManagerForBrowser(browser())->DownloadUrl(params.Pass());
   observer->WaitForFinished();
   EXPECT_EQ(1u, observer->NumDownloadsSeenInState(DownloadItem::COMPLETE));
   CheckDownloadStates(1, DownloadItem::COMPLETE);
   EXPECT_TRUE(DidShowFileChooser());
 
   // Check state.
   EXPECT_EQ(1, browser()->tab_count());
   ASSERT_TRUE(CheckDownload(browser(), file, file));
   EXPECT_TRUE(browser()->window()->IsDownloadShelfVisible());
 }
 
 IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadUrlToPath) {
   FilePath file(FILE_PATH_LITERAL("download-test1.lib"));
   GURL url(URLRequestMockHTTPJob::GetMockUrl(file));
 
   WebContents* web_contents = chrome::GetActiveWebContents(browser());
   ASSERT_TRUE(web_contents);
 
   ScopedTempDir other_directory;
   ASSERT_TRUE(other_directory.CreateUniqueTempDir());
   FilePath target_file_full_path
       = other_directory.path().Append(file.BaseName());
-  content::DownloadSaveInfo save_info;
-  save_info.file_path = target_file_full_path;
+  scoped_ptr<content::DownloadSaveInfo> save_info(
+      new content::DownloadSaveInfo());
+  save_info->file_path = target_file_full_path;
 
   content::DownloadTestObserver* observer(CreateWaiter(browser(), 1));
   scoped_ptr<DownloadUrlParameters> params(
-      DownloadUrlParameters::FromWebContents(web_contents, url, save_info));
+      DownloadUrlParameters::FromWebContents(
+          web_contents, url, save_info.Pass()));
   DownloadManagerForBrowser(browser())->DownloadUrl(params.Pass());
   observer->WaitForFinished();
   EXPECT_EQ(1u, observer->NumDownloadsSeenInState(DownloadItem::COMPLETE));
 
   // Check state.
   EXPECT_EQ(1, browser()->tab_count());
   ASSERT_TRUE(CheckDownloadFullPaths(browser(),
                                      target_file_full_path,
                                      OriginFile(file)));
 
@@ skipping 474 matching lines @@
   FilePath file(FILE_PATH_LITERAL("download-test1.lib"));
   GURL url(URLRequestMockHTTPJob::GetMockUrl(file));
 
   scoped_refptr<DownloadManager> download_manager =
       DownloadManagerForBrowser(browser());
   scoped_ptr<content::DownloadTestObserver> observer(
       new content::DownloadTestObserverTerminal(
           download_manager,
           1,
           content::DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_FAIL));
-  content::DownloadSaveInfo save_info;
-  save_info.prompt_for_save_location = false;
+  scoped_ptr<content::DownloadSaveInfo> save_info(
+      new content::DownloadSaveInfo());
+  save_info->prompt_for_save_location = false;
 
   // Download and set IsHiddenDownload to true.
   WebContents* web_contents = chrome::GetActiveWebContents(browser());
   scoped_ptr<DownloadUrlParameters> params(
-      DownloadUrlParameters::FromWebContents(web_contents, url, save_info));
+      DownloadUrlParameters::FromWebContents(
+          web_contents, url, save_info.Pass()));
   params->set_callback(
       base::Bind(&SetHiddenDownloadCallback, download_manager));
   download_manager->DownloadUrl(params.Pass());
   observer->WaitForFinished();
 
   // Verify that download shelf is not shown.
   EXPECT_FALSE(browser()->window()->IsDownloadShelfVisible());
 }