Fixes Issue 34722: Meaningless value in 'Save as type' combobox of 'Save page as...' dialog

Adds extension base don MIME type of the page.
Adds capability of the complete save of the "application/xhtml+xml" pages
BUG=34722
TEST=In the bug + try pages with other MIME types, such as "text/xml", "text/plain" or "text/css"
Review URL: http://codereview.chromium.org/650176

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@39804 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 133 insertions, 8 deletions

diff --git a/chrome/browser/download/save_package.cc b/chrome/browser/download/save_package.cc
index 36b090f..a124b3e 100644
--- a/chrome/browser/download/save_package.cc
+++ b/chrome/browser/download/save_package.cc
@@ -1024,7 +1024,8 @@ void SavePackage::SetShouldPromptUser(bool should_prompt) {
 
 // static.
 FilePath SavePackage::GetSuggestedNameForSaveAs(const FilePath& name,
-                                                bool can_save_as_complete) {
+    bool can_save_as_complete,
+    const FilePath::StringType& contents_mime_type) {
   // If the name is a URL, try to use the last path component or if there is
   // none, the domain as the file name.
   FilePath name_with_proper_ext = name;
@@ -1046,6 +1047,9 @@ FilePath SavePackage::GetSuggestedNameForSaveAs(const FilePath& name,
   }
 
   // Ask user for getting final saving name.
+  name_with_proper_ext = EnsureMimeExtension(name_with_proper_ext,
+                                             contents_mime_type);
+  // Adjust extension for complete types.
   if (can_save_as_complete)
     name_with_proper_ext = EnsureHtmlExtension(name_with_proper_ext);
 
@@ -1067,6 +1071,45 @@ FilePath SavePackage::EnsureHtmlExtension(const FilePath& name) {
   return name;
 }
 
+FilePath SavePackage::EnsureMimeExtension(const FilePath& name,
+    const FilePath::StringType& contents_mime_type) {
+  // Start extension at 1 to skip over period if non-empty.
+  FilePath::StringType ext = name.Extension().length() ?
+      name.Extension().substr(1) : name.Extension();
+  FilePath::StringType suggested_extension =
+      ExtensionForMimeType(contents_mime_type);
+  std::string mime_type;
+  if (!suggested_extension.empty() &&
+      (!net::GetMimeTypeFromExtension(ext, &mime_type) ||
+      !IsSavableContents(mime_type))) {
+    // Extension is absent or needs to be updated.
+    return FilePath(name.value() + FILE_PATH_LITERAL(".") +
+                    suggested_extension);
+  }
+  return name;
+}
+
+const FilePath::CharType *SavePackage::ExtensionForMimeType(
+    const FilePath::StringType& contents_mime_type) {
+  static const struct {
+    const FilePath::CharType *mime_type;
+    const FilePath::CharType *suggested_extension;
+  } extensions[] = {
+    { FILE_PATH_LITERAL("text/html"), kDefaultHtmlExtension },
+    { FILE_PATH_LITERAL("text/xml"), FILE_PATH_LITERAL("xml") },
+    { FILE_PATH_LITERAL("application/xhtml+xml"), FILE_PATH_LITERAL("xhtml") },
+    { FILE_PATH_LITERAL("text/plain"), FILE_PATH_LITERAL("txt") },
+    { FILE_PATH_LITERAL("text/css"), FILE_PATH_LITERAL("css") },
+  };
+  for (uint32 i = 0; i < ARRAYSIZE_UNSAFE(extensions); ++i) {
+    if (contents_mime_type == extensions[i].mime_type)
+      return extensions[i].suggested_extension;
+  }
+  return FILE_PATH_LITERAL("");
+}
+
+
+
 // static.
 // Check whether the preference has the preferred directory for saving file. If
 // not, initialize it with default directory.
@@ -1119,19 +1162,40 @@ void SavePackage::ContinueGetSaveInfo(FilePath save_dir) {
 
   FilePath title =
       FilePath::FromWStringHack(UTF16ToWideHack(tab_contents_->GetTitle()));
+
+#if defined(OS_POSIX)
+  FilePath::StringType mime_type(save_params->current_tab_mime_type);
+#elif defined(OS_WIN)
+  FilePath::StringType mime_type(
+      UTF8ToWide(save_params->current_tab_mime_type));
+#endif  // OS_WIN
+
   FilePath suggested_path =
-      save_dir.Append(GetSuggestedNameForSaveAs(title, can_save_as_complete));
+      save_dir.Append(GetSuggestedNameForSaveAs(title, can_save_as_complete,
+      mime_type));
 
   // If the contents can not be saved as complete-HTML, do not show the
   // file filters.
   if (can_save_as_complete) {
+    bool add_extra_extension = false;
+    FilePath::StringType extra_extension;
+    if (!suggested_path.Extension().empty() &&
+        suggested_path.Extension().compare(FILE_PATH_LITERAL("htm")) &&
+        suggested_path.Extension().compare(FILE_PATH_LITERAL("html"))) {
+      add_extra_extension = true;
+      extra_extension = suggested_path.Extension().substr(1);
+    }
     file_type_info.extensions.resize(2);
     file_type_info.extensions[0].push_back(FILE_PATH_LITERAL("htm"));
     file_type_info.extensions[0].push_back(FILE_PATH_LITERAL("html"));
+    if (add_extra_extension)
+      file_type_info.extensions[0].push_back(extra_extension);
     file_type_info.extension_description_overrides.push_back(
         WideToUTF16(l10n_util::GetString(IDS_SAVE_PAGE_DESC_HTML_ONLY)));
     file_type_info.extensions[1].push_back(FILE_PATH_LITERAL("htm"));
     file_type_info.extensions[1].push_back(FILE_PATH_LITERAL("html"));
+    if (add_extra_extension)
+      file_type_info.extensions[1].push_back(extra_extension);
     file_type_info.extension_description_overrides.push_back(
         WideToUTF16(l10n_util::GetString(IDS_SAVE_PAGE_DESC_COMPLETE)));
     file_type_info.include_all_files = false;
@@ -1228,7 +1292,8 @@ bool SavePackage::IsSavableContents(const std::string& contents_mime_type) {
 
 // Static
 bool SavePackage::CanSaveAsComplete(const std::string& contents_mime_type) {
-  return contents_mime_type == "text/html";
+  return contents_mime_type == "text/html" ||
+         contents_mime_type == "application/xhtml+xml";
 }
 
 // Static
diff --git a/chrome/browser/download/save_package.h b/chrome/browser/download/save_package.h
index 4df7174..c41014b 100644
--- a/chrome/browser/download/save_package.h
+++ b/chrome/browser/download/save_package.h
@@ -251,12 +251,23 @@ class SavePackage : public base::RefCountedThreadSafe<SavePackage>,
   // Helper function for preparing suggested name for the SaveAs Dialog. The
   // suggested name is determined by the web document's title.
   static FilePath GetSuggestedNameForSaveAs(const FilePath& name,
-                                            bool can_save_as_complete);
+      bool can_save_as_complete,
+      const FilePath::StringType& contents_mime_type);
 
-  // Ensure that the file name has a proper extension for HTML by adding ".htm"
+  // Ensures that the file name has a proper extension for HTML by adding ".htm"
   // if necessary.
   static FilePath EnsureHtmlExtension(const FilePath& name);
 
+  // Ensures that the file name has a proper extension for supported formats
+  // if necessary.
+  static FilePath EnsureMimeExtension(const FilePath& name,
+      const FilePath::StringType& contents_mime_type);
+
+  // Returns extension for supported MIME types (for example, for "text/plain"
+  // it returns "txt").
+  static const FilePath::CharType* ExtensionForMimeType(
+      const FilePath::StringType& contents_mime_type);
+
   typedef std::queue<SaveItem*> SaveItemQueue;
   // A queue for items we are about to start saving.
   SaveItemQueue waiting_item_queue_;
diff --git a/chrome/browser/download/save_package_unittest.cc b/chrome/browser/download/save_package_unittest.cc
index bc2ea7c..c082797 100644
--- a/chrome/browser/download/save_package_unittest.cc
+++ b/chrome/browser/download/save_package_unittest.cc
@@ -92,9 +92,16 @@ class SavePackageTest : public testing::Test {
     return SavePackage::EnsureHtmlExtension(name);
   }
 
+  FilePath EnsureMimeExtension(const FilePath& name,
+                               const FilePath::StringType& content_mime_type) {
+    return SavePackage::EnsureMimeExtension(name, content_mime_type);
+  }
+
   FilePath GetSuggestedNameForSaveAs(const FilePath& title,
-                                     bool ensure_html_extension) {
-    return SavePackage::GetSuggestedNameForSaveAs(title, ensure_html_extension);
+      bool ensure_html_extension,
+      const FilePath::StringType& contents_mime_type) {
+    return SavePackage::GetSuggestedNameForSaveAs(title, ensure_html_extension,
+                                                  contents_mime_type);
   }
 
  private:
@@ -199,6 +206,8 @@ static const struct {
   // Extension is preserved if it is already proper for HTML.
   {FPL("filename.html"), FPL("filename.html")},
   {FPL("filename.HTML"), FPL("filename.HTML")},
+  {FPL("filename.XHTML"), FPL("filename.XHTML")},
+  {FPL("filename.xhtml"), FPL("filename.xhtml")},
   {FPL("filename.htm"), FPL("filename.htm")},
   // ".htm" is added if the extension is improper for HTML.
   {FPL("hello.world"), FPL("hello.world") FPL_HTML_EXTENSION},
@@ -219,6 +228,45 @@ TEST_F(SavePackageTest, TestEnsureHtmlExtension) {
   }
 }
 
+TEST_F(SavePackageTest, TestEnsureMimeExtension) {
+  static const struct {
+    const FilePath::CharType* page_title;
+    const FilePath::CharType* expected_name;
+    const FilePath::CharType* contents_mime_type;
+  } kExtensionTests[] = {
+    { FPL("filename.html"), FPL("filename.html"), FPL("text/html") },
+    { FPL("filename.htm"), FPL("filename.htm"), FPL("text/html") },
+    { FPL("filename.xhtml"), FPL("filename.xhtml"), FPL("text/html") },
+#if defined(OS_WIN)
+    { FPL("filename"), FPL("filename.htm"), FPL("text/html") },
+#else  // defined(OS_WIN)
+    { FPL("filename"), FPL("filename.html"), FPL("text/html") },
+#endif  // defined(OS_WIN)
+    { FPL("filename.html"), FPL("filename.html"), FPL("text/xml") },
+    { FPL("filename.xml"), FPL("filename.xml"), FPL("text/xml") },
+    { FPL("filename"), FPL("filename.xml"), FPL("text/xml") },
+    { FPL("filename.xhtml"), FPL("filename.xhtml"),
+      FPL("application/xhtml+xml") },
+    { FPL("filename.html"), FPL("filename.html"),
+      FPL("application/xhtml+xml") },
+    { FPL("filename"), FPL("filename.xhtml"), FPL("application/xhtml+xml") },
+    { FPL("filename.txt"), FPL("filename.txt"), FPL("text/plain") },
+    { FPL("filename"), FPL("filename.txt"), FPL("text/plain") },
+    { FPL("filename.css"), FPL("filename.css"), FPL("text/css") },
+    { FPL("filename"), FPL("filename.css"), FPL("text/css") },
+    { FPL("filename.abc"), FPL("filename.abc"), FPL("unknown/unknown") },
+    { FPL("filename"), FPL("filename"), FPL("unknown/unknown") },
+  };
+  for (uint32 i = 0; i < ARRAYSIZE_UNSAFE(kExtensionTests); ++i) {
+    FilePath original = FilePath(kExtensionTests[i].page_title);
+    FilePath expected = FilePath(kExtensionTests[i].expected_name);
+    FilePath::StringType mime_type(kExtensionTests[i].contents_mime_type);
+    FilePath actual = EnsureMimeExtension(original, mime_type);
+    EXPECT_EQ(expected.value(), actual.value()) << "Failed for page title: " <<
+        kExtensionTests[i].page_title << " MIME:" << mime_type;
+  }
+}
+
 // Test that the suggested names generated by SavePackage are reasonable:
 // If the name is a URL, retrieve only the path component since the path name
 // generation code will turn the entire URL into the file name leading to bad
@@ -243,7 +291,8 @@ TEST_F(SavePackageTest, TestSuggestedSaveNames) {
     FilePath title = FilePath(kSuggestedSaveNames[i].page_title);
     FilePath save_name =
         GetSuggestedNameForSaveAs(title,
-                                  kSuggestedSaveNames[i].ensure_html_extension);
+                                  kSuggestedSaveNames[i].ensure_html_extension,
+                                  FilePath::StringType());
     EXPECT_EQ(save_name.value(), kSuggestedSaveNames[i].expected_name);
   }
 }