Ports TextDatabase and ExpireHistoryTest to use FilePath.

Patch by rohitrao@google.com
http://codereview.chromium.org/40001

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

8 files changed, 81 insertions, 68 deletions

diff --git a/chrome/browser/history/expire_history_backend_unittest.cc b/chrome/browser/history/expire_history_backend_unittest.cc
index 0799dbc..316d988 100644
--- a/chrome/browser/history/expire_history_backend_unittest.cc
+++ b/chrome/browser/history/expire_history_backend_unittest.cc
@@ -24,6 +24,14 @@ using base::Time;
 using base::TimeDelta;
 using base::TimeTicks;
 
+// Filename constants.
+static const FilePath::CharType kTestDir[] = FILE_PATH_LITERAL("ExpireTest");
+static const FilePath::CharType kHistoryFile[] = FILE_PATH_LITERAL("History");
+static const FilePath::CharType kArchivedHistoryFile[] =
+    FILE_PATH_LITERAL("Archived History");
+static const FilePath::CharType kThumbnailFile[] =
+    FILE_PATH_LITERAL("Thumbnails");
+
 // The test must be in the history namespace for the gtest forward declarations
 // to work. It also eliminates a bunch of ugly "history::".
 namespace history {
@@ -67,7 +75,7 @@ class ExpireHistoryTest : public testing::Test,
         bookmark_model_.GetBookmarkBarNode(), 0, std::wstring(), url);
   }
 
-  static bool IsStringInFile(std::wstring& filename, const char* str);
+  static bool IsStringInFile(const FilePath& filename, const char* str);
 
   BookmarkModel bookmark_model_;
 
@@ -91,34 +99,33 @@ class ExpireHistoryTest : public testing::Test,
   NotificationList notifications_;
 
   // Directory for the history files.
