Remove deprecated file_util::GetFilenameWithoutExtensionFromPath(), also convert ElideFilename() to take a FilePath.

Review URL: http://codereview.chromium.org/92060

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

7 files changed, 64 insertions, 62 deletions

diff --git a/base/file_util.cc b/base/file_util.cc
index ab0ae58..65cdf91 100644
--- a/base/file_util.cc
+++ b/base/file_util.cc
@@ -90,12 +90,6 @@ FilePath::StringType GetFileExtensionFromPath(const FilePath& path) {
                               file_name, last_dot+1);
 }
 
-std::wstring GetFilenameWithoutExtensionFromPath(const std::wstring& path) {
-  std::wstring file_name = GetFilenameFromPath(path);
-  std::wstring::size_type last_dot = file_name.rfind(L'.');
-  return file_name.substr(0, last_dot);
-}
-
 void InsertBeforeExtension(FilePath* path, const FilePath::StringType& suffix) {
   FilePath::StringType& value =
       const_cast<FilePath::StringType&>(path->value());
diff --git a/base/file_util.h b/base/file_util.h
index 102dd4b..fb7a739 100644
--- a/base/file_util.h
+++ b/base/file_util.h
@@ -80,9 +80,6 @@ FilePath::StringType GetFileExtensionFromPath(const FilePath& path);
 // Deprecated temporary compatibility function.
 std::wstring GetFileExtensionFromPath(const std::wstring& path);
 
-// Deprecated compatibility function.  Use FilePath::RemoveExtension.
-std::wstring GetFilenameWithoutExtensionFromPath(const std::wstring& path);
-
 // Returns the directory component of a path, without the trailing
 // path separator, or an empty string on error. The function does not
 // check for the existence of the path, so if it is passed a directory
diff --git a/chrome/browser/gtk/download_item_gtk.cc b/chrome/browser/gtk/download_item_gtk.cc
index ca3984e..ff65999 100644
--- a/chrome/browser/gtk/download_item_gtk.cc
+++ b/chrome/browser/gtk/download_item_gtk.cc
@@ -155,7 +155,7 @@ DownloadItemGtk::DownloadItemGtk(BaseDownloadItemModel* download_model,
   // much padding when we set the size request. We need to either use ChromeFont
   // or somehow extend TextElider.
   std::wstring elided_filename = gfx::ElideFilename(
-      download_model_->download()->GetFileName().ToWStringHack(),
+      download_model_->download()->GetFileName(),
       ChromeFont(), kTextWidth);
   gchar* label_markup =
       g_markup_printf_escaped(kLabelColorMarkup, kFilenameColor,
diff --git a/chrome/browser/views/download_item_view.cc b/chrome/browser/views/download_item_view.cc
index fb2c1a7..ffef0a6 100644
--- a/chrome/browser/views/download_item_view.cc
+++ b/chrome/browser/views/download_item_view.cc
@@ -6,7 +6,7 @@
 
 #include <vector>
 
-#include "base/file_util.h"
+#include "base/file_path.h"
 #include "base/string_util.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/download/download_item_model.h"
@@ -266,14 +266,17 @@ DownloadItemView::DownloadItemView(DownloadItem* download,
     discard_button_->set_ignore_minimum_size(true);
     AddChildView(save_button_);
     AddChildView(discard_button_);
-    std::wstring file_name = download->original_name().ToWStringHack();
 
     // Ensure the file name is not too long.
 
     // Extract the file extension (if any).
-    std::wstring extension = file_util::GetFileExtensionFromPath(file_name);
-    std::wstring rootname =
-        file_util::GetFilenameWithoutExtensionFromPath(file_name);
+    FilePath filepath(download->original_name());
+    std::wstring extension = filepath.Extension();
+
+    // Remove leading '.'
+    if (extension.length() > 0)
+      extension = extension.substr(1);
+    std::wstring rootname = filepath.BaseName().RemoveExtension().value();
 
     // Elide giant extensions (this shouldn't currently be hit, but might
     // in future, should we ever notice unsafe giant extensions).
@@ -532,13 +535,12 @@ void DownloadItemView::Paint(ChromeCanvas* canvas) {
   if (!IsDangerousMode()) {
     std::wstring filename;
     if (!disabled_while_opening_) {
-      filename = gfx::ElideFilename(download_->GetFileName().ToWStringHack(),
+      filename = gfx::ElideFilename(download_->GetFileName(),
                                     font_, kTextWidth);
     } else {
-      filename =
-          l10n_util::GetStringF(IDS_DOWNLOAD_STATUS_OPENING,
-                                download_->GetFileName().ToWStringHack());
-      filename = gfx::ElideFilename(filename, font_, kTextWidth);
+      FilePath filepath(l10n_util::GetStringF(IDS_DOWNLOAD_STATUS_OPENING,
+                                download_->GetFileName().ToWStringHack()));
+      filename = gfx::ElideFilename(filepath, font_, kTextWidth);
     }
 
     if (show_status_text_) {
diff --git a/chrome/common/gfx/text_elider.cc b/chrome/common/gfx/text_elider.cc
index 3122d3d..e25f19c 100644
--- a/chrome/common/gfx/text_elider.cc
+++ b/chrome/common/gfx/text_elider.cc
@@ -2,8 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "base/file_util.h"
+#include "base/file_path.h"
 #include "base/string_util.h"
+#include "base/sys_string_conversions.h"
 #include "chrome/common/gfx/chrome_font.h"
 #include "chrome/common/gfx/text_elider.h"
 #include "chrome/common/pref_names.h"
@@ -271,22 +272,23 @@ std::wstring ElideUrl(const GURL& url,
   return ElideText(final_elided_url_string, font, available_pixel_width);
 }
 
-std::wstring ElideFilename(const std::wstring& filename,
+std::wstring ElideFilename(const FilePath& filename,
                            const ChromeFont& font,
                            int available_pixel_width) {
-  int full_width = font.GetStringWidth(filename);
+  int full_width = font.GetStringWidth(filename.ToWStringHack());
   if (full_width <= available_pixel_width)
-    return filename;
+    return filename.ToWStringHack();
 
-  std::wstring extension =
-      file_util::GetFileExtensionFromPath(filename);
+#if defined(OS_WIN)
+  std::wstring extension = filename.Extension();
+#elif defined(OS_POSIX)
+  std::wstring extension = base::SysNativeMBToWide(filename.Extension());
+#endif
   std::wstring rootname =
-      file_util::GetFilenameWithoutExtensionFromPath(filename);
+      filename.BaseName().RemoveExtension().ToWStringHack();
 
   if (rootname.empty() || extension.empty())
-    return ElideText(filename, font, available_pixel_width);
-
-  extension = L"." + extension;
+    return ElideText(filename.ToWStringHack(), font, available_pixel_width);
 
   int ext_width = font.GetStringWidth(extension);
   int root_width = font.GetStringWidth(rootname);
diff --git a/chrome/common/gfx/text_elider.h b/chrome/common/gfx/text_elider.h
index 1aa4334..2f88845 100644
--- a/chrome/common/gfx/text_elider.h
+++ b/chrome/common/gfx/text_elider.h
@@ -12,6 +12,7 @@
 #include "base/string16.h"
 #include "chrome/common/gfx/chrome_font.h"
 
+class FilePath;
 class GURL;
 
 namespace url_parse {
@@ -57,7 +58,7 @@ std::wstring ElideText(const std::wstring& text,
 // Elide a filename to fit a given pixel width, with an emphasis on not hiding
 // the extension unless we have to. If filename contains a path, the path will
 // be removed if filename doesn't fit into available_pixel_width.
-std::wstring ElideFilename(const std::wstring& filename,
+std::wstring ElideFilename(const FilePath& filename,
                            const ChromeFont& font,
                            int available_pixel_width);
 
diff --git a/chrome/common/gfx/text_elider_unittest.cc b/chrome/common/gfx/text_elider_unittest.cc
index 06b9755..c47e0bd 100644
--- a/chrome/common/gfx/text_elider_unittest.cc
+++ b/chrome/common/gfx/text_elider_unittest.cc
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/file_path.h"
 #include "base/string_util.h"
 #include "chrome/common/gfx/chrome_font.h"
 #include "chrome/common/gfx/text_elider.h"
@@ -19,6 +20,11 @@ struct Testcase {
   const std::wstring output;
 };
 
+struct FileTestcase {
+  const FilePath::StringType input;
+  const std::wstring output;
+};
+
 struct WideTestcase {
   const std::wstring input;
   const std::wstring output;
@@ -138,41 +144,41 @@ TEST(TextEliderTest, TestFileURLEliding) {
 
 TEST(TextEliderTest, TestFilenameEliding) {
   const std::wstring kEllipsisStr(kEllipsis);
-// TODO(port): this should probably use FilePath::kPathSeparators[0], but we
-// will change this unit test after porting text elider to string16/FilePath,
-// so it's not worth using FilePath stuff at the moment.
-#if defined(OS_POSIX)
-  const std::wstring kPathSeparator(L"/");
-#elif defined(OS_WIN)
-  const std::wstring kPathSeparator(L"\\");
-#endif
-
-  WideTestcase testcases[] = {
-    {L"", L""},
-    {L".", L"."},
-    {L"filename.exe", L"filename.exe"},
-    {L".longext", L".longext"},
-    {L"pie", L"pie"},
-    {L"c:" + kPathSeparator + L"path" + kPathSeparator + L"filename.pie",
-     L"filename.pie"},
-    {L"c:" + kPathSeparator + L"path" + kPathSeparator + L"longfilename.pie",
-     L"long" + kEllipsisStr + L".pie"},
-    {L"http://path.com/filename.pie", L"filename.pie"},
-    {L"http://path.com/longfilename.pie", L"long" + kEllipsisStr + L".pie"},
-    {L"piesmashingtacularpants", L"pie" + kEllipsisStr},
-    {L".piesmashingtacularpants", L".pie" + kEllipsisStr},
-    {L"cheese.", L"cheese."},
-    {L"file name.longext", L"file" + kEllipsisStr + L".longext"},
-    {L"fil ename.longext", L"fil " + kEllipsisStr + L".longext"},
-    {L"filename.longext", L"file" + kEllipsisStr + L".longext"},
-    {L"filename.middleext.longext",
-     L"filename.mid" + kEllipsisStr + L".longext"}
+  const FilePath::StringType kPathSeparator =
+      FilePath::StringType().append(1, FilePath::kSeparators[0]);
+
+  FileTestcase testcases[] = {
+    {FILE_PATH_LITERAL(""), L""},
+    {FILE_PATH_LITERAL("."), L"."},
+    {FILE_PATH_LITERAL("filename.exe"), L"filename.exe"},
+    {FILE_PATH_LITERAL(".longext"), L".longext"},
+    {FILE_PATH_LITERAL("pie"), L"pie"},
+    {FILE_PATH_LITERAL("c:") + kPathSeparator + FILE_PATH_LITERAL("path") +
+      kPathSeparator + FILE_PATH_LITERAL("filename.pie"),
+      L"filename.pie"},
+    {FILE_PATH_LITERAL("c:") + kPathSeparator + FILE_PATH_LITERAL("path") +
+      kPathSeparator + FILE_PATH_LITERAL("longfilename.pie"),
+      L"long" + kEllipsisStr + L".pie"},
+    {FILE_PATH_LITERAL("http://path.com/filename.pie"), L"filename.pie"},
+    {FILE_PATH_LITERAL("http://path.com/longfilename.pie"),
+      L"long" + kEllipsisStr + L".pie"},
+    {FILE_PATH_LITERAL("piesmashingtacularpants"), L"pie" + kEllipsisStr},
+    {FILE_PATH_LITERAL(".piesmashingtacularpants"), L".pie" + kEllipsisStr},
+    {FILE_PATH_LITERAL("cheese."), L"cheese."},
+    {FILE_PATH_LITERAL("file name.longext"),
+      L"file" + kEllipsisStr + L".longext"},
+    {FILE_PATH_LITERAL("fil ename.longext"),
+      L"fil " + kEllipsisStr + L".longext"},
+    {FILE_PATH_LITERAL("filename.longext"),
+      L"file" + kEllipsisStr + L".longext"},
+    {FILE_PATH_LITERAL("filename.middleext.longext"),
+      L"filename.mid" + kEllipsisStr + L".longext"}
   };
 
   static const ChromeFont font;
   for (size_t i = 0; i < arraysize(testcases); ++i) {
-    const std::wstring filename(testcases[i].input);
-    EXPECT_EQ(testcases[i].output, ElideFilename(filename,
+    FilePath filepath(testcases[i].input);
+    EXPECT_EQ(testcases[i].output, ElideFilename(filepath,
         font,
         font.GetStringWidth(testcases[i].output)));
   }