diff options
| author | atwilson@chromium.org <atwilson@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-09-15 19:22:42 +0000 |
|---|---|---|
| committer | atwilson@chromium.org <atwilson@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-09-15 19:22:42 +0000 |
| commit | f4e003f0633de7f2beb2a1e979bc720ec4e8144e (patch) | |
| tree | 9543981aa21adeec6c93484e1a1733be9e31492a /chrome | |
| parent | 2cbde8369789835e981bbb464ef605be5a4a6076 (diff) | |
| download | chromium_src-f4e003f0633de7f2beb2a1e979bc720ec4e8144e.zip chromium_src-f4e003f0633de7f2beb2a1e979bc720ec4e8144e.tar.gz chromium_src-f4e003f0633de7f2beb2a1e979bc720ec4e8144e.tar.bz2 | |
Fixup last_visit timestamp since it can get out of whack due to bug96341
BUG=96341
TEST=Run unit tests (I also have a profile with data integrity issues I can run against, but it has my personal data in it and I can't share it).
Review URL: http://codereview.chromium.org/7900001
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@101355 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome')
4 files changed, 27 insertions, 25 deletions
diff --git a/chrome/browser/sync/glue/typed_url_change_processor.cc b/chrome/browser/sync/glue/typed_url_change_processor.cc index aa0ca0a..a649f98 100644 --- a/chrome/browser/sync/glue/typed_url_change_processor.cc +++ b/chrome/browser/sync/glue/typed_url_change_processor.cc @@ -88,11 +88,11 @@ void TypedUrlChangeProcessor::HandleURLsModified( } bool TypedUrlChangeProcessor::CreateOrUpdateSyncNode( - const history::URLRow& url, sync_api::WriteTransaction* trans) { + history::URLRow url, sync_api::WriteTransaction* trans) { // Get the visits for this node. history::VisitVector visit_vector; - if (!TypedUrlModelAssociator::GetVisitsForURL( - history_backend_, url, &visit_vector)) { + if (!TypedUrlModelAssociator::FixupURLAndGetVisits( + history_backend_, &url, &visit_vector)) { error_handler()->OnUnrecoverableError(FROM_HERE, "Could not get the url's visits."); return false; @@ -264,8 +264,8 @@ void TypedUrlChangeProcessor::ApplyChangesFromSyncModel( } history::VisitVector visits; - if (!TypedUrlModelAssociator::GetVisitsForURL( - history_backend_, old_url, &visits)) { + if (!TypedUrlModelAssociator::FixupURLAndGetVisits( + history_backend_, &old_url, &visits)) { error_handler()->OnUnrecoverableError(FROM_HERE, "Could not get the url's visits."); return; diff --git a/chrome/browser/sync/glue/typed_url_change_processor.h b/chrome/browser/sync/glue/typed_url_change_processor.h index cc62a48..3219103 100644 --- a/chrome/browser/sync/glue/typed_url_change_processor.h +++ b/chrome/browser/sync/glue/typed_url_change_processor.h @@ -80,7 +80,7 @@ class TypedUrlChangeProcessor : public ChangeProcessor, // Utility routine that either updates an existing sync node or creates a // new one for the passed |typed_url| if one does not already exist. Returns // false and sets an unrecoverable error if the operation failed. - bool CreateOrUpdateSyncNode(const history::URLRow& typed_url, + bool CreateOrUpdateSyncNode(history::URLRow typed_url, sync_api::WriteTransaction* transaction); // The two models should be associated according to this ModelAssociator. diff --git a/chrome/browser/sync/glue/typed_url_model_associator.cc b/chrome/browser/sync/glue/typed_url_model_associator.cc index d2691c1..cdfd779 100644 --- a/chrome/browser/sync/glue/typed_url_model_associator.cc +++ b/chrome/browser/sync/glue/typed_url_model_associator.cc @@ -70,10 +70,11 @@ TypedUrlModelAssociator::~TypedUrlModelAssociator() {} // static -bool TypedUrlModelAssociator::GetVisitsForURL(history::HistoryBackend* backend, - const history::URLRow& url, - history::VisitVector* visits) { - if (!backend->GetMostRecentVisitsForURL(url.id(), kMaxVisitsToFetch, visits)) +bool TypedUrlModelAssociator::FixupURLAndGetVisits( + history::HistoryBackend* backend, + history::URLRow* url, + history::VisitVector* visits) { + if (!backend->GetMostRecentVisitsForURL(url->id(), kMaxVisitsToFetch, visits)) return false; // Sometimes (due to a bug elsewhere in the history or sync code, or due to @@ -83,7 +84,7 @@ bool TypedUrlModelAssociator::GetVisitsForURL(history::HistoryBackend* backend, // This is a workaround for http://crbug.com/84258. if (visits->empty()) { history::VisitRow visit( - url.id(), url.last_visit(), 0, PageTransition::TYPED, 0); + url->id(), url->last_visit(), 0, PageTransition::TYPED, 0); visits->push_back(visit); } @@ -91,11 +92,10 @@ bool TypedUrlModelAssociator::GetVisitsForURL(history::HistoryBackend* backend, // we need it, so reverse it. std::reverse(visits->begin(), visits->end()); - // Checking DB consistency to try to track down http://crbug.com/94733 - if - // we start hitting this DCHECK, we can try to fixup the data by adding our - // own mock visit as we do for empty visit vectors. - DCHECK_EQ(url.last_visit().ToInternalValue(), - visits->back().visit_time.ToInternalValue()); + // Sometimes, the last_visit field in the URL doesn't match the timestamp of + // the last visit in our visit array (they come from different tables, so + // crashes/bugs can cause them to mismatch), so just set it here. + url->set_last_visit(visits->back().visit_time); DCHECK(CheckVisitOrdering(*visits)); return true; } @@ -116,7 +116,8 @@ bool TypedUrlModelAssociator::AssociateModels(SyncError* error) { std::map<history::URLID, history::VisitVector> visit_vectors; for (std::vector<history::URLRow>::iterator ix = typed_urls.begin(); ix != typed_urls.end(); ++ix) { - if (!GetVisitsForURL(history_backend_, *ix, &(visit_vectors[ix->id()]))) { + if (!FixupURLAndGetVisits( + history_backend_, &(*ix), &(visit_vectors[ix->id()]))) { error->Reset(FROM_HERE, "Could not get the url's visits.", model_type()); return false; } @@ -132,9 +133,9 @@ bool TypedUrlModelAssociator::AssociateModels(SyncError* error) { sync_api::ReadNode typed_url_root(&trans); if (!typed_url_root.InitByTagLookup(kTypedUrlTag)) { error->Reset(FROM_HERE, - "Server did not create the top-level typed_url node. We " - "might be running against an out-of-date server.", - model_type()); + "Server did not create the top-level typed_url node. We " + "might be running against an out-of-date server.", + model_type()); return false; } diff --git a/chrome/browser/sync/glue/typed_url_model_associator.h b/chrome/browser/sync/glue/typed_url_model_associator.h index b43b195..c186380 100644 --- a/chrome/browser/sync/glue/typed_url_model_associator.h +++ b/chrome/browser/sync/glue/typed_url_model_associator.h @@ -148,11 +148,12 @@ class TypedUrlModelAssociator // the passed URL. This function compensates for the fact that the history DB // has rather poor data integrity (duplicate visits, visit timestamps that // don't match the last_visit timestamp, huge data sets that exhaust memory - // when fetched, etc). Returns false if we could not fetch the visits for the - // passed URL. - static bool GetVisitsForURL(history::HistoryBackend* backend, - const history::URLRow& url, - history::VisitVector* visits); + // when fetched, etc) by modifying the passed |url| object and |visits| + // vector. + // Returns false if we could not fetch the visits for the passed URL. + static bool FixupURLAndGetVisits(history::HistoryBackend* backend, + history::URLRow* url, + history::VisitVector* visits); // Updates the passed |url_row| based on the values in |specifics|. Fields // that are not contained in |specifics| (such as typed_count) are left |