Move filename determination to net_util

net/base/net_util.h

Index: net/base/net_util.h
diff --git a/net/base/net_util.h b/net/base/net_util.h
index 344a9903d06795dbaa4167adeca482b8c3ed1937..97a4d8372d1a17b0fb667acf25e414bd29c8bf55 100644
--- a/net/base/net_util.h
+++ b/net/base/net_util.h
@@ -171,11 +171,10 @@ NET_API std::string GetHeaderParamValue(const std::string& header,
 // This function does not do any escaping and callers are responsible for
 // escaping 'unsafe' characters (e.g. (back)slash, colon) as they see fit.
 //
-// TODO(jungshik): revisit this issue. At the moment, the only caller
-// net_util::GetSuggestedFilename and it calls ReplaceIllegalCharacters.  The
-// other caller is a unit test. Need to figure out expose this function only to
+// TODO(jungshik): revisit this issue. At the moment, the only caller is
+// net::GenerateFilename and it calls ReplaceIllegalCharacters.  The other
+// caller is a unit test. Need to figure out expose this function only to
 // net_util_unittest.

[rvargas (doing something else), 2011/07/26 00:10:57]

You can remove this TODO because that's basically the new behavior.

[asanka, 2011/07/28 20:04:38]

Done.

-//
 NET_TEST std::string GetFileNameFromCD(const std::string& header,
                                        const std::string& referrer_charset);
 
@@ -246,21 +245,68 @@ NET_API std::string GetDirectoryListingEntry(const string16& name,
 // unmodified.
 NET_API string16 StripWWW(const string16& text);
 
-// Gets the filename in the following order:
-// 1) the raw Content-Disposition header (as read from the network).
-//    |referrer_charset| is used as one of charsets to interpret a raw 8bit
-//    string in C-D header (after interpreting as UTF-8 fails).
-//    See the comment for GetFilenameFromCD for more details.
-// 2) the suggested name
-// 3) the last path component name or hostname from |url|
-// 4) the given |default_name|
-// 5) the hard-coded name "download", as the last resort
+// Generates a filename using the first successful method from the following (in
+// order):
+//
+// 1) The raw Content-Disposition header in |content_disposition| (as read from
+//    the network.  |referrer_charset| is used as described in the comment for
+//    GetFileNameFromCD().
+// 2) |suggested_name| if specified.  |suggested_name| is assumed to be in
+//    UTF-8.
+// 3) The filename extracted from the |url|.  |referrer_charset| will be used to
+//    interpret the URL if there are non-ascii characters.
+// 4) |default_name|.  If non-empty, |default_name| is assumed to be a filename
+//    and shouldn't contain a path.
+// 5) The hostname portion from the |url|
+// 6) The string "download"
+//
+// For each method, except 4), leading and trailing '.'s will be removed. On
+// OS_WIN, trailing spaces are also removed.

[rvargas (doing something else), 2011/07/26 22:25:41]

nit: I think that we use "Windows" when describing the code.

[asanka, 2011/07/28 20:04:38]

Done.

+//
+// Once the filename is determined, any illegal characters will be replaced by
+// '-'.  If the filename doesn't contain an extension, and a |mime_type| is
+// specified, the preferred extension for the |mime_type| will be appended to
+// the filename.  The resulting filename is then checked against a list of
+// reserved names on OS_WIN.  If the name is reserved, an underscore will be
+// prepended to the filename.
+//
+// Note: |mime_type| should only be specified if this function is called from a
+// thread that allows IO.
 NET_API string16 GetSuggestedFilename(const GURL& url,
                                       const std::string& content_disposition,
                                       const std::string& referrer_charset,
                                       const std::string& suggested_name,
+                                      const std::string& mime_type,
                                       const string16& default_name);
 
+// Similar to GetSuggestedFilename(), but returns a FilePath.

[rvargas (doing something else), 2011/07/26 22:25:41]

It looks like long term we should have just one method.

[asanka, 2011/07/28 20:04:38]

Perhaps the correct solution is to move the encoding switching somewhere else
(FilePath? file_util?) and then have callers use that to create a FilePath from
the string16() that GetSuggestedFilename() returns.

Some call sites (e.g.: in weburlloader_impl.cc) require that the filename be
Unicode.  So, for the moment, we can't do away with a  function that generates a
string16() filename.

All GenerateFileName() does for us now is avoid duplication of the encoding
switching code.

[rvargas (doing something else), 2011/07/29 01:34:53]

Yeah, I saw the WebUrlLoader case... The helper can be part of this file
(although that's a little weird), but having two  public 6-arguments methods
that only differ on the return type feels wrong.

In any case, that can wait for another CL.

+NET_API FilePath GenerateFileName(const GURL& url,
+                                  const std::string& content_disposition,
+                                  const std::string& referrer_charset,
+                                  const std::string& suggested_name,
+                                  const std::string& mime_type,
+                                  const string16& default_name);
+
+// Ensure that the filename and extension is safe to use in the filesystem.

[rvargas (doing something else), 2011/07/26 22:25:41]

nit: Ensures

[asanka, 2011/07/28 20:04:38]

Done.

+//
+// Assumes that |file_path| already contains a valid path or file name.  On
+// Windows if the extension causes the file to have an unsafe interaction with
+// the shell (see net_util::IsShellIntegratedExtension()), then it will be
+// replaced by the string 'download'.  If |file_path| doesn't contain an
+// extension and |mime_type| is non-empty, the preferred extension for
+// |mime_type| will be used as the extension.
+//
+// On Windows, the filename will be checked against a set of reserved names, and
+// if so, an underscore will be prepended to the name.
+//
+// |file_name| can either be just the file name or it can be a full path to a
+// file.
+//
+// Note: |mime_type| should only be non-empty if this function is called from a
+// thread that allows IO.
+NET_API void GenerateSafeFileName(const std::string& mime_type,
+                                  FilePath* file_path);
+
 // Checks the given port against a list of ports which are restricted by
 // default.  Returns true if the port is allowed, false if it is restricted.
 bool IsPortAllowedByDefault(int port);