When picking a filename to save, check whether the user's extension is registered on the system at all, rather than checking whether it has a known MIME type. Many known extensions have no MIME type.

BUG=7499
TEST=Save a text file as "foo.reg" and ensure it doesn't get ".txt" appended.
Review URL: http://codereview.chromium.org/3050031

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

2 files changed, 42 insertions, 93 deletions

diff --git a/chrome/browser/views/shell_dialogs_win.cc b/chrome/browser/views/shell_dialogs_win.cc
index c1b6f5c..17d5b1e 100644
--- a/chrome/browser/views/shell_dialogs_win.cc
+++ b/chrome/browser/views/shell_dialogs_win.cc
@@ -23,7 +23,6 @@
 #include "gfx/font.h"
 #include "grit/app_strings.h"
 #include "grit/generated_resources.h"
-#include "net/base/mime_util.h"
 
 // This function takes the output of a SaveAs dialog: a filename, a filter and
 // the extension originally suggested to the user (shown in the dialog box) and
@@ -43,11 +42,13 @@ std::wstring AppendExtensionIfNeeded(const std::wstring& filename,
 
   // If we wanted a specific extension, but the user's filename deleted it or
   // changed it to something that the system doesn't understand, re-append.
-  std::string selected_mime_type;
-  std::wstring file_extension = file_util::GetFileExtensionFromPath(filename);
+  // Careful: Checking net::GetMimeTypeFromExtension() will only find
+  // extensions with a known MIME type, which many "known" extensions on Windows
+  // don't have.  So we check directly for the "known extension" registry key.
+  std::wstring file_extension(file_util::GetFileExtensionFromPath(filename));
+  std::wstring key(L"." + file_extension);
   if (!(filter_selected.empty() || filter_selected == L"*.*") &&
-      !net::GetMimeTypeFromExtension(
-          file_extension, &selected_mime_type) &&
+      !RegKey(HKEY_CLASSES_ROOT, key.c_str()).Valid() &&
       file_extension != suggested_ext) {
     if (return_value[return_value.length() - 1] != L'.')
       return_value.append(L".");
diff --git a/chrome/browser/views/shell_dialogs_win_unittest.cc b/chrome/browser/views/shell_dialogs_win_unittest.cc
index bbe2525..0c2cad5 100644
--- a/chrome/browser/views/shell_dialogs_win_unittest.cc
+++ b/chrome/browser/views/shell_dialogs_win_unittest.cc
@@ -5,92 +5,40 @@
 #include "chrome/browser/shell_dialogs.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-TEST(AppendExtensionIfNeeded, EndingInPeriod_ExtensionAppended) {
-  const std::wstring filename = L"sample.txt.";
-  const std::wstring filter_selected = L"*.txt";
-  const std::wstring suggested_ext = L"txt";
-
-  const std::wstring actual_filename = AppendExtensionIfNeeded(filename,
-      filter_selected, suggested_ext);
-
-  ASSERT_EQ(L"sample.txt.txt", actual_filename);
-}
-
-TEST(AppendExtensionIfNeeded, UnknownMimeType_ExtensionAppended) {
-  const std::wstring filename = L"sample.unknown-mime-type";
-  const std::wstring filter_selected = L"*.txt";
-  const std::wstring suggested_ext = L"txt";
-
-  const std::wstring actual_filename = AppendExtensionIfNeeded(filename,
-      filter_selected, suggested_ext);
-
-  ASSERT_EQ(L"sample.unknown-mime-type.txt", actual_filename);
-}
-
-TEST(AppendExtensionIfNeeded, RecognizableMimeType_NoExtensionAppended) {
-  const std::wstring filename = L"sample.html";
-  const std::wstring filter_selected = L"*.txt";
-  const std::wstring suggested_ext = L"txt";
-
-  const std::wstring actual_filename = AppendExtensionIfNeeded(filename,
-      filter_selected, suggested_ext);
-
-  ASSERT_EQ(L"sample.html", actual_filename);
-}
-
-TEST(AppendExtensionIfNeeded, OnlyPeriods_ExtensionAppended) {
-  const std::wstring filename = L"...";
-  const std::wstring filter_selected = L"*.txt";
-  const std::wstring suggested_ext = L"txt";
-
-  const std::wstring actual_filename = AppendExtensionIfNeeded(filename,
-      filter_selected, suggested_ext);
-
-  ASSERT_EQ(L"...txt", actual_filename);
-}
-
-TEST(AppendExtensionIfNeeded, EqualToExtension_ExtensionAppended) {
-  const std::wstring filename = L"txt";
-  const std::wstring filter_selected = L"*.txt";
-  const std::wstring suggested_ext = L"txt";
-
-  const std::wstring actual_filename = AppendExtensionIfNeeded(filename,
-      filter_selected, suggested_ext);
-
-  ASSERT_EQ(L"txt.txt", actual_filename);
-}
-
-TEST(AppendExtensionIfNeeded, AllFilesFilter_NoExtensionAppended) {
-  const std::wstring filename = L"sample.unknown-mime-type";
-  const std::wstring filter_selected = L"*.*";
-  const std::wstring suggested_ext;
-
-  const std::wstring actual_filename = AppendExtensionIfNeeded(filename,
-      filter_selected, suggested_ext);
-
-  ASSERT_EQ(L"sample.unknown-mime-type", actual_filename);
-}
-
-TEST(AppendExtensionIfNeeded, StripsDotsForUnknownSelectedMimeType) {
-  const std::wstring filename = L"product";
-  const std::wstring filter_selected = L"*.unknown-extension.";
-  const std::wstring suggested_ext = L"html";
-
-  const std::wstring actual_filename = AppendExtensionIfNeeded(filename,
-      filter_selected, suggested_ext);
-
-  ASSERT_EQ(L"product.html", actual_filename);
-}
-
-TEST(AppendExtensionIfNeeded, EqualToExtension_NoDoubleExtension) {
-  // Make sure we don't add a duplicate extension like .tbl.tbl for
-  // files that the system doesn't have a mime type for.
-  const std::wstring filename = L"product.tbl";
-  const std::wstring filter_selected = L"*.tbl";
-  const std::wstring suggested_ext = L"tbl";
-
-  const std::wstring actual_filename = AppendExtensionIfNeeded(filename,
-      filter_selected, suggested_ext);
-
-  ASSERT_EQ(L"product.tbl", actual_filename);
+TEST(ShellDialogsWin, AppendExtensionIfNeeded) {
+  struct AppendExtensionTestCase {
+    wchar_t* filename;
+    wchar_t* filter_selected;
+    wchar_t* suggested_ext;
+    wchar_t* expected_filename;
+  } test_cases[] = {
+    // Known extensions, with or without associated MIME types, should not get
+    // an extension appended.
+    { L"sample.html",    L"*.txt",     L"txt",     L"sample.html" },
+    { L"sample.reg",     L"*.txt",     L"txt",     L"sample.reg" },
+
+    // An unknown extension, or no extension, should get the default extension
+    // appended.
+    { L"sample.unknown", L"*.txt",     L"txt",     L"sample.unknown.txt" },
+    { L"sample",         L"*.txt",     L"txt",     L"sample.txt" },
+    // ...unless the unknown and default extensions match.
+    { L"sample.unknown", L"*.unknown", L"unknown", L"sample.unknown" },
+
+    // The extension alone should be treated like a filename with no extension.
+    { L"txt",            L"*.txt",     L"txt",     L"txt.txt" },
+
+    // Trailing dots should cause us to append an extension.
+    { L"sample.txt.",    L"*.txt",     L"txt",     L"sample.txt.txt" },
+    { L"...",            L"*.txt",     L"txt",     L"...txt" },
+
+    // If the filter is changed to "All files", we allow any filename.
+    { L"sample.unknown", L"*.*",       L"",        L"sample.unknown" },
+  };
+
+  for (size_t i = 0; i < arraysize(test_cases); ++i) {
+    EXPECT_EQ(std::wstring(test_cases[i].expected_filename),
+              AppendExtensionIfNeeded(test_cases[i].filename,
+                                      test_cases[i].filter_selected,
+                                      test_cases[i].suggested_ext));
+  }
 }