Reduced the lifetime of DownloadCreateInfo.

chrome/browser/download/download_util.h

 // Copyright (c) 2011 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.
 //
 // Download utilities.
 
 #ifndef CHROME_BROWSER_DOWNLOAD_DOWNLOAD_UTIL_H_
 #define CHROME_BROWSER_DOWNLOAD_DOWNLOAD_UTIL_H_
 #pragma once
 
@@ skipping 43 matching lines @@
 
 // Return true if the |download_path| is dangerous path.
 bool DownloadPathIsDangerous(const FilePath& download_path);
 
 // Create an extension based on the file name and mime type.
 void GenerateExtension(const FilePath& file_name,
                        const std::string& mime_type,
                        FilePath::StringType* generated_extension);
 
 // Create a file name based on the response from the server.
-void GenerateFileNameFromInfo(DownloadCreateInfo* info,
-                              FilePath* generated_name);
+void GenerateFileNameFromRequest(const GURL& url,
+                                 const std::string& content_disposition,
+                                 const std::string& referrer_charset,
+                                 const std::string& mime_type,
+                                 FilePath* generated_name);

[Paweł Hajdan Jr., 2011/05/19 16:18:25]

Instead of having a return parameter and void return type, how about just
converting the return type to FilePath? Should be more natural.

[ahendrickson, 2011/05/19 20:16:49]

I think that's for another CL.

[Paweł Hajdan Jr., 2011/05/20 09:04:42]

Okay, I don't want to increase the already high burden for this CL. Please just
add a TODO.

 
 void GenerateFileNameFromSuggestedName(const GURL& url,
                                        const std::string& suggested_name,
                                        const std::string& mime_type,
                                        FilePath* generated_name);
 
 void GenerateFileName(const GURL& url,
                       const std::string& content_disposition,
                       const std::string& referrer_charset,
                       const std::string& mime_type,
@@ skipping 188 matching lines @@
 
 // Erases all downloaded files with the specified path and name prefix.
 // Used by download UI tests to clean up the download directory.
 void EraseUniqueDownloadFiles(const FilePath& path_prefix);
 
 // Returns a .crdownload intermediate path for the |suggested_path|.
 FilePath GetCrDownloadPath(const FilePath& suggested_path);
 
 // Returns true if this download should show the "dangerous file" warning.
 // Various factors are considered, such as the type of the file, whether a
-// user action initiated the download, and whether the user has explictly
+// user action initiated the download, and whether the user has explicitly
 // marked the file type as "auto open".
-bool IsDangerous(DownloadCreateInfo* info, Profile* profile, bool auto_open);
+bool IsDangerous(const GURL& url,

[Paweł Hajdan Jr., 2011/05/19 16:18:25]

This has a lot of parameters now, and two bool parameters next to each other,
very easy to confuse. Could you somehow reduce the number of parameters to
something more like 4?

[ahendrickson, 2011/05/19 20:16:49]

I used to have a DownloadRequestInfo structure that I passed in (replacing 3
parameters with 1), but I'm trying to minimize the new structures.

There's an additional complexity here to minimize complexity elsewhere.

[Paweł Hajdan Jr., 2011/05/20 09:04:42]

Hmm, okay, I trust this judgment.

[Randy Smith (Not in Mondays), 2011/05/20 19:49:29]

Let me suggest an alternative (Pawel, your opinion is solicited).  This routine
is, as best I can tell, called from only one place:
DownloadManager::CheckDownloadUrlDone.  Given that I'd like to be moving stuff
out of download_util:: anyway, how about pulling it into DownloadManager?   I
believe that means we could collapse the seven args down to two: download, and
state; the other arguments come from the DownloadManager object and could be
folded in.  And we drop the number of functions in download_util by one, which
is a win.

Please keep the todo about tests; we really should write a unit test for this
function (would now go into download_manager_unittest.cc).

[ahendrickson, 2011/05/20 22:12:44]

Done.

[Paweł Hajdan Jr., 2011/05/23 14:04:23]

Sounds good to me.

+                 const GURL& referrer_url,
+                 const FilePath& suggested_path,
+                 bool has_user_gesture,
+                 bool is_extension_install,
+                 Profile* profile,
+                 bool auto_open);
 
 }  // namespace download_util
 
 #endif  // CHROME_BROWSER_DOWNLOAD_DOWNLOAD_UTIL_H_