New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 672619 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature



Sign in to add a comment

[USS] Implement AutocompleteSyncBridge::ApplySyncChanges

Project Member Reported by maxbogue@chromium.org, Dec 8 2016

Issue description

Comment 1 by s...@chromium.org, Dec 8 2016

Status: Started (was: Assigned)
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0299ea23ebf462d0bec0d9571711b8d6c437cb8c

commit 0299ea23ebf462d0bec0d9571711b8d6c437cb8c
Author: skym <skym@chromium.org>
Date: Fri Jan 13 17:48:53 2017

[Sync] ApplySyncChanges autofill implementation.

Updates autofill USS implementation to accept incoming sync changes. The
previous implementation can be found at
AutocompleteSyncableService::ProcessSyncChanges, which was used to base
the behavior off of for this CL. I've tried to setup the code in such a
way that allows apply and merge to share as much code as possible. I
tried to keep the overhead/complexity to a minimum, not quite sure how
successful I have been. Some of the odd/gnarly pieces of logic that
were (or not) copied over that I think may be worth calling out:

1. AutofillTable::GetAllAutofillEntries is only called if needed. While
the processor shouldn't call us with no changes for ApplySyncChanges,
it's up to us to avoid this when we're only receiving deletes. This is
the previous behavior.
2. AutofillSpecifics should be ignored when .has_value() returns false.
These are dropped on the floor with no attempt to delete. As far back
as I could find, like over 5 years ago, this has been the case. No
special handling exists for .has_name(), which can return false and
we'll treat it as normal. This is the previous behavior.
3. Don't trust the incoming time stamps from Sync. This is a change
from how AutocompleteSyncableService was implemented. In an attempt to
be conservative in what we send, and liberal in what we accept, I've
tried to add the ability to handle incorrectly ordered timestamps, and
correct them locally. The cost of this should be negligible since
typically there should be 1 or 2 timestmaps on any given remote
specifics.
4. Converting from an AutofillSpecifics with no timestamps is kind of
invalid, we end up setting our creation timestamp to time 0. We
explicitly avoid this scenario when updating existing data, but when
the sync change is the first time we've seen this data, we don't have
quite as reasonable of an option, so we set creation and last use time
to time 0. This is going to result in an immediate eviction by
RemoveExpiredFormElements, but this the previous behavior, so I kept
it.

BUG= 672619 

Review-Url: https://codereview.chromium.org/2624883002
Cr-Commit-Position: refs/heads/master@{#443601}

[modify] https://crrev.com/0299ea23ebf462d0bec0d9571711b8d6c437cb8c/components/autofill/core/browser/webdata/autocomplete_sync_bridge.cc
[modify] https://crrev.com/0299ea23ebf462d0bec0d9571711b8d6c437cb8c/components/autofill/core/browser/webdata/autocomplete_sync_bridge.h
[modify] https://crrev.com/0299ea23ebf462d0bec0d9571711b8d6c437cb8c/components/autofill/core/browser/webdata/autocomplete_sync_bridge_unittest.cc
[modify] https://crrev.com/0299ea23ebf462d0bec0d9571711b8d6c437cb8c/components/autofill/core/browser/webdata/autofill_entry.cc
[modify] https://crrev.com/0299ea23ebf462d0bec0d9571711b8d6c437cb8c/components/autofill/core/browser/webdata/autofill_entry.h
[modify] https://crrev.com/0299ea23ebf462d0bec0d9571711b8d6c437cb8c/components/autofill/core/browser/webdata/autofill_metadata_change_list.cc
[modify] https://crrev.com/0299ea23ebf462d0bec0d9571711b8d6c437cb8c/components/autofill/core/browser/webdata/autofill_metadata_change_list.h

Comment 3 by s...@chromium.org, Mar 10 2017

Status: Fixed (was: Started)

Sign in to add a comment