Move filename determination to net_util

content/browser/download/save_package.cc

 // 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.
 
 #include "content/browser/download/save_package.h"
 
 #include <algorithm>
 
 #include "base/file_path.h"
 #include "base/file_util.h"
@@ skipping 353 matching lines @@
   pure_file_name->clear();
   return false;
 }
 
 // Generate name for saving resource.
 bool SavePackage::GenerateFileName(const std::string& disposition,
                                    const GURL& url,
                                    bool need_html_ext,
                                    FilePath::StringType* generated_name) {
   // TODO(jungshik): Figure out the referrer charset when having one
-  // makes sense and pass it to GetSuggestedFilename.
-  string16 suggested_name =
-      net::GetSuggestedFilename(url, disposition, "", "",
-                                ASCIIToUTF16(kDefaultSaveName));
-
-  // TODO(evan): this code is totally wrong -- we should just generate
-  // Unicode filenames and do all this encoding switching at the end.
-  // However, I'm just shuffling wrong code around, at least not adding
-  // to it.
-#if defined(OS_WIN)
-  FilePath file_path = FilePath(suggested_name);
-#else
-  FilePath file_path = FilePath(
-      base::SysWideToNativeMB(UTF16ToWide(suggested_name)));
-#endif
+  // makes sense and pass it to GenerateFileName.
+  FilePath file_path = net::GenerateFileName(url, disposition, "", "", "",
+                                             ASCIIToUTF16(kDefaultSaveName));
 
   DCHECK(!file_path.empty());
   FilePath::StringType pure_file_name =
       file_path.RemoveExtension().BaseName().value();
   FilePath::StringType file_name_ext = file_path.Extension();
 
   // If it is HTML resource, use ".htm{l,}" as its extension.
   if (need_html_ext) {
     file_name_ext = FILE_PATH_LITERAL(".");
     file_name_ext.append(kDefaultHtmlExtension);
@@ skipping 872 matching lines @@
 
   content::GetContentClient()->browser()->ChooseSavePath(
       AsWeakPtr(), suggested_path, can_save_as_complete);
 }
 
 // Called after the save file dialog box returns.
 void SavePackage::OnPathPicked(const FilePath& final_name,
                                SavePackageType type) {
   // Ensure the filename is safe.
   saved_main_file_path_ = final_name;
-  download_util::GenerateSafeFileName(tab_contents()->contents_mime_type(),
-                                      &saved_main_file_path_);
+  // TODO(asanka): This call is going to block on IO and shouldn't be
+  // made from the UI thread.

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

Could you clarify (for me) what does this mean?. Are we changing the current
behavior (it doesn't look like)? I'm just double checking that this is just
documenting a pre-existing problem.

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

This is documenting a pre-existing problem.  net::GenerateSafeFileName() (as did
download_util::GenerateSafeFileName()) ends up calling
net::GetPreferredExtensionForMimeType().  The lookup logic may go to disk to
look-up the MIME->extension mapping.  See http://crbug.com/61827.

I'll add a bug annotation.

+  net::GenerateSafeFileName(tab_contents()->contents_mime_type(),
+                            &saved_main_file_path_);
 
   saved_main_directory_path_ = saved_main_file_path_.DirName();
 
   PrefService* prefs = tab_contents()->profile()->GetPrefs();
   StringPrefMember save_file_path;
   save_file_path.Init(prefs::kSaveFileDefaultDirectory, prefs, NULL);
 #if defined(OS_POSIX)
   std::string path_string = saved_main_directory_path_.value();
 #elif defined(OS_WIN)
   std::string path_string = WideToUTF8(saved_main_directory_path_.value());
@@ skipping 33 matching lines @@
 bool SavePackage::IsSavableContents(const std::string& contents_mime_type) {
   // WebKit creates Document object when MIME type is application/xhtml+xml,
   // so we also support this MIME type.
   return contents_mime_type == "text/html" ||
          contents_mime_type == "text/xml" ||
          contents_mime_type == "application/xhtml+xml" ||
          contents_mime_type == "text/plain" ||
          contents_mime_type == "text/css" ||
          net::IsSupportedJavascriptMimeType(contents_mime_type.c_str());
 }