Correctly handle expired and truncated visits in typed url sync

Changed the TypedUrlModelAssociator to correctly merge changes between the sync and history DB:

1) Now no longer resurrects expired visits (if a visit in the sync DB is older than any visit in the history DB, we ignore it).
2) Does not treat "visit list truncation" as if the truncated visits were deleted (so items omitted from the sync DB because we overflow the 100-visit limit will not also get removed from the history DB).


BUG=101033
TEST=Added new unit tests

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=106854

[Andrew T Wilson (Slow), 2011-10-21 13:14:05]

http://codereview.chromium.org/8289026/diff/4001/chrome/browser/sync/glue/syn...
File chrome/browser/sync/glue/sync_backend_registrar.cc (right):

http://codereview.chromium.org/8289026/diff/4001/chrome/browser/sync/glue/syn...
chrome/browser/sync/glue/sync_backend_registrar.cc:126: LOG(WARNING) << "No
password worker -- removing PASSWORDS";
This copy/paste error keeps bugging me, so I fixed it to be more correct.

[Andrew T Wilson (Slow), 2011-10-21 16:58:05]

PTAL

[Nicolas Zea, 2011-10-21 22:07:28]

LGTM with nits

http://codereview.chromium.org/8289026/diff/6001/chrome/browser/sync/glue/typ...
File chrome/browser/sync/glue/typed_url_change_processor.cc (right):

http://codereview.chromium.org/8289026/diff/6001/chrome/browser/sync/glue/typ...
chrome/browser/sync/glue/typed_url_change_processor.cc:99:
DCHECK(url.typed_count() > 0);
DCHECK_GT

http://codereview.chromium.org/8289026/diff/6001/chrome/browser/sync/glue/typ...
File chrome/browser/sync/glue/typed_url_model_associator.cc (right):

http://codereview.chromium.org/8289026/diff/6001/chrome/browser/sync/glue/typ...
chrome/browser/sync/glue/typed_url_model_associator.cc:123:
DCHECK(visit_vectors.find(ix->id()) == visit_vectors.end());
DCHECK_EQ (assuming there's no operator== magic going on here)

http://codereview.chromium.org/8289026/diff/6001/chrome/browser/sync/glue/typ...
chrome/browser/sync/glue/typed_url_model_associator.cc:735: if (new_index > 0) {
merge this into parent if statement? if (old_visits... && new_index > 0)

[Andrew T Wilson (Slow), 2011-10-21 22:51:05]

http://codereview.chromium.org/8289026/diff/6001/chrome/browser/sync/glue/typ...
File chrome/browser/sync/glue/typed_url_change_processor.cc (right):

http://codereview.chromium.org/8289026/diff/6001/chrome/browser/sync/glue/typ...
chrome/browser/sync/glue/typed_url_change_processor.cc:99:
DCHECK(url.typed_count() > 0);
On 2011/10/21 22:07:29, nzea wrote:
> DCHECK_GT

Done.

http://codereview.chromium.org/8289026/diff/6001/chrome/browser/sync/glue/typ...
File chrome/browser/sync/glue/typed_url_model_associator.cc (right):

http://codereview.chromium.org/8289026/diff/6001/chrome/browser/sync/glue/typ...
chrome/browser/sync/glue/typed_url_model_associator.cc:123:
DCHECK(visit_vectors.find(ix->id()) == visit_vectors.end());
On 2011/10/21 22:07:29, nzea wrote:
> DCHECK_EQ (assuming there's no operator== magic going on here)

Done.

http://codereview.chromium.org/8289026/diff/6001/chrome/browser/sync/glue/typ...
chrome/browser/sync/glue/typed_url_model_associator.cc:735: if (new_index > 0) {
On 2011/10/21 22:07:29, nzea wrote:
> merge this into parent if statement? if (old_visits... && new_index > 0)

I think this will break the code because it will give the else clause a chance
to execute if new_index == 0, which we don't want. So I'm going to leave this
as-is.

[commit-bot: I haz the power, 2011-10-22 07:31:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/atwilson@chromium.org/8289026/12006