summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authormrossetti@chromium.org <mrossetti@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-10-19 13:05:19 +0000
committermrossetti@chromium.org <mrossetti@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-10-19 13:05:19 +0000
commitbbf3c9d2ef091f6c109d9ab93c2bd2e477806395 (patch)
tree632e748a461fea09bfa256ddc73ce5c1294dc533
parent610ca838d7afb9c551bb186888e2ebf25ca913e7 (diff)
downloadchromium_src-bbf3c9d2ef091f6c109d9ab93c2bd2e477806395.zip
chromium_src-bbf3c9d2ef091f6c109d9ab93c2bd2e477806395.tar.gz
chromium_src-bbf3c9d2ef091f6c109d9ab93c2bd2e477806395.tar.bz2
Minor Changes Seeking Stability.
Short-circuit saving the InMemoryURLCache earlier if there is no history directory (helps with unit tests). Check in ~InMemoryURLIndex to insure the cache has been saved before dtor called. (This will signal if the dtor is called before ShutDown for some reason.) BUG=95876 TEST=Unit tests continue to succeed. Review URL: http://codereview.chromium.org/8344002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@106271 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--chrome/browser/autocomplete/history_quick_provider_unittest.cc3
-rw-r--r--chrome/browser/history/in_memory_url_index.cc23
-rw-r--r--chrome/browser/history/in_memory_url_index.h7
-rw-r--r--chrome/browser/history/in_memory_url_index_unittest.cc25
4 files changed, 35 insertions, 23 deletions
diff --git a/chrome/browser/autocomplete/history_quick_provider_unittest.cc b/chrome/browser/autocomplete/history_quick_provider_unittest.cc
index f936860..df3ac82 100644
--- a/chrome/browser/autocomplete/history_quick_provider_unittest.cc
+++ b/chrome/browser/autocomplete/history_quick_provider_unittest.cc
@@ -161,8 +161,7 @@ void HistoryQuickProviderTest::FillData() {
history::SOURCE_BROWSED);
}
- history::InMemoryURLIndex* index =
- new history::InMemoryURLIndex(FilePath(FILE_PATH_LITERAL("/dummy")));
+ history::InMemoryURLIndex* index = new history::InMemoryURLIndex(FilePath());
PrefService* prefs = profile_->GetPrefs();
std::string languages(prefs->GetString(prefs::kAcceptLanguages));
index->Init(db, languages);
diff --git a/chrome/browser/history/in_memory_url_index.cc b/chrome/browser/history/in_memory_url_index.cc
index c557acb..6388686 100644
--- a/chrome/browser/history/in_memory_url_index.cc
+++ b/chrome/browser/history/in_memory_url_index.cc
@@ -137,17 +137,23 @@ int ScoreForValue(int value, const int* value_ranks) {
InMemoryURLIndex::InMemoryURLIndex(const FilePath& history_dir)
: history_dir_(history_dir),
- history_item_count_(0) {
+ history_item_count_(0),
+ cached_at_shutdown_(false) {
InMemoryURLIndex::InitializeSchemeWhitelist(&scheme_whitelist_);
}
// Called only by unit tests.
InMemoryURLIndex::InMemoryURLIndex()
- : history_item_count_(0) {
+ : history_item_count_(0),
+ cached_at_shutdown_(false) {
InMemoryURLIndex::InitializeSchemeWhitelist(&scheme_whitelist_);
}
-InMemoryURLIndex::~InMemoryURLIndex() {}
+InMemoryURLIndex::~InMemoryURLIndex() {
+ // If there was a history directory (which there won't be for some unit tests)
+ // then insure that the cache has already been saved.
+ DCHECK(history_dir_.empty() || cached_at_shutdown_);
+}
// static
void InMemoryURLIndex::InitializeSchemeWhitelist(
@@ -174,6 +180,7 @@ bool InMemoryURLIndex::Init(history::URLDatabase* history_db,
void InMemoryURLIndex::ShutDown() {
// Write our cache.
SaveToCacheFile();
+ cached_at_shutdown_ = true;
}
bool InMemoryURLIndex::IndexRow(const URLRow& row) {
@@ -292,6 +299,10 @@ bool InMemoryURLIndex::RestoreFromCacheFile() {
bool InMemoryURLIndex::SaveToCacheFile() {
// TODO(mrossetti): Move File IO to another thread.
base::ThreadRestrictions::ScopedAllowIO allow_io;
+ FilePath file_path;
+ if (!GetCacheFilePath(&file_path))
+ return false;
+
base::TimeTicks beginning_time = base::TimeTicks::Now();
InMemoryURLIndexCacheItem index_cache;
SavePrivateData(&index_cache);
@@ -301,12 +312,6 @@ bool InMemoryURLIndex::SaveToCacheFile() {
return false;
}
- // TODO(mrossetti): Write the cache to a file then swap it for the old cache,
- // if any, and delete the old cache.
- FilePath file_path;
- if (!GetCacheFilePath(&file_path))
- return false;
-
int size = data.size();
if (file_util::WriteFile(file_path, data.c_str(), size) != size) {
LOG(WARNING) << "Failed to write " << file_path.value();
diff --git a/chrome/browser/history/in_memory_url_index.h b/chrome/browser/history/in_memory_url_index.h
index be31abe..5de5604 100644
--- a/chrome/browser/history/in_memory_url_index.h
+++ b/chrome/browser/history/in_memory_url_index.h
@@ -420,6 +420,13 @@ class InMemoryURLIndex {
// Only URLs with a whitelisted scheme are indexed.
std::set<std::string> scheme_whitelist_;
+ // Set to true at shutdown when the cache has been written to disk. Used
+ // as a temporary safety check to insure that the cache is saved before
+ // the index has been destructed.
+ // TODO(mrossetti): Eliminate once the transition to SQLite has been done.
+ // http://crbug.com/83659
+ bool cached_at_shutdown_;
+
DISALLOW_COPY_AND_ASSIGN(InMemoryURLIndex);
};
diff --git a/chrome/browser/history/in_memory_url_index_unittest.cc b/chrome/browser/history/in_memory_url_index_unittest.cc
index de8e4e0c9..62b708d 100644
--- a/chrome/browser/history/in_memory_url_index_unittest.cc
+++ b/chrome/browser/history/in_memory_url_index_unittest.cc
@@ -203,7 +203,7 @@ void ExpandedInMemoryURLIndexTest::SetUp() {
}
TEST_F(InMemoryURLIndexTest, Construction) {
- url_index_.reset(new InMemoryURLIndex(FilePath(FILE_PATH_LITERAL("/dummy"))));
+ url_index_.reset(new InMemoryURLIndex(FilePath()));
EXPECT_TRUE(url_index_.get());
}
@@ -215,7 +215,7 @@ TEST_F(LimitedInMemoryURLIndexTest, Initialization) {
uint64 row_count = 0;
while (statement.Step()) ++row_count;
EXPECT_EQ(1U, row_count);
- url_index_.reset(new InMemoryURLIndex);
+ url_index_.reset(new InMemoryURLIndex(FilePath()));
url_index_->Init(this, "en,ja,hi,zh");
EXPECT_EQ(1, url_index_->history_item_count_);
@@ -226,7 +226,7 @@ TEST_F(LimitedInMemoryURLIndexTest, Initialization) {
}
TEST_F(InMemoryURLIndexTest, Retrieval) {
- url_index_.reset(new InMemoryURLIndex(FilePath(FILE_PATH_LITERAL("/dummy"))));
+ url_index_.reset(new InMemoryURLIndex(FilePath()));
url_index_->Init(this, "en,ja,hi,zh");
// The term will be lowercased by the search.
@@ -279,7 +279,7 @@ TEST_F(InMemoryURLIndexTest, Retrieval) {
}
TEST_F(ExpandedInMemoryURLIndexTest, ShortCircuit) {
- url_index_.reset(new InMemoryURLIndex(FilePath(FILE_PATH_LITERAL("/dummy"))));
+ url_index_.reset(new InMemoryURLIndex(FilePath()));
url_index_->Init(this, "en,ja,hi,zh");
// A search for 'w' should short-circuit and not return any matches.
@@ -293,7 +293,7 @@ TEST_F(ExpandedInMemoryURLIndexTest, ShortCircuit) {
}
TEST_F(InMemoryURLIndexTest, TitleSearch) {
- url_index_.reset(new InMemoryURLIndex());
+ url_index_.reset(new InMemoryURLIndex(FilePath()));
url_index_->Init(this, "en,ja,hi,zh");
// Signal if someone has changed the test DB.
EXPECT_EQ(27U, url_index_->history_info_map_.size());
@@ -316,7 +316,7 @@ TEST_F(InMemoryURLIndexTest, TitleSearch) {
}
TEST_F(InMemoryURLIndexTest, NonUniqueTermCharacterSets) {
- url_index_.reset(new InMemoryURLIndex());
+ url_index_.reset(new InMemoryURLIndex(FilePath()));
url_index_->Init(this, "en,ja,hi,zh");
// The presence of duplicate characters should succeed. Exercise by cycling
@@ -447,7 +447,7 @@ TEST_F(InMemoryURLIndexTest, TypedCharacterCaching) {
typedef InMemoryURLIndex::SearchTermCacheMap::iterator CacheIter;
typedef InMemoryURLIndex::SearchTermCacheItem CacheItem;
- url_index_.reset(new InMemoryURLIndex(FilePath(FILE_PATH_LITERAL("/dummy"))));
+ url_index_.reset(new InMemoryURLIndex(FilePath()));
url_index_->Init(this, "en,ja,hi,zh");
InMemoryURLIndex::SearchTermCacheMap& cache(url_index_->search_term_cache_);
@@ -550,7 +550,7 @@ TEST_F(InMemoryURLIndexTest, Scoring) {
}
TEST_F(InMemoryURLIndexTest, AddNewRows) {
- url_index_.reset(new InMemoryURLIndex(FilePath(FILE_PATH_LITERAL("/dummy"))));
+ url_index_.reset(new InMemoryURLIndex(FilePath()));
url_index_->Init(this, "en,ja,hi,zh");
InMemoryURLIndex::String16Vector terms;
@@ -575,7 +575,7 @@ TEST_F(InMemoryURLIndexTest, AddNewRows) {
}
TEST_F(InMemoryURLIndexTest, DeleteRows) {
- url_index_.reset(new InMemoryURLIndex(FilePath(FILE_PATH_LITERAL("/dummy"))));
+ url_index_.reset(new InMemoryURLIndex(FilePath()));
url_index_->Init(this, "en,ja,hi,zh");
InMemoryURLIndex::String16Vector terms;
@@ -662,8 +662,7 @@ TEST_F(InMemoryURLIndexTest, WhitelistedURLs) {
{ "xmpp:node@example.com", false },
{ "xmpp://guest@example.com", false },
};
- url_index_.reset(new InMemoryURLIndex(FilePath(FILE_PATH_LITERAL(
- "/flammmy/frammy/"))));
+ url_index_.reset(new InMemoryURLIndex(FilePath()));
for (size_t i = 0; i < ARRAYSIZE_UNSAFE(data); ++i) {
GURL url(data[i].url_spec);
EXPECT_EQ(data[i].expected_is_whitelisted,
@@ -685,11 +684,13 @@ TEST_F(InMemoryURLIndexTest, CacheFilePath) {
size_t count = expected_parts.size();
for (size_t i = 0; i < count; ++i)
EXPECT_EQ(expected_parts[i], actual_parts[i]);
+ // Must clear the history_dir_ to satisfy the dtor's DCHECK.
+ url_index_->history_dir_.clear();
}
TEST_F(InMemoryURLIndexTest, CacheSaveRestore) {
// Save the cache to a protobuf, restore it, and compare the results.
- url_index_.reset(new InMemoryURLIndex(FilePath(FILE_PATH_LITERAL("/dummy"))));
+ url_index_.reset(new InMemoryURLIndex(FilePath()));
InMemoryURLIndex& url_index(*(url_index_.get()));
url_index.Init(this, "en,ja,hi,zh");
in_memory_url_index::InMemoryURLIndexCacheItem index_cache;