Reduced the lifetime of DownloadCreateInfo.

chrome/browser/download/download_util.h

Index: chrome/browser/download/download_util.h
diff --git a/chrome/browser/download/download_util.h b/chrome/browser/download/download_util.h
index 707dc07ae822e9dd8ea9152bf7837f39166e1303..5bcf60a03121698772c7cf81cbb927e7a688ca8e 100644
--- a/chrome/browser/download/download_util.h
+++ b/chrome/browser/download/download_util.h
@@ -61,8 +61,11 @@ void GenerateExtension(const FilePath& file_name,
                        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,
@@ -271,9 +274,15 @@ 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