Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(25)

Side by Side Diff: chrome/browser/history/download_database.cc

Issue 10823203: Fix downloads db state=3 corruption using version=23 (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: merge Created 8 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch | Annotate | Revision Log
« no previous file with comments | « chrome/browser/history/download_database.h ('k') | chrome/browser/history/history_database.cc » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "chrome/browser/history/download_database.h" 5 #include "chrome/browser/history/download_database.h"
6 6
7 #include <limits> 7 #include <limits>
8 #include <string> 8 #include <string>
9 #include <vector> 9 #include <vector>
10 10
(...skipping 20 matching lines...) Expand all
31 "id INTEGER PRIMARY KEY," // SQLite-generated primary key. 31 "id INTEGER PRIMARY KEY," // SQLite-generated primary key.
32 "full_path LONGVARCHAR NOT NULL," // Location of the download on disk. 32 "full_path LONGVARCHAR NOT NULL," // Location of the download on disk.
33 "url LONGVARCHAR NOT NULL," // URL of the downloaded file. 33 "url LONGVARCHAR NOT NULL," // URL of the downloaded file.
34 "start_time INTEGER NOT NULL," // When the download was started. 34 "start_time INTEGER NOT NULL," // When the download was started.
35 "received_bytes INTEGER NOT NULL," // Total size downloaded. 35 "received_bytes INTEGER NOT NULL," // Total size downloaded.
36 "total_bytes INTEGER NOT NULL," // Total size of the download. 36 "total_bytes INTEGER NOT NULL," // Total size of the download.
37 "state INTEGER NOT NULL," // 1=complete, 2=cancelled, 4=interrupted 37 "state INTEGER NOT NULL," // 1=complete, 2=cancelled, 4=interrupted
38 "end_time INTEGER NOT NULL," // When the download completed. 38 "end_time INTEGER NOT NULL," // When the download completed.
39 "opened INTEGER NOT NULL)"; // 1 if it has ever been opened else 0 39 "opened INTEGER NOT NULL)"; // 1 if it has ever been opened else 0
40 40
41 // These constants and next two functions are used to allow
42 // DownloadItem::DownloadState to change without breaking the database schema.
43 // They guarantee that the values of the |state| field in the database are one
44 // of the values returned by StateToInt, and that the values of the |state|
45 // field of the DownloadPersistentStoreInfos returned by QueryDownloads() are
46 // one of the values returned by IntToState().
47 static const int kStateInvalid = -1;
48 static const int kStateInProgress = 0;
49 static const int kStateComplete = 1;
50 static const int kStateCancelled = 2;
51 static const int kStateBug140687 = 3;
52 static const int kStateInterrupted = 4;
53
54 int StateToInt(DownloadItem::DownloadState state) {
55 switch (state) {
56 case DownloadItem::IN_PROGRESS: return kStateInProgress;
57 case DownloadItem::COMPLETE: return kStateComplete;
58 case DownloadItem::CANCELLED: return kStateCancelled;
59 case DownloadItem::REMOVING: return kStateInvalid;
60 case DownloadItem::INTERRUPTED: return kStateInterrupted;
61 case DownloadItem::MAX_DOWNLOAD_STATE: return kStateInvalid;
62 default: return kStateInvalid;
63 }
64 }
65
66 DownloadItem::DownloadState IntToState(int state) {
67 switch (state) {
68 case kStateInProgress: return DownloadItem::IN_PROGRESS;
69 case kStateComplete: return DownloadItem::COMPLETE;
70 case kStateCancelled: return DownloadItem::CANCELLED;
71 // We should not need kStateBug140687 here because MigrateDownloadState()
72 // is called in HistoryDatabase::Init().
73 case kStateInterrupted: return DownloadItem::INTERRUPTED;
74 default: return DownloadItem::MAX_DOWNLOAD_STATE;
75 }
76 }
77
41 #if defined(OS_POSIX) 78 #if defined(OS_POSIX)
42 79
43 // Binds/reads the given file path to the given column of the given statement. 80 // Binds/reads the given file path to the given column of the given statement.
44 void BindFilePath(sql::Statement& statement, const FilePath& path, int col) { 81 void BindFilePath(sql::Statement& statement, const FilePath& path, int col) {
45 statement.BindString(col, path.value()); 82 statement.BindString(col, path.value());
46 } 83 }
47 FilePath ColumnFilePath(sql::Statement& statement, int col) { 84 FilePath ColumnFilePath(sql::Statement& statement, int col) {
48 return FilePath(statement.ColumnString(col)); 85 return FilePath(statement.ColumnString(col));
49 } 86 }
50 87
(...skipping 34 matching lines...) Expand 10 before | Expand all | Expand 10 after
85 } 122 }
86 } 123 }
87 124
88 bool DownloadDatabase::EnsureColumnExists( 125 bool DownloadDatabase::EnsureColumnExists(
89 const std::string& name, const std::string& type) { 126 const std::string& name, const std::string& type) {
90 std::string add_col = "ALTER TABLE downloads ADD COLUMN " + name + " " + type; 127 std::string add_col = "ALTER TABLE downloads ADD COLUMN " + name + " " + type;
91 return GetDB().DoesColumnExist("downloads", name.c_str()) || 128 return GetDB().DoesColumnExist("downloads", name.c_str()) ||
92 GetDB().Execute(add_col.c_str()); 129 GetDB().Execute(add_col.c_str());
93 } 130 }
94 131
132 bool DownloadDatabase::MigrateDownloadsState() {
133 sql::Statement statement(GetDB().GetUniqueStatement(
134 "UPDATE downloads SET state=? WHERE state=?"));
135 statement.BindInt(0, kStateInterrupted);
136 statement.BindInt(1, kStateBug140687);
137 return statement.Run();
138 }
139
95 bool DownloadDatabase::InitDownloadTable() { 140 bool DownloadDatabase::InitDownloadTable() {
96 CheckThread(); 141 CheckThread();
97 GetMetaTable().GetValue(kNextDownloadId, &next_id_); 142 GetMetaTable().GetValue(kNextDownloadId, &next_id_);
98 if (GetDB().DoesTableExist("downloads")) { 143 if (GetDB().DoesTableExist("downloads")) {
99 return EnsureColumnExists("end_time", "INTEGER NOT NULL DEFAULT 0") && 144 return EnsureColumnExists("end_time", "INTEGER NOT NULL DEFAULT 0") &&
100 EnsureColumnExists("opened", "INTEGER NOT NULL DEFAULT 0"); 145 EnsureColumnExists("opened", "INTEGER NOT NULL DEFAULT 0");
101 } else { 146 } else {
102 return GetDB().Execute(kSchema); 147 return GetDB().Execute(kSchema);
103 } 148 }
104 } 149 }
(...skipping 18 matching lines...) Expand all
123 "ORDER BY start_time")); 168 "ORDER BY start_time"));
124 169
125 while (statement.Step()) { 170 while (statement.Step()) {
126 DownloadPersistentStoreInfo info; 171 DownloadPersistentStoreInfo info;
127 info.db_handle = statement.ColumnInt64(0); 172 info.db_handle = statement.ColumnInt64(0);
128 info.path = ColumnFilePath(statement, 1); 173 info.path = ColumnFilePath(statement, 1);
129 info.url = GURL(statement.ColumnString(2)); 174 info.url = GURL(statement.ColumnString(2));
130 info.start_time = base::Time::FromTimeT(statement.ColumnInt64(3)); 175 info.start_time = base::Time::FromTimeT(statement.ColumnInt64(3));
131 info.received_bytes = statement.ColumnInt64(4); 176 info.received_bytes = statement.ColumnInt64(4);
132 info.total_bytes = statement.ColumnInt64(5); 177 info.total_bytes = statement.ColumnInt64(5);
133 info.state = statement.ColumnInt(6); 178 int state = statement.ColumnInt(6);
179 info.state = IntToState(state);
134 info.end_time = base::Time::FromTimeT(statement.ColumnInt64(7)); 180 info.end_time = base::Time::FromTimeT(statement.ColumnInt64(7));
135 info.opened = statement.ColumnInt(8) != 0; 181 info.opened = statement.ColumnInt(8) != 0;
136 results->push_back(info);
137 if (info.db_handle >= next_db_handle_) 182 if (info.db_handle >= next_db_handle_)
138 next_db_handle_ = info.db_handle + 1; 183 next_db_handle_ = info.db_handle + 1;
139 if (!db_handles.insert(info.db_handle).second) { 184 if (!db_handles.insert(info.db_handle).second) {
140 // info.db_handle was already in db_handles. The database is corrupt. 185 // info.db_handle was already in db_handles. The database is corrupt.
141 base::debug::Alias(&info.db_handle); 186 base::debug::Alias(&info.db_handle);
142 DCHECK(false); 187 DCHECK(false);
143 } 188 }
189 if (info.state == DownloadItem::MAX_DOWNLOAD_STATE) {
190 UMA_HISTOGRAM_COUNTS("Download.DatabaseInvalidState", state);
191 continue;
192 }
193 results->push_back(info);
144 } 194 }
145 } 195 }
146 196
147 bool DownloadDatabase::UpdateDownload(const DownloadPersistentStoreInfo& data) { 197 bool DownloadDatabase::UpdateDownload(const DownloadPersistentStoreInfo& data) {
148 CheckThread(); 198 CheckThread();
149 DCHECK(data.db_handle > 0); 199 DCHECK(data.db_handle > 0);
200 int state = StateToInt(data.state);
201 if (state == kStateInvalid) {
202 // TODO(benjhayden) [D]CHECK instead.
203 return false;
204 }
150 sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE, 205 sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
151 "UPDATE downloads " 206 "UPDATE downloads "
152 "SET received_bytes=?, state=?, end_time=?, opened=? WHERE id=?")); 207 "SET received_bytes=?, state=?, end_time=?, opened=? WHERE id=?"));
153 statement.BindInt64(0, data.received_bytes); 208 statement.BindInt64(0, data.received_bytes);
154 statement.BindInt(1, data.state); 209 statement.BindInt(1, state);
155 statement.BindInt64(2, data.end_time.ToTimeT()); 210 statement.BindInt64(2, data.end_time.ToTimeT());
156 statement.BindInt(3, (data.opened ? 1 : 0)); 211 statement.BindInt(3, (data.opened ? 1 : 0));
157 statement.BindInt64(4, data.db_handle); 212 statement.BindInt64(4, data.db_handle);
158 213
159 return statement.Run(); 214 return statement.Run();
160 } 215 }
161 216
162 bool DownloadDatabase::UpdateDownloadPath(const FilePath& path, 217 bool DownloadDatabase::UpdateDownloadPath(const FilePath& path,
163 DownloadID db_handle) { 218 DownloadID db_handle) {
164 CheckThread(); 219 CheckThread();
165 DCHECK(db_handle > 0); 220 DCHECK(db_handle > 0);
166 sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE, 221 sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
167 "UPDATE downloads SET full_path=? WHERE id=?")); 222 "UPDATE downloads SET full_path=? WHERE id=?"));
168 BindFilePath(statement, path, 0); 223 BindFilePath(statement, path, 0);
169 statement.BindInt64(1, db_handle); 224 statement.BindInt64(1, db_handle);
170 225
171 return statement.Run(); 226 return statement.Run();
172 } 227 }
173 228
174 bool DownloadDatabase::CleanUpInProgressEntries() { 229 bool DownloadDatabase::CleanUpInProgressEntries() {
175 CheckThread(); 230 CheckThread();
176 sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE, 231 sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
177 "UPDATE downloads SET state=? WHERE state=?")); 232 "UPDATE downloads SET state=? WHERE state=?"));
178 statement.BindInt(0, DownloadItem::CANCELLED); 233 statement.BindInt(0, kStateCancelled);
179 statement.BindInt(1, DownloadItem::IN_PROGRESS); 234 statement.BindInt(1, kStateInProgress);
180 235
181 return statement.Run(); 236 return statement.Run();
182 } 237 }
183 238
184 int64 DownloadDatabase::CreateDownload( 239 int64 DownloadDatabase::CreateDownload(
185 const DownloadPersistentStoreInfo& info) { 240 const DownloadPersistentStoreInfo& info) {
186 CheckThread(); 241 CheckThread();
187 242
188 if (next_db_handle_ == 0) { 243 if (next_db_handle_ == 0) {
189 // This is unlikely. All current known tests and users already call 244 // This is unlikely. All current known tests and users already call
190 // QueryDownloads() before CreateDownload(). 245 // QueryDownloads() before CreateDownload().
191 std::vector<DownloadPersistentStoreInfo> results; 246 std::vector<DownloadPersistentStoreInfo> results;
192 QueryDownloads(&results); 247 QueryDownloads(&results);
193 CHECK_NE(0, next_db_handle_); 248 CHECK_NE(0, next_db_handle_);
194 } 249 }
195 250
251 int state = StateToInt(info.state);
252 if (state == kStateInvalid)
253 return false;
254
196 sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE, 255 sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
197 "INSERT INTO downloads " 256 "INSERT INTO downloads "
198 "(id, full_path, url, start_time, received_bytes, total_bytes, state, " 257 "(id, full_path, url, start_time, received_bytes, total_bytes, state, "
199 "end_time, opened) " 258 "end_time, opened) "
200 "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)")); 259 "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)"));
201 260
202 int db_handle = next_db_handle_++; 261 int db_handle = next_db_handle_++;
203 262
204 statement.BindInt64(0, db_handle); 263 statement.BindInt64(0, db_handle);
205 BindFilePath(statement, info.path, 1); 264 BindFilePath(statement, info.path, 1);
206 statement.BindString(2, info.url.spec()); 265 statement.BindString(2, info.url.spec());
207 statement.BindInt64(3, info.start_time.ToTimeT()); 266 statement.BindInt64(3, info.start_time.ToTimeT());
208 statement.BindInt64(4, info.received_bytes); 267 statement.BindInt64(4, info.received_bytes);
209 statement.BindInt64(5, info.total_bytes); 268 statement.BindInt64(5, info.total_bytes);
210 statement.BindInt(6, info.state); 269 statement.BindInt(6, state);
211 statement.BindInt64(7, info.end_time.ToTimeT()); 270 statement.BindInt64(7, info.end_time.ToTimeT());
212 statement.BindInt(8, info.opened ? 1 : 0); 271 statement.BindInt(8, info.opened ? 1 : 0);
213 272
214 if (statement.Run()) { 273 if (statement.Run()) {
215 // TODO(benjhayden) if(info.id>next_id_){setvalue;next_id_=info.id;} 274 // TODO(benjhayden) if(info.id>next_id_){setvalue;next_id_=info.id;}
216 GetMetaTable().SetValue(kNextDownloadId, ++next_id_); 275 GetMetaTable().SetValue(kNextDownloadId, ++next_id_);
217 276
218 return db_handle; 277 return db_handle;
219 } 278 }
220 return 0; 279 return 0;
(...skipping 17 matching lines...) Expand all
238 297
239 int num_downloads_deleted = -1; 298 int num_downloads_deleted = -1;
240 { 299 {
241 sql::Statement count(GetDB().GetCachedStatement(SQL_FROM_HERE, 300 sql::Statement count(GetDB().GetCachedStatement(SQL_FROM_HERE,
242 "SELECT count(*) FROM downloads WHERE start_time >= ? " 301 "SELECT count(*) FROM downloads WHERE start_time >= ? "
243 "AND start_time < ? AND (State = ? OR State = ? OR State = ?)")); 302 "AND start_time < ? AND (State = ? OR State = ? OR State = ?)"));
244 count.BindInt64(0, start_time); 303 count.BindInt64(0, start_time);
245 count.BindInt64( 304 count.BindInt64(
246 1, 305 1,
247 end_time ? end_time : std::numeric_limits<int64>::max()); 306 end_time ? end_time : std::numeric_limits<int64>::max());
248 count.BindInt(2, DownloadItem::COMPLETE); 307 count.BindInt(2, kStateComplete);
249 count.BindInt(3, DownloadItem::CANCELLED); 308 count.BindInt(3, kStateCancelled);
250 count.BindInt(4, DownloadItem::INTERRUPTED); 309 count.BindInt(4, kStateInterrupted);
251 if (count.Step()) 310 if (count.Step())
252 num_downloads_deleted = count.ColumnInt(0); 311 num_downloads_deleted = count.ColumnInt(0);
253 } 312 }
254 313
255 314
256 bool success = false; 315 bool success = false;
257 base::TimeTicks started_removing = base::TimeTicks::Now(); 316 base::TimeTicks started_removing = base::TimeTicks::Now();
258 { 317 {
259 // This does not use an index. We currently aren't likely to have enough 318 // This does not use an index. We currently aren't likely to have enough
260 // downloads where an index by time will give us a lot of benefit. 319 // downloads where an index by time will give us a lot of benefit.
261 sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE, 320 sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
262 "DELETE FROM downloads WHERE start_time >= ? AND start_time < ? " 321 "DELETE FROM downloads WHERE start_time >= ? AND start_time < ? "
263 "AND (State = ? OR State = ? OR State = ?)")); 322 "AND (State = ? OR State = ? OR State = ?)"));
264 statement.BindInt64(0, start_time); 323 statement.BindInt64(0, start_time);
265 statement.BindInt64( 324 statement.BindInt64(
266 1, 325 1,
267 end_time ? end_time : std::numeric_limits<int64>::max()); 326 end_time ? end_time : std::numeric_limits<int64>::max());
268 statement.BindInt(2, DownloadItem::COMPLETE); 327 statement.BindInt(2, kStateComplete);
269 statement.BindInt(3, DownloadItem::CANCELLED); 328 statement.BindInt(3, kStateCancelled);
270 statement.BindInt(4, DownloadItem::INTERRUPTED); 329 statement.BindInt(4, kStateInterrupted);
271 330
272 success = statement.Run(); 331 success = statement.Run();
273 } 332 }
274 333
275 base::TimeTicks finished_removing = base::TimeTicks::Now(); 334 base::TimeTicks finished_removing = base::TimeTicks::Now();
276 335
277 if (num_downloads_deleted >= 0) { 336 if (num_downloads_deleted >= 0) {
278 UMA_HISTOGRAM_COUNTS("Download.DatabaseRemoveDownloadsCount", 337 UMA_HISTOGRAM_COUNTS("Download.DatabaseRemoveDownloadsCount",
279 num_downloads_deleted); 338 num_downloads_deleted);
280 base::TimeDelta micros = (1000 * (finished_removing - started_removing)); 339 base::TimeDelta micros = (1000 * (finished_removing - started_removing));
281 UMA_HISTOGRAM_TIMES("Download.DatabaseRemoveDownloadsTime", micros); 340 UMA_HISTOGRAM_TIMES("Download.DatabaseRemoveDownloadsTime", micros);
282 if (num_downloads_deleted > 0) { 341 if (num_downloads_deleted > 0) {
283 UMA_HISTOGRAM_TIMES("Download.DatabaseRemoveDownloadsTimePerRecord", 342 UMA_HISTOGRAM_TIMES("Download.DatabaseRemoveDownloadsTimePerRecord",
284 (1000 * micros) / num_downloads_deleted); 343 (1000 * micros) / num_downloads_deleted);
285 } 344 }
286 } 345 }
287 346
288 return success; 347 return success;
289 } 348 }
290 349
291 } // namespace history 350 } // namespace history
OLDNEW
« no previous file with comments | « chrome/browser/history/download_database.h ('k') | chrome/browser/history/history_database.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698