Index: chrome/browser/history/history_unittest.cc |
diff --git a/chrome/browser/history/history_unittest.cc b/chrome/browser/history/history_unittest.cc |
index 364a5d0154e2c3802735d2bbc29cd0d4286f9cb9..5f2c0a8aeb99f6441bc59b87d0dd20f315f1fba5 100644 |
--- a/chrome/browser/history/history_unittest.cc |
+++ b/chrome/browser/history/history_unittest.cc |
@@ -137,15 +137,20 @@ class HistoryBackendDBTest : public testing::Test { |
} |
int64 AddDownload(DownloadItem::DownloadState state, const Time& time) { |
+ std::vector<GURL> url_chain; |
+ url_chain.push_back(GURL("foo-url")); |
+ |
DownloadPersistentStoreInfo download( |
FilePath(FILE_PATH_LITERAL("foo-path")), |
- GURL("foo-url"), |
+ FilePath(FILE_PATH_LITERAL("foo-path")), |
+ url_chain, |
GURL(""), |
time, |
time, |
0, |
512, |
state, |
+ content::DOWNLOAD_INTERRUPT_REASON_NONE, |
0, |
0); |
return db_->CreateDownload(download); |
@@ -185,6 +190,19 @@ void BackendDelegate::BroadcastNotifications(int type, |
namespace { |
+// Schema for the downloads database for verion 23 and earlier. |
+const char* kVersion23DownloadsSchema = |
+ "CREATE TABLE downloads (" |
+ "id INTEGER PRIMARY KEY," |
+ "full_path LONGVARCHAR NOT NULL," |
+ "url LONGVARCHAR NOT NULL," |
+ "start_time INTEGER NOT NULL," |
+ "received_bytes INTEGER NOT NULL," |
+ "total_bytes INTEGER NOT NULL," |
+ "state INTEGER NOT NULL," |
+ "end_time INTEGER NOT NULL," |
+ "opened INTEGER NOT NULL)"; |
+ |
TEST_F(HistoryBackendDBTest, ClearBrowsingData_Downloads) { |
CreateBackendAndDatabase(); |
@@ -228,15 +246,11 @@ TEST_F(HistoryBackendDBTest, ClearBrowsingData_Downloads) { |
db_->QueryDownloads(&downloads); |
EXPECT_EQ(2U, downloads.size()); |
- // Download manager converts to TimeT, which is lossy, so we do the same |
- // for comparison. |
- Time month_ago_lossy = Time::FromTimeT(month_ago.ToTimeT()); |
- |
// Make sure the right values remain. |
EXPECT_EQ(DownloadItem::COMPLETE, downloads[0].state); |
EXPECT_EQ(0, downloads[0].start_time.ToInternalValue()); |
EXPECT_EQ(DownloadItem::IN_PROGRESS, downloads[1].state); |
- EXPECT_EQ(month_ago_lossy.ToInternalValue(), |
+ EXPECT_EQ(month_ago.ToInternalValue(), |
downloads[1].start_time.ToInternalValue()); |
// Change state so we can delete the downloads. |
@@ -286,6 +300,11 @@ TEST_F(HistoryBackendDBTest, MigrateDownloadsState) { |
"UPDATE meta SET value=22 WHERE key='version'")); |
ASSERT_TRUE(version22.Run()); |
} |
+ // Nuke the new tables and create an old one. |
+ ASSERT_TRUE(db.Execute("DROP TABLE downloads")); |
+ ASSERT_TRUE(db.Execute("DROP TABLE downloads_url_chains")); |
+ ASSERT_TRUE(db.Execute(kVersion23DownloadsSchema)); |
+ |
// Manually insert corrupted rows; there's infrastructure in place now to |
// make this impossible, at least according to the test above. |
for (int state = 0; state < 5; ++state) { |
@@ -343,6 +362,259 @@ TEST_F(HistoryBackendDBTest, MigrateDownloadsState) { |
} |
} |
+TEST_F(HistoryBackendDBTest, MigrateDownloadsReasonAndPaths) { |
+ Time now(base::Time::Now()); |
+ |
+ // Create the db and close it so that we can reopen it directly. |
+ CreateBackendAndDatabase(); |
+ DeleteBackend(); |
+ { |
+ // Re-open the db for manual manipulation. |
+ sql::Connection db; |
+ ASSERT_TRUE(db.Open(history_dir_.Append(chrome::kHistoryFilename))); |
+ { |
+ // Manually force the version to 23. |
+ sql::Statement version23(db.GetUniqueStatement( |
+ "UPDATE meta SET value=23 WHERE key='version'")); |
+ ASSERT_TRUE(version23.Run()); |
+ } |
+ |
+ // Nuke the new tables and create an old one with some hand crafted |
+ // values in it. |
+ ASSERT_TRUE(db.Execute("DROP TABLE downloads")); |
+ ASSERT_TRUE(db.Execute("DROP TABLE downloads_url_chains")); |
+ ASSERT_TRUE(db.Execute(kVersion23DownloadsSchema)); |
+ |
+ // Manually insert some rows. |
+ sql::Statement s(db.GetUniqueStatement( |
+ "INSERT INTO downloads (id, full_path, url, start_time, " |
+ "received_bytes, total_bytes, state, end_time, opened) VALUES " |
+ "(?, ?, ?, ?, ?, ?, ?, ?, ?)")); |
+ |
+ int64 db_handle = 0; |
+ // Null path. |
+ s.BindInt64(0, ++db_handle); |
+ s.BindString(1, ""); |
+ s.BindString(2, "http://whatever.com/index.html"); |
+ s.BindInt64(3, now.ToTimeT()); |
+ s.BindInt64(4, 100); |
+ s.BindInt64(5, 100); |
+ s.BindInt(6, 1); |
+ s.BindInt64(7, now.ToTimeT()); |
+ s.BindInt(8, 1); |
+ ASSERT_TRUE(s.Run()); |
+ s.Reset(true); |
+ |
+ // Non-null path. |
+ s.BindInt64(0, ++db_handle); |
+ s.BindString(1, "/path/to/some/file"); |
+ s.BindString(2, "http://whatever.com/index1.html"); |
+ s.BindInt64(3, now.ToTimeT()); |
+ s.BindInt64(4, 100); |
+ s.BindInt64(5, 100); |
+ s.BindInt(6, 1); |
+ s.BindInt64(7, now.ToTimeT()); |
+ s.BindInt(8, 1); |
+ ASSERT_TRUE(s.Run()); |
+ } |
+ |
+ // Re-open the db using the HistoryDatabase, which should migrate from version |
+ // 23 to 24, creating the new tables and creating the new path and reason |
+ // columns. |
+ CreateBackendAndDatabase(); |
+ DeleteBackend(); |
+ { |
+ // Re-open the db for manual manipulation. |
+ sql::Connection db; |
+ ASSERT_TRUE(db.Open(history_dir_.Append(chrome::kHistoryFilename))); |
+ { |
+ // The version should have been updated. |
+ int cur_version = HistoryDatabase::GetCurrentVersion(); |
+ ASSERT_LT(23, cur_version); |
+ sql::Statement s(db.GetUniqueStatement( |
+ "SELECT value FROM meta WHERE key = 'version'")); |
+ EXPECT_TRUE(s.Step()); |
+ EXPECT_EQ(cur_version, s.ColumnInt(0)); |
+ } |
+ { |
+ base::Time nowish(base::Time::FromTimeT(now.ToTimeT())); |
+ |
+ // Confirm downloads table is valid. |
+ sql::Statement statement(db.GetUniqueStatement( |
+ "SELECT id, interrupt_reason, target_path, current_path, " |
+ " start_time, end_time " |
+ "FROM downloads ORDER BY id")); |
+ EXPECT_TRUE(statement.Step()); |
+ EXPECT_EQ(1, statement.ColumnInt64(0)); |
+ EXPECT_EQ(content::DOWNLOAD_INTERRUPT_REASON_NONE, |
+ statement.ColumnInt(1)); |
+ EXPECT_EQ("", statement.ColumnString(2)); |
+ EXPECT_EQ("", statement.ColumnString(3)); |
+ EXPECT_EQ(nowish.ToInternalValue(), statement.ColumnInt64(4)); |
+ EXPECT_EQ(nowish.ToInternalValue(), statement.ColumnInt64(5)); |
+ |
+ EXPECT_TRUE(statement.Step()); |
+ EXPECT_EQ(2, statement.ColumnInt64(0)); |
+ EXPECT_EQ(content::DOWNLOAD_INTERRUPT_REASON_NONE, |
+ statement.ColumnInt(1)); |
+ EXPECT_EQ("/path/to/some/file", statement.ColumnString(2)); |
+ EXPECT_EQ("/path/to/some/file", statement.ColumnString(3)); |
+ EXPECT_EQ(nowish.ToInternalValue(), statement.ColumnInt64(4)); |
+ EXPECT_EQ(nowish.ToInternalValue(), statement.ColumnInt64(5)); |
+ |
+ EXPECT_FALSE(statement.Step()); |
+ } |
+ { |
+ // Confirm donwloads_url_chains table is valid. |
+ sql::Statement statement(db.GetUniqueStatement( |
+ "SELECT id, chain_index, url FROM downloads_url_chains " |
+ " ORDER BY id, chain_index")); |
+ EXPECT_TRUE(statement.Step()); |
+ EXPECT_EQ(1, statement.ColumnInt64(0)); |
+ EXPECT_EQ(0, statement.ColumnInt(1)); |
+ EXPECT_EQ("http://whatever.com/index.html", statement.ColumnString(2)); |
+ |
+ EXPECT_TRUE(statement.Step()); |
+ EXPECT_EQ(2, statement.ColumnInt64(0)); |
+ EXPECT_EQ(0, statement.ColumnInt(1)); |
+ EXPECT_EQ("http://whatever.com/index1.html", statement.ColumnString(2)); |
+ |
+ EXPECT_FALSE(statement.Step()); |
+ } |
+ } |
+} |
+ |
+TEST_F(HistoryBackendDBTest, ConfirmDownloadRowCreateAndDelete) { |
+ // Create the DB. |
+ CreateBackendAndDatabase(); |
+ |
+ base::Time now(base::Time::Now()); |
+ |
+ // Add some downloads. |
+ AddDownload(DownloadItem::COMPLETE, now); |
+ int64 did2 = AddDownload(DownloadItem::COMPLETE, now + |
+ base::TimeDelta::FromDays(2)); |
+ int64 did3 = AddDownload(DownloadItem::COMPLETE, now - |
+ base::TimeDelta::FromDays(2)); |
+ |
+ // Confirm that resulted in the correct number of rows in the DB. |
+ DeleteBackend(); |
+ { |
+ sql::Connection db; |
+ ASSERT_TRUE(db.Open(history_dir_.Append(chrome::kHistoryFilename))); |
+ sql::Statement statement(db.GetUniqueStatement( |
+ "Select Count(*) from downloads")); |
+ EXPECT_TRUE(statement.Step()); |
+ EXPECT_EQ(3, statement.ColumnInt(0)); |
+ |
+ sql::Statement statement1(db.GetUniqueStatement( |
+ "Select Count(*) from downloads_url_chains")); |
+ EXPECT_TRUE(statement1.Step()); |
+ EXPECT_EQ(3, statement1.ColumnInt(0)); |
+ } |
+ |
+ // Delete some rows and make sure the results are still correct. |
+ CreateBackendAndDatabase(); |
+ db_->RemoveDownload(did2); |
+ db_->RemoveDownload(did3); |
+ DeleteBackend(); |
+ { |
+ sql::Connection db; |
+ ASSERT_TRUE(db.Open(history_dir_.Append(chrome::kHistoryFilename))); |
+ sql::Statement statement(db.GetUniqueStatement( |
+ "Select Count(*) from downloads")); |
+ EXPECT_TRUE(statement.Step()); |
+ EXPECT_EQ(1, statement.ColumnInt(0)); |
+ |
+ sql::Statement statement1(db.GetUniqueStatement( |
+ "Select Count(*) from downloads_url_chains")); |
+ EXPECT_TRUE(statement1.Step()); |
+ EXPECT_EQ(1, statement1.ColumnInt(0)); |
+ } |
+} |
+ |
+struct InterruptReasonAssociation { |
+ std::string name; |
+ int value; |
+}; |
+ |
+// Test is dependent on interrupt reasons being listed in header file |
+// in order. |
+const InterruptReasonAssociation current_reasons[] = { |
+#define INTERRUPT_REASON(a, b) { #a, b }, |
+#include "content/public/browser/download_interrupt_reason_values.h" |
+#undef INTERRUPT_REASON |
+}; |
+const InterruptReasonAssociation historical_reasons[] = { |
benjhayden
2012/11/19 15:22:07
Newline before this line and a comment forbidding
Randy Smith (Not in Mondays)
2012/12/09 15:14:45
Done.
|
+ { "FILE_FAILED", 1 }, |
+ { "FILE_ACCESS_DENIED", 2 }, |
+ { "FILE_NO_SPACE", 3 }, |
+ { "FILE_NAME_TOO_LONG", 5 }, |
+ { "FILE_TOO_LARGE", 6 }, |
+ { "FILE_VIRUS_INFECTED", 7 }, |
+ { "FILE_TRANSIENT_ERROR", 10 }, |
+ { "FILE_BLOCKED", 11 }, |
+ { "FILE_SECURITY_CHECK_FAILED", 12 }, |
+ { "NETWORK_FAILED", 20 }, |
+ { "NETWORK_TIMEOUT", 21 }, |
+ { "NETWORK_DISCONNECTED", 22 }, |
+ { "NETWORK_SERVER_DOWN", 23 }, |
+ { "SERVER_FAILED", 30 }, |
+ { "SERVER_NO_RANGE", 31 }, |
+ { "SERVER_PRECONDITION", 32 }, |
+ { "SERVER_BAD_CONTENT", 33 }, |
+ { "USER_CANCELED", 40 }, |
+ { "USER_SHUTDOWN", 41 }, |
+ { "CRASH", 50 }, |
+}; |
+ |
+// Make sure no one has changed a DownloadInterruptReason we've previously |
+// persisted. |
+TEST_F(HistoryBackendDBTest, ConfirmDownloadInterruptReasonOk) { |
benjhayden
2012/11/19 15:22:07
"Confirm...Ok" is too vague. All tests confirm tha
Randy Smith (Not in Mondays)
2012/12/09 15:14:45
Done.
|
+ // Are there any casesin which a historical number has been repurposed |
benjhayden
2012/11/19 15:22:07
"cases in"
Randy Smith (Not in Mondays)
2012/12/09 15:14:45
Done.
|
+ // for an error other than it's original? |
+ for (size_t i = 0; i < arraysize(current_reasons); i++) { |
+ const InterruptReasonAssociation& cur_reason(current_reasons[i]); |
+ bool found = false; |
+ |
+ for (size_t j = 0; j < arraysize(historical_reasons); ++j) { |
+ const InterruptReasonAssociation& hist_reason(historical_reasons[j]); |
+ |
+ if (hist_reason.value == cur_reason.value) { |
+ EXPECT_EQ(cur_reason.name, hist_reason.name) |
+ << "Same integer value used for old error \"" |
+ << hist_reason.name |
+ << "\" as for new error \"" |
+ << cur_reason.name |
+ << "\"." << std::endl |
+ << "**This will cause database conflicts with persisted values**" |
+ << std::endl |
+ << "Please assign a new, non-conflicting value for the new error."; |
+ } |
+ |
+ if (hist_reason.name == cur_reason.name) { |
+ EXPECT_EQ(cur_reason.value, hist_reason.value) |
+ << "Same name (\"" << hist_reason.name |
+ << "\") maps to a different value historically (" |
+ << hist_reason.value << ") and currently (" |
+ << cur_reason.value << ")" << std::endl |
+ << "This may cause database conflicts with persisted values" |
+ << std::endl |
+ << "You are advised to use the same value for the same error, " |
+ << "or to update this test with approriate commenting if you " |
benjhayden
2012/11/19 15:22:07
When might this be appropriate?
Randy Smith (Not in Mondays)
2012/12/09 15:14:45
I couldn't think of a time, but it won't Seriously
|
+ << "know what you are doing."; |
+ |
+ found = true; |
+ } |
+ } |
+ |
+ EXPECT_TRUE(found) |
+ << "Error \"" << cur_reason.name << "\" not found in historical list." |
+ << std::endl |
+ << "Please add it."; |
+ } |
+} |
+ |
// The tracker uses RenderProcessHost pointers for scoping but never |
// dereferences them. We use ints because it's easier. This function converts |
// between the two. |