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

Issue 675992 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

[USS] Autocomplete sync bridge needs to use a StorageKey that is easily reversable

Project Member Reported by s...@chromium.org, Dec 20 2016

Issue description

The whole reason we separated StorageKey and ClientTags was to allow cases like autocomplete to provide different values. ClientTag needs to be backwards compatible (see  issue 675991 ), while StorageKey is all the information we're going to have when reacting to deletes. Slapping a bunch of values into a string with a simple delimiter is not a good approach. We talked in the past about creating a simple proto and serializing to strings.
 

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

Owner: s...@chromium.org
Status: Started (was: Available)

Comment 2 by s...@chromium.org, Dec 23 2016

It looks like net::EscapePath does escape the '|' character (to %7C), so it should be possible to keep things the way they are. Just need to add manual parsing code. Unclear what the correct path forward. I've created https://codereview.chromium.org/2598113002/ to drive conversion.
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 6 2017

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

commit ba936951fafeeb20fe1ecb72e3ec40b98bce9fee
Author: skym <skym@chromium.org>
Date: Fri Jan 06 19:43:15 2017

[Sync] Use a proto to generate AutofillSyncStorageKey's storage keys.

Previously we were going to use a | delimiter and manually construct
storage keys ourselves. The requires us to write logic to reverse it,
and is inefficient when encoding characters that need to be escaped.
The disadvantages are that we always have 4 bytes (instead of 1) of
overhead, and printing the string is less readable.

BUG= 675992 

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

[modify] https://crrev.com/ba936951fafeeb20fe1ecb72e3ec40b98bce9fee/components/autofill/core/browser/proto/BUILD.gn
[add] https://crrev.com/ba936951fafeeb20fe1ecb72e3ec40b98bce9fee/components/autofill/core/browser/proto/autofill_sync.proto
[modify] https://crrev.com/ba936951fafeeb20fe1ecb72e3ec40b98bce9fee/components/autofill/core/browser/webdata/autocomplete_sync_bridge.cc
[modify] https://crrev.com/ba936951fafeeb20fe1ecb72e3ec40b98bce9fee/components/autofill/core/browser/webdata/autocomplete_sync_bridge.h
[modify] https://crrev.com/ba936951fafeeb20fe1ecb72e3ec40b98bce9fee/components/autofill/core/browser/webdata/autocomplete_sync_bridge_unittest.cc

Comment 4 by s...@chromium.org, Jan 11 2017

Status: Fixed (was: Started)

Sign in to add a comment