-  std::wstring dir_;
+  FilePath dir_;
 
  private:
   void SetUp() {
-    PathService::Get(base::DIR_TEMP, &dir_);
-    file_util::AppendToPath(&dir_, L"ExpireTest");
+    FilePath temp_dir;
+    PathService::Get(base::DIR_TEMP, &temp_dir);
+    dir_ = temp_dir.Append(kTestDir);
     file_util::Delete(dir_, true);
     file_util::CreateDirectory(dir_);
 
-    std::wstring history_name(dir_);
-    file_util::AppendToPath(&history_name, L"History");
+    FilePath history_name = dir_.Append(kHistoryFile);
     main_db_.reset(new HistoryDatabase);
-    if (main_db_->Init(history_name, std::wstring()) != INIT_OK)
+    if (main_db_->Init(history_name.ToWStringHack(), std::wstring()) !=
+        INIT_OK)
       main_db_.reset();
 
-    std::wstring archived_name(dir_);
-    file_util::AppendToPath(&archived_name, L"Archived History");
+    FilePath archived_name = dir_.Append(kArchivedHistoryFile);
     archived_db_.reset(new ArchivedDatabase);
-    if (!archived_db_->Init(archived_name))
+    if (!archived_db_->Init(archived_name.ToWStringHack()))
       archived_db_.reset();
 
-    std::wstring thumb_name(dir_);
-    file_util::AppendToPath(&thumb_name, L"Thumbnails");
+    FilePath thumb_name = dir_.Append(kThumbnailFile);
     thumb_db_.reset(new ThumbnailDatabase);
-    if (thumb_db_->Init(thumb_name, NULL) != INIT_OK)
+    if (thumb_db_->Init(thumb_name.ToWStringHack(), NULL) != INIT_OK)
       thumb_db_.reset();
 
-    text_db_.reset(new TextDatabaseManager(dir_,
+    text_db_.reset(new TextDatabaseManager(dir_.ToWStringHack(),
                                            main_db_.get(), main_db_.get()));
     if (!text_db_->Init(NULL))
       text_db_.reset();
@@ -333,14 +340,6 @@ void ExpireHistoryTest::EnsureURLInfoGone(const URLRow& row) {
   EXPECT_EQ(row.typed_count() > 0, found_typed_changed_notification);
 }
 
-// static
-bool ExpireHistoryTest::IsStringInFile(std::wstring& filename,
-                                       const char* str) {
-  std::string contents;
-  EXPECT_TRUE(file_util::ReadFileToString(filename, &contents));
-  return contents.find(str) != std::string::npos;
-}
-
 TEST_F(ExpireHistoryTest, DeleteFaviconsIfPossible) {
   // Add a favicon record.
   const GURL favicon_url("http://www.google.com/favicon.ico");
@@ -372,6 +371,14 @@ TEST_F(ExpireHistoryTest, DeleteFaviconsIfPossible) {
   EXPECT_TRUE(HasFavIcon(icon_id));
 }
 
+// static
+bool ExpireHistoryTest::IsStringInFile(const FilePath& filename,
+                                       const char* str) {
+  std::string contents;
+  EXPECT_TRUE(file_util::ReadFileToString(filename, &contents));
+  return contents.find(str) != std::string::npos;
+}
+
 // Deletes a URL with a favicon that it is the last referencer of, so that it
 // should also get deleted.
 TEST_F(ExpireHistoryTest, DeleteURLAndFavicon) {
@@ -396,15 +403,15 @@ TEST_F(ExpireHistoryTest, DeleteURLAndFavicon) {
                        visits[0].visit_time);
 
   // Compute the text DB filename.
-  std::wstring fts_filename = dir_;
-  file_util::AppendToPath(&fts_filename,
+  FilePath fts_filename = dir_.Append(
       TextDatabase::IDToFileName(text_db_->TimeToID(visit_times[3])));
 
   // When checking the file, the database must be closed. We then re-initialize
   // it just like the test set-up did.
   text_db_.reset();
   EXPECT_TRUE(IsStringInFile(fts_filename, "goats"));
-  text_db_.reset(new TextDatabaseManager(dir_, main_db_.get(), main_db_.get()));
+  text_db_.reset(new TextDatabaseManager(dir_.ToWStringHack(),
+                                         main_db_.get(), main_db_.get()));
   ASSERT_TRUE(text_db_->Init(NULL));
   expirer_.SetDatabases(main_db_.get(), archived_db_.get(), thumb_db_.get(),
                         text_db_.get());
@@ -416,7 +423,8 @@ TEST_F(ExpireHistoryTest, DeleteURLAndFavicon) {
   // doesn't remove it from the file, we want to be sure we're doing the latter.
   text_db_.reset();
   EXPECT_FALSE(IsStringInFile(fts_filename, "goats"));
-  text_db_.reset(new TextDatabaseManager(dir_, main_db_.get(), main_db_.get()));
+  text_db_.reset(new TextDatabaseManager(dir_.ToWStringHack(),
+                                         main_db_.get(), main_db_.get()));
   ASSERT_TRUE(text_db_->Init(NULL));
   expirer_.SetDatabases(main_db_.get(), archived_db_.get(), thumb_db_.get(),
                         text_db_.get());
@@ -593,7 +601,6 @@ TEST_F(ExpireHistoryTest, FlushRecentURLsStarred) {
   EXPECT_TRUE(HasThumbnail(new_url_row1.id()));
   EXPECT_TRUE(HasFavIcon(new_url_row2.favicon_id()));
   EXPECT_TRUE(HasThumbnail(new_url_row2.id()));
-
 }
 
 TEST_F(ExpireHistoryTest, ArchiveHistoryBeforeUnstarred) {
diff --git a/chrome/browser/history/text_database.cc b/chrome/browser/history/text_database.cc
index 37b15ba..6554afa 100644
--- a/chrome/browser/history/text_database.cc
+++ b/chrome/browser/history/text_database.cc
@@ -48,8 +48,7 @@ const char kTitleColumnIndex[] = "1";
 const char kBodyColumnIndex[] = "2";
 
 // The string prepended to the database identifier to generate the filename.
-const wchar_t kFilePrefix[] = L"History Index ";
-const size_t kFilePrefixLen = arraysize(kFilePrefix) - 1;  // Don't count NULL.
+const FilePath::CharType kFilePrefix[] = FILE_PATH_LITERAL("History Index ");
 
 // We do not allow rollback, but this simple scoper makes it easy to always
 // remember to commit a begun transaction. This protects against some errors
@@ -57,7 +56,7 @@ const size_t kFilePrefixLen = arraysize(kFilePrefix) - 1;  // Don't count NULL.
 // the full protection of a transaction's rollback abilities.
 class ScopedTransactionCommitter {
  public:
-   ScopedTransactionCommitter(TextDatabase* db) : db_(db) {
+  ScopedTransactionCommitter(TextDatabase* db) : db_(db) {
     db_->BeginTransaction();
   }
   ~ScopedTransactionCommitter() {
@@ -70,7 +69,7 @@ class ScopedTransactionCommitter {
 
 }  // namespace
 
-TextDatabase::TextDatabase(const std::wstring& path,
+TextDatabase::TextDatabase(const FilePath& path,
                            DBIdent id,
                            bool allow_create)
     : db_(NULL),
@@ -80,8 +79,7 @@ TextDatabase::TextDatabase(const std::wstring& path,
       allow_create_(allow_create),
       transaction_nesting_(0) {
   // Compute the file name.
-  file_name_ = path_;
-  file_util::AppendToPath(&file_name_, IDToFileName(ident_));
+  file_name_ = path_.Append(IDToFileName(ident_));
 }
 
 TextDatabase::~TextDatabase() {
@@ -97,23 +95,26 @@ TextDatabase::~TextDatabase() {
 }
 
 // static
-const wchar_t* TextDatabase::file_base() {
+const FilePath::CharType* TextDatabase::file_base() {
   return kFilePrefix;
 }
 
 // static
-std::wstring TextDatabase::IDToFileName(DBIdent id) {
+FilePath TextDatabase::IDToFileName(DBIdent id) {
   // Identifiers are intended to be a combination of the year and month, for
   // example, 200801 for January 2008. We convert this to
   // "History Index 2008-01". However, we don't make assumptions about this
   // scheme: the caller should assign IDs as it feels fit with the knowledge
   // that they will apppear on disk in this form.
-  return StringPrintf(L"%ls%d-%02d", file_base(), id / 100, id % 100);
+  FilePath::StringType filename(file_base());
+  StringAppendF(&filename, FILE_PATH_LITERAL("%d-%02d"),
+                id / 100, id % 100);
+  return FilePath(filename);
 }
 
 // static
-TextDatabase::DBIdent TextDatabase::FileNameToID(const std::wstring& file_path){
-  std::wstring file_name = file_util::GetFilenameFromPath(file_path);
+TextDatabase::DBIdent TextDatabase::FileNameToID(const FilePath& file_path) {
+  FilePath::StringType file_name = file_path.BaseName().value();
 
   // We don't actually check the prefix here. Since the file system could
   // be case insensitive in ways we can't predict (NTFS), checking could
@@ -121,14 +122,16 @@ TextDatabase::DBIdent TextDatabase::FileNameToID(const std::wstring& file_path){
   static const size_t kIDStringLength = 7;  // Room for "xxxx-xx".
   if (file_name.length() < kIDStringLength)
     return 0;
-  const std::wstring suffix(&file_name[file_name.length() - kIDStringLength]);
+  const FilePath::StringType suffix(
+      &file_name[file_name.length() - kIDStringLength]);
 
-  if (suffix.length() != kIDStringLength || suffix[4] != L'-') {
+  if (suffix.length() != kIDStringLength ||
+      suffix[4] != FILE_PATH_LITERAL('-')) {
     return 0;
   }
 
-  int year = StringToInt(WideToUTF16Hack(suffix.substr(0, 4)));
-  int month = StringToInt(WideToUTF16Hack(suffix.substr(5, 2)));
+  int year = StringToInt(suffix.substr(0, 4));
+  int month = StringToInt(suffix.substr(5, 2));
 
   return year * 100 + month;
 }
@@ -141,7 +144,7 @@ bool TextDatabase::Init() {
   }
 
   // Attach the database to our index file.
-  if (sqlite3_open(WideToUTF8(file_name_).c_str(), &db_) != SQLITE_OK)
+  if (OpenSqliteDb(file_name_, &db_) != SQLITE_OK)
     return false;
   statement_cache_ = new SqliteStatementCache(db_);
 
diff --git a/chrome/browser/history/text_database.h b/chrome/browser/history/text_database.h
index a07e71a..62e6b6e 100644
--- a/chrome/browser/history/text_database.h
+++ b/chrome/browser/history/text_database.h
@@ -9,6 +9,7 @@
 #include <vector>
 
 #include "base/basictypes.h"
+#include "base/file_path.h"
 #include "chrome/browser/history/history_types.h"
 #include "chrome/browser/meta_table_helper.h"
 #include "chrome/common/sqlite_compiled_statement.h"
@@ -48,7 +49,7 @@ class TextDatabase {
 
   // Note: You must call init which must succeed before using this class.
   //
-  // Computes the mathes for the query, returning results in decreasing order
+  // Computes the matches for the query, returning results in decreasing order
   // of visit time.
   //
   // This function will attach the new database to the given database
@@ -64,7 +65,7 @@ class TextDatabase {
   // |allow_create| indicates if we want to allow creation of the file if it
   // doesn't exist. For files associated with older time periods, we don't want
   // to create them if they don't exist, so this flag would be false.
-  TextDatabase(const std::wstring& path,
+  TextDatabase(const FilePath& path,
                DBIdent id,
                bool allow_create);
   ~TextDatabase();
@@ -83,16 +84,16 @@ class TextDatabase {
 
   // For testing, returns the file name of the database so it can be deleted
   // after the test. This is valid even before Init() is called.
-  const std::wstring& file_name() const { return file_name_; }
+  const FilePath& file_name() const { return file_name_; }
 
   // Returns a NULL-terminated string that is the base of history index files,
   // which is the part before the database identifier. For example
   // "History Index *". This is for finding existing database files.
-  static const wchar_t* file_base();
+  static const FilePath::CharType* file_base();
 
   // Converts a filename on disk (optionally including a path) to a database
   // identifier. If the filename doesn't have the correct format, returns 0.
-  static DBIdent FileNameToID(const std::wstring& file_path);
+  static DBIdent FileNameToID(const FilePath& file_path);
 
   // Changing operations -------------------------------------------------------
 
@@ -137,7 +138,7 @@ class TextDatabase {
 
   // Converts the given database identifier to a filename. This does not include
   // the path, just the file and extension.
-  static std::wstring IDToFileName(DBIdent id);
+  static FilePath IDToFileName(DBIdent id);
 
  private:
   // Ensures that the tables and indices are created. Returns true on success.
@@ -147,12 +148,12 @@ class TextDatabase {
   sqlite3* db_;
   SqliteStatementCache* statement_cache_;
 
-  const std::wstring path_;
+  const FilePath path_;
   const DBIdent ident_;
   const bool allow_create_;
 
   // Full file name of the file on disk, computed in Init().
-  std::wstring file_name_;
+  FilePath file_name_;
 
   // Nesting levels of transactions. Since sqlite only allows one open
   // transaction, we simulate nested transactions by mapping the outermost one
diff --git a/chrome/browser/history/text_database_manager.cc b/chrome/browser/history/text_database_manager.cc
index 2feea81..1e3687f 100644
--- a/chrome/browser/history/text_database_manager.cc
+++ b/chrome/browser/history/text_database_manager.cc
@@ -70,7 +70,7 @@ bool TextDatabaseManager::PageInfo::Expired(TimeTicks now) const {
 TextDatabaseManager::TextDatabaseManager(const std::wstring& dir,
                                          URLDatabase* url_database,
                                          VisitDatabase* visit_database)
-    : dir_(dir),
+    : dir_(FilePath::FromWStringHack(dir)),
       db_(NULL),
       url_database_(url_database),
       visit_database_(visit_database),
@@ -147,12 +147,12 @@ void TextDatabaseManager::InitDBList() {
   present_databases_loaded_ = true;
 
   // Find files on disk matching our pattern so we can quickly test for them.
-  file_util::FileEnumerator enumerator(FilePath::FromWStringHack(dir_), false,
-      file_util::FileEnumerator::FILES,
-      FilePath::FromWStringHack(
-          std::wstring(TextDatabase::file_base()) + L"*").value());
-  std::wstring cur_file;
-  while (!(cur_file = enumerator.Next().ToWStringHack()).empty()) {
+  FilePath::StringType filepattern(TextDatabase::file_base());
+  filepattern.append(FILE_PATH_LITERAL("*"));
+  file_util::FileEnumerator enumerator(
+      dir_, false, file_util::FileEnumerator::FILES, filepattern);
+  FilePath cur_file;
+  while (!(cur_file = enumerator.Next()).empty()) {
     // Convert to the number representing this file.
     TextDatabase::DBIdent id = TextDatabase::FileNameToID(cur_file);
     if (id)  // Will be 0 on error.
@@ -374,8 +374,7 @@ void TextDatabaseManager::DeleteAll() {
   // Now go through and delete all the files.
   for (DBIdentSet::iterator i = present_databases_.begin();
        i != present_databases_.end(); ++i) {
-    std::wstring file_name(dir_);
-    file_util::AppendToPath(&file_name, TextDatabase::IDToFileName(*i));
+    FilePath file_name = dir_.Append(TextDatabase::IDToFileName(*i));
     file_util::Delete(file_name, false);
   }
 }
diff --git a/chrome/browser/history/text_database_manager.h b/chrome/browser/history/text_database_manager.h
index 7dd4150..e1d5d2e 100644
--- a/chrome/browser/history/text_database_manager.h
+++ b/chrome/browser/history/text_database_manager.h
@@ -241,7 +241,7 @@ class TextDatabaseManager {
   void FlushOldChangesForTime(base::TimeTicks now);
 
   // Directory holding our index files.
-  const std::wstring dir_;
+  const FilePath dir_;
 
   // The database connection.
   sqlite3* db_;
diff --git a/chrome/browser/history/text_database_unittest.cc b/chrome/browser/history/text_database_unittest.cc
index 9da2b7c..32c05de 100644
--- a/chrome/browser/history/text_database_unittest.cc
+++ b/chrome/browser/history/text_database_unittest.cc
@@ -124,13 +124,13 @@ class TextDatabaseTest : public PlatformTest {
   }
 
   // Directory containing the databases.
-  std::wstring temp_path_;
+  FilePath temp_path_;
 
   // Name of the main database file.
-  std::wstring file_name_;
+  FilePath file_name_;
   sqlite3* db_;
 
-  std::vector<std::wstring> opened_files_;
+  std::vector<FilePath> opened_files_;
 };
 
 TEST_F(TextDatabaseTest, AttachDetach) {
diff --git a/chrome/common/sqlite_utils.cc b/chrome/common/sqlite_utils.cc
index 3f7b525..dc730de 100644
--- a/chrome/common/sqlite_utils.cc
+++ b/chrome/common/sqlite_utils.cc
@@ -10,7 +10,9 @@
 
 int OpenSqliteDb(const FilePath& filepath, sqlite3** database) {
 #if defined(OS_WIN)
-  return sqlite3_open16(filepath.value().c_str(), database);
+  // We want the default encoding to always be UTF-8, so we use the
+  // 8-bit version of open().
+  return sqlite3_open(WideToUTF8(filepath.value()).c_str(), database);
 #elif defined(OS_POSIX)
   return sqlite3_open(filepath.value().c_str(), database);
 #endif
diff --git a/chrome/common/sqlite_utils.h b/chrome/common/sqlite_utils.h
index b7325af..836ad15 100644
--- a/chrome/common/sqlite_utils.h
+++ b/chrome/common/sqlite_utils.h
@@ -309,9 +309,10 @@ class SQLStatement : public scoped_sqlite3_stmt_ptr {
 
 // TODO(estade): wrap the following static functions in a namespace.
 
-// Opens the DB in the file pointed to by |filepath|.
-// See http://www.sqlite.org/capi3ref.html#sqlite3_open for an explanation
-// of the return value.
+// Opens the DB in the file pointed to by |filepath|.  This method forces the
+// database to be in UTF-8 mode on all platforms. See
+// http://www.sqlite.org/capi3ref.html#sqlite3_open for an explanation of the
+// return value.
 int OpenSqliteDb(const FilePath& filepath, sqlite3** database);
 
 // Returns true if there is a table with the given name in the database.