Bug fix for 10876, file extension handling refactoring

chrome/browser/download/download_manager.cc

Index: chrome/browser/download/download_manager.cc
===================================================================
--- chrome/browser/download/download_manager.cc	(revision 30182)
+++ chrome/browser/download/download_manager.cc	(working copy)
@@ -526,8 +526,8 @@
   std::vector<std::wstring> extensions;
   SplitString(extensions_to_open, L':', &extensions);
   for (size_t i = 0; i < extensions.size(); ++i) {
-    if (!extensions[i].empty() && !IsExecutable(
-        FilePath::FromWStringHack(extensions[i]).value()))
+    if (!extensions[i].empty() && !IsExecutableFile(
+        FilePath::FromWStringHack(extensions[i])))
       auto_open_.insert(FilePath::FromWStringHack(extensions[i]).value());
   }
 
@@ -857,19 +857,13 @@
   if (it != dangerous_finished_.end())
     dangerous_finished_.erase(it);
 
-  // Open the download if the user or user prefs indicate it should be.
-  FilePath::StringType extension = download->full_path().Extension();
-  // Drop the leading period. (The auto-open list is period-less.)
-  if (extension.size() > 0)
-    extension = extension.substr(1);
-
   // Handle chrome extensions explicitly and skip the shell execute.
   if (IsExtensionInstall(download)) {
     OpenChromeExtension(download->full_path(), download->url(),
                         download->referrer_url());
     download->set_auto_opened(true);
   } else if (download->open_when_complete() ||
-             ShouldOpenFileExtension(extension)) {
+             ShouldOpenFileBasedOnExtension(download->full_path())) {
     OpenDownloadInShell(download, NULL);
     download->set_auto_opened(true);
   }
@@ -878,6 +872,7 @@
   // state to complete but did not notify).
   download->UpdateObservers();
 }
+
 // Called on the file thread.  Renames the downloaded file to its original name.
 void DownloadManager::ProceedWithFinishedDangerousDownload(
     int64 download_handle,
@@ -1004,11 +999,7 @@
 
 bool DownloadManager::IsDangerous(const FilePath& file_name) {
   // TODO(jcampan): Improve me.
-  FilePath::StringType extension = file_name.Extension();
-  // Drop the leading period.
-  if (extension.size() > 0)
-    extension = extension.substr(1);
-  return IsExecutable(extension);
+  return IsExecutableFile(file_name);
 }
 
 void DownloadManager::RenameDownload(DownloadItem* download,
@@ -1155,7 +1146,7 @@
     return;
   }
 
-  if (IsExecutable(extension) && !IsExecutableMimeType(mime_type)) {
+  if (IsExecutableExtension(extension) && !IsExecutableMimeType(mime_type)) {
     // We want to be careful about executable extensions.  The worry here is
     // that a trusted web site could be tricked into dropping an executable file
     // on the user's filesystem.
@@ -1183,7 +1174,7 @@
     if (net::GetPreferredExtensionForMimeType(mime_type, &append_extension)) {
       if (append_extension != FILE_PATH_LITERAL("txt") &&
           append_extension != extension &&
-          !IsExecutable(append_extension) &&
+          !IsExecutableExtension(append_extension) &&
           !(append_extension == FILE_PATH_LITERAL("gz") &&
             extension == FILE_PATH_LITERAL("tgz")) &&
           (append_extension != FILE_PATH_LITERAL("tar") ||
@@ -1282,21 +1273,32 @@
 #endif
 }
 
-void DownloadManager::OpenFilesOfExtension(
-    const FilePath::StringType& extension, bool open) {
-  if (open && !IsExecutable(extension))
+void DownloadManager::OpenFilesBasedOnExtension(
+    const FilePath& path, bool open) {
+  FilePath::StringType extension = path.Extension();
+  if (extension.empty())
+    return;
+  DCHECK(extension[0] == FilePath::kExtensionSeparator);
+  extension.erase(0, 1);
+  if (open && !IsExecutableExtension(extension))
     auto_open_.insert(extension);
   else
     auto_open_.erase(extension);
   SaveAutoOpens();
 }
 
-bool DownloadManager::ShouldOpenFileExtension(
-    const FilePath::StringType& extension) {
+bool DownloadManager::ShouldOpenFileBasedOnExtension(
+    const FilePath& path) const {
   // Special-case Chrome extensions as always-open.
-  if (!IsExecutable(extension) &&
-      (auto_open_.find(extension) != auto_open_.end() ||
-       Extension::IsExtension(FilePath(extension))))
+  FilePath::StringType extension = path.Extension();
+  if (extension.empty())
+    return false;
+  if (IsExecutableExtension(extension))
+    return false;
+  DCHECK(extension[0] == FilePath::kExtensionSeparator);
+  extension.erase(0, 1);
+  if (auto_open_.find(extension) != auto_open_.end() ||
+      Extension::IsExtension(path))
     return true;
   return false;
 }
@@ -1333,7 +1335,14 @@
   return net::MatchesMimeType("application/*", mime_type);
 }
 
-bool DownloadManager::IsExecutable(const FilePath::StringType& extension) {
+bool DownloadManager::IsExecutableFile(const FilePath& path) const {
+  return IsExecutableExtension(path.Extension());
+}
+
+bool DownloadManager::IsExecutableExtension(
+    const FilePath::StringType& extension) const {
+  if (extension.empty())
+    return false;
   if (!IsStringASCII(extension))
     return false;
 #if defined(OS_WIN)
@@ -1343,6 +1352,10 @@
 #endif
   StringToLowerASCII(&ascii_extension);
 
+  // Strip out leading dot if it's still there
+  if (ascii_extension[0] == FilePath::kExtensionSeparator)
+    ascii_extension.erase(0, 1);
+
   return exe_types_.find(ascii_extension) != exe_types_.end();
 }
 
@@ -1359,7 +1372,7 @@
   PrefService* prefs = profile_->GetPrefs();
   if (prefs) {
     FilePath::StringType extensions;
-    for (std::set<FilePath::StringType>::iterator it = auto_open_.begin();
+    for (AutoOpenSet::iterator it = auto_open_.begin();
          it != auto_open_.end(); ++it) {
       extensions += *it + FILE_PATH_LITERAL(":");
     }