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

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , iOS , Chrome , Mac , Fuchsia
Pri: 2
Type: Feature

Blocked on:
issue 928421

Blocking:
issue 923287



Sign in to add a comment
link

Issue 902349: Sync: Passwords migration to full-blown USS

Reported by mamir@chromium.org, Nov 6 Project Member

Issue description

Tracking issue for design/implementation changes needed to migrate Passwords to use the new USS architecture.
 

Comment 1 by mamir@chromium.org, Nov 6

Labels: OS-Android OS-Chrome OS-Fuchsia OS-iOS OS-Linux OS-Mac OS-Windows

Comment 2 by bugdroid1@chromium.org, Nov 7

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/044528b4da710fe6e31b1e11070909abdda3d00d

commit 044528b4da710fe6e31b1e11070909abdda3d00d
Author: Mohamed Amir Yosef <mamir@chromium.org>
Date: Wed Nov 07 09:27:33 2018

[Sync::USS] Rename PasswordModelTypeController class

This CL renames PasswordModelTypeController class.
Currently it is used for the Pseudo USS implementation as an intermediate
stage till releasing the full-blown USS implementation for passwords.

We should reserve that name for the full-blown implementation.

Bug: 902349
Change-Id: I01b9df1e9e4afcb0307f58a95172769a9f249f18
Reviewed-on: https://chromium-review.googlesource.com/c/1320170
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606001}
[modify] https://crrev.com/044528b4da710fe6e31b1e11070909abdda3d00d/components/browser_sync/profile_sync_components_factory_impl.cc
[modify] https://crrev.com/044528b4da710fe6e31b1e11070909abdda3d00d/components/password_manager/core/browser/BUILD.gn
[rename] https://crrev.com/044528b4da710fe6e31b1e11070909abdda3d00d/components/password_manager/core/browser/password_syncable_service_based_model_type_controller.cc
[rename] https://crrev.com/044528b4da710fe6e31b1e11070909abdda3d00d/components/password_manager/core/browser/password_syncable_service_based_model_type_controller.h

Comment 3 by bugdroid1@chromium.org, Nov 8

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/59fd8e313ad4b19652bf69a435457da8e6c99ca5

commit 59fd8e313ad4b19652bf69a435457da8e6c99ca5
Author: Mohamed Amir Yosef <mamir@chromium.org>
Date: Thu Nov 08 20:45:18 2018

[Sync::USS] Move password sync related classes to /sync sub-folder

Bug: 902349
Change-Id: I565b746737cc1567cf75abf4924c77eb648c3b46
Reviewed-on: https://chromium-review.googlesource.com/c/1322869
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606595}
[modify] https://crrev.com/59fd8e313ad4b19652bf69a435457da8e6c99ca5/chrome/browser/sync/chrome_sync_client.cc
[modify] https://crrev.com/59fd8e313ad4b19652bf69a435457da8e6c99ca5/components/browser_sync/profile_sync_components_factory_impl.cc
[modify] https://crrev.com/59fd8e313ad4b19652bf69a435457da8e6c99ca5/components/password_manager/core/browser/BUILD.gn
[modify] https://crrev.com/59fd8e313ad4b19652bf69a435457da8e6c99ca5/components/password_manager/core/browser/password_store.cc
[add] https://crrev.com/59fd8e313ad4b19652bf69a435457da8e6c99ca5/components/password_manager/core/browser/sync/OWNERS
[rename] https://crrev.com/59fd8e313ad4b19652bf69a435457da8e6c99ca5/components/password_manager/core/browser/sync/password_data_type_controller.cc
[rename] https://crrev.com/59fd8e313ad4b19652bf69a435457da8e6c99ca5/components/password_manager/core/browser/sync/password_data_type_controller.h
[rename] https://crrev.com/59fd8e313ad4b19652bf69a435457da8e6c99ca5/components/password_manager/core/browser/sync/password_model_worker.cc
[rename] https://crrev.com/59fd8e313ad4b19652bf69a435457da8e6c99ca5/components/password_manager/core/browser/sync/password_model_worker.h
[rename] https://crrev.com/59fd8e313ad4b19652bf69a435457da8e6c99ca5/components/password_manager/core/browser/sync/password_syncable_service.cc
[rename] https://crrev.com/59fd8e313ad4b19652bf69a435457da8e6c99ca5/components/password_manager/core/browser/sync/password_syncable_service.h
[rename] https://crrev.com/59fd8e313ad4b19652bf69a435457da8e6c99ca5/components/password_manager/core/browser/sync/password_syncable_service_based_model_type_controller.cc
[rename] https://crrev.com/59fd8e313ad4b19652bf69a435457da8e6c99ca5/components/password_manager/core/browser/sync/password_syncable_service_based_model_type_controller.h
[rename] https://crrev.com/59fd8e313ad4b19652bf69a435457da8e6c99ca5/components/password_manager/core/browser/sync/password_syncable_service_unittest.cc
[modify] https://crrev.com/59fd8e313ad4b19652bf69a435457da8e6c99ca5/ios/chrome/browser/sync/ios_chrome_sync_client.mm
[modify] https://crrev.com/59fd8e313ad4b19652bf69a435457da8e6c99ca5/ios/web_view/internal/sync/web_view_sync_client.mm

Comment 4 by bugdroid1@chromium.org, Nov 8

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/59fd8e313ad4b19652bf69a435457da8e6c99ca5

commit 59fd8e313ad4b19652bf69a435457da8e6c99ca5
Author: Mohamed Amir Yosef <mamir@chromium.org>
Date: Thu Nov 08 20:45:18 2018

[Sync::USS] Move password sync related classes to /sync sub-folder

Bug: 902349
Change-Id: I565b746737cc1567cf75abf4924c77eb648c3b46
Reviewed-on: https://chromium-review.googlesource.com/c/1322869
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606595}
[modify] https://crrev.com/59fd8e313ad4b19652bf69a435457da8e6c99ca5/chrome/browser/sync/chrome_sync_client.cc
[modify] https://crrev.com/59fd8e313ad4b19652bf69a435457da8e6c99ca5/components/browser_sync/profile_sync_components_factory_impl.cc
[modify] https://crrev.com/59fd8e313ad4b19652bf69a435457da8e6c99ca5/components/password_manager/core/browser/BUILD.gn
[modify] https://crrev.com/59fd8e313ad4b19652bf69a435457da8e6c99ca5/components/password_manager/core/browser/password_store.cc
[add] https://crrev.com/59fd8e313ad4b19652bf69a435457da8e6c99ca5/components/password_manager/core/browser/sync/OWNERS
[rename] https://crrev.com/59fd8e313ad4b19652bf69a435457da8e6c99ca5/components/password_manager/core/browser/sync/password_data_type_controller.cc
[rename] https://crrev.com/59fd8e313ad4b19652bf69a435457da8e6c99ca5/components/password_manager/core/browser/sync/password_data_type_controller.h
[rename] https://crrev.com/59fd8e313ad4b19652bf69a435457da8e6c99ca5/components/password_manager/core/browser/sync/password_model_worker.cc
[rename] https://crrev.com/59fd8e313ad4b19652bf69a435457da8e6c99ca5/components/password_manager/core/browser/sync/password_model_worker.h
[rename] https://crrev.com/59fd8e313ad4b19652bf69a435457da8e6c99ca5/components/password_manager/core/browser/sync/password_syncable_service.cc
[rename] https://crrev.com/59fd8e313ad4b19652bf69a435457da8e6c99ca5/components/password_manager/core/browser/sync/password_syncable_service.h
[rename] https://crrev.com/59fd8e313ad4b19652bf69a435457da8e6c99ca5/components/password_manager/core/browser/sync/password_syncable_service_based_model_type_controller.cc
[rename] https://crrev.com/59fd8e313ad4b19652bf69a435457da8e6c99ca5/components/password_manager/core/browser/sync/password_syncable_service_based_model_type_controller.h
[rename] https://crrev.com/59fd8e313ad4b19652bf69a435457da8e6c99ca5/components/password_manager/core/browser/sync/password_syncable_service_unittest.cc
[modify] https://crrev.com/59fd8e313ad4b19652bf69a435457da8e6c99ca5/ios/chrome/browser/sync/ios_chrome_sync_client.mm
[modify] https://crrev.com/59fd8e313ad4b19652bf69a435457da8e6c99ca5/ios/web_view/internal/sync/web_view_sync_client.mm

Comment 5 by bugdroid1@chromium.org, Nov 9

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/194abb348b6be616fd50fc1a91f6026e7c6e8932

commit 194abb348b6be616fd50fc1a91f6026e7c6e8932
Author: Mohamed Amir Yosef <mamir@chromium.org>
Date: Fri Nov 09 08:07:13 2018

[Sync::USS] Introducing PasswordSyncBridge

This CL adds the skeleton of the PasswordSyncBridge and plumps it into
PasswordStore.
In addition it also include the feature toggle for Passwords USS
migration.

Bug: 902349
Change-Id: I343f2bb83ecf2c993bacab9e299e7d0deea868b2
Reviewed-on: https://chromium-review.googlesource.com/c/1323653
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606766}
[modify] https://crrev.com/194abb348b6be616fd50fc1a91f6026e7c6e8932/components/password_manager/core/browser/BUILD.gn
[modify] https://crrev.com/194abb348b6be616fd50fc1a91f6026e7c6e8932/components/password_manager/core/browser/DEPS
[modify] https://crrev.com/194abb348b6be616fd50fc1a91f6026e7c6e8932/components/password_manager/core/browser/password_store.cc
[modify] https://crrev.com/194abb348b6be616fd50fc1a91f6026e7c6e8932/components/password_manager/core/browser/password_store.h
[add] https://crrev.com/194abb348b6be616fd50fc1a91f6026e7c6e8932/components/password_manager/core/browser/sync/password_sync_bridge.cc
[add] https://crrev.com/194abb348b6be616fd50fc1a91f6026e7c6e8932/components/password_manager/core/browser/sync/password_sync_bridge.h
[modify] https://crrev.com/194abb348b6be616fd50fc1a91f6026e7c6e8932/components/sync/driver/sync_driver_switches.cc
[modify] https://crrev.com/194abb348b6be616fd50fc1a91f6026e7c6e8932/components/sync/driver/sync_driver_switches.h

Comment 6 by bugdroid1@chromium.org, Nov 14

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/76ceba075153b24ae7b5d0b043fca5bd00f19694

commit 76ceba075153b24ae7b5d0b043fca5bd00f19694
Author: Mohamed Amir Yosef <mamir@chromium.org>
Date: Wed Nov 14 16:52:19 2018

[Sync::USS] Introducing PasswordModelTypeController

Bug: 902349
Change-Id: Ic8f20ba353bf9805d79916f440fe10137a97aff7
Reviewed-on: https://chromium-review.googlesource.com/c/1325981
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608009}
[modify] https://crrev.com/76ceba075153b24ae7b5d0b043fca5bd00f19694/components/browser_sync/profile_sync_components_factory_impl.cc
[modify] https://crrev.com/76ceba075153b24ae7b5d0b043fca5bd00f19694/components/password_manager/core/browser/BUILD.gn
[modify] https://crrev.com/76ceba075153b24ae7b5d0b043fca5bd00f19694/components/password_manager/core/browser/password_store.cc
[modify] https://crrev.com/76ceba075153b24ae7b5d0b043fca5bd00f19694/components/password_manager/core/browser/password_store.h
[add] https://crrev.com/76ceba075153b24ae7b5d0b043fca5bd00f19694/components/password_manager/core/browser/sync/password_model_type_controller.cc
[add] https://crrev.com/76ceba075153b24ae7b5d0b043fca5bd00f19694/components/password_manager/core/browser/sync/password_model_type_controller.h
[modify] https://crrev.com/76ceba075153b24ae7b5d0b043fca5bd00f19694/components/sync/model_impl/client_tag_based_model_type_processor.cc

Comment 7 by bugdroid1@chromium.org, Nov 27

Project Member

Comment 8 by bugdroid1@chromium.org, Nov 28

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8ea63a762c64a985273fe6959358ded96efe931e

commit 8ea63a762c64a985273fe6959358ded96efe931e
Author: Mohamed Amir Yosef <mamir@chromium.org>
Date: Wed Nov 28 09:54:21 2018

[Sync::USS] Encrypt passwords in NonBlockingTypeCommitContribution

This is necessary for full-blown USS.

Bug: 902349
Change-Id: I1de4a3ac610151feb236c2f1696ba22b9f047568
Reviewed-on: https://chromium-review.googlesource.com/c/1336129
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611620}
[modify] https://crrev.com/8ea63a762c64a985273fe6959358ded96efe931e/components/sync/engine_impl/non_blocking_type_commit_contribution.cc
[modify] https://crrev.com/8ea63a762c64a985273fe6959358ded96efe931e/components/sync/engine_impl/non_blocking_type_commit_contribution.h

Comment 9 by bugdroid1@chromium.org, Jan 8

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2c65b1d886772e64b039a0dbbb65ef11268b4f88

commit 2c65b1d886772e64b039a0dbbb65ef11268b4f88
Author: Mohamed Amir Yosef <mamir@chromium.org>
Date: Tue Jan 08 14:49:29 2019

[Sync:USS] Add primary key to LoginDatabase

LoginDatabase doesn't have a primary key, however migration to USS
requires the concept of storage key which is ideally the primary key
of the table.

LoginDatabase uses SQLTableBuilder to migrate from different database
versions. SQLTableBuilder migration doesn't support adding primary keys
at arbitrary version, rather only in the first version.

This CL adds the support to introduce primary keys and unique keys at
any version and adjusts the migration code accordingly.
In addition, it uses the updated SQLTableBuilder to add a primary key
to the LoginDatabase.

Bug: 902349

Change-Id: I08f0d6f9884c5ef2b99650a76716197ca4f18b9c
Reviewed-on: https://chromium-review.googlesource.com/c/1361861
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620713}
[modify] https://crrev.com/2c65b1d886772e64b039a0dbbb65ef11268b4f88/components/password_manager/core/browser/BUILD.gn
[modify] https://crrev.com/2c65b1d886772e64b039a0dbbb65ef11268b4f88/components/password_manager/core/browser/login_database.cc
[modify] https://crrev.com/2c65b1d886772e64b039a0dbbb65ef11268b4f88/components/password_manager/core/browser/login_database.h
[modify] https://crrev.com/2c65b1d886772e64b039a0dbbb65ef11268b4f88/components/password_manager/core/browser/login_database_ios.cc
[modify] https://crrev.com/2c65b1d886772e64b039a0dbbb65ef11268b4f88/components/password_manager/core/browser/login_database_unittest.cc
[modify] https://crrev.com/2c65b1d886772e64b039a0dbbb65ef11268b4f88/components/password_manager/core/browser/sql_table_builder.cc
[modify] https://crrev.com/2c65b1d886772e64b039a0dbbb65ef11268b4f88/components/password_manager/core/browser/sql_table_builder.h
[modify] https://crrev.com/2c65b1d886772e64b039a0dbbb65ef11268b4f88/components/password_manager/core/browser/sql_table_builder_unittest.cc
[add] https://crrev.com/2c65b1d886772e64b039a0dbbb65ef11268b4f88/components/test/data/password_manager/login_db_v20.sql

Comment 10 by bugdroid1@chromium.org, Jan 8

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

commit fa68f994f0a0838447d9762f50a8190e78a366f2
Author: Mohamed Amir Yosef <mamir@chromium.org>
Date: Tue Jan 08 17:05:35 2019

[Sync::USS] Introduce Sync metadata tables in LoginDatabase

In order to migrate Passwords to the new USS architecture, sync
metadata should be stored together with passoword's model data in the
same database. This is to achieve atomic writes of the data and the
metadata.

This CL is introducing the tables required for storing the per-entitiy
and per-model sync metadata tables. (namely sync_entities_metadata and
sync_model_metadata respectively).

Bug: 902349
Change-Id: Ic1e5cd30880757c4ebb5e312ffe186fdadc9b25b
Reviewed-on: https://chromium-review.googlesource.com/c/1369188
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620763}
[modify] https://crrev.com/fa68f994f0a0838447d9762f50a8190e78a366f2/components/password_manager/core/browser/BUILD.gn
[modify] https://crrev.com/fa68f994f0a0838447d9762f50a8190e78a366f2/components/password_manager/core/browser/login_database.cc
[modify] https://crrev.com/fa68f994f0a0838447d9762f50a8190e78a366f2/components/password_manager/core/browser/sql_table_builder.cc
[add] https://crrev.com/fa68f994f0a0838447d9762f50a8190e78a366f2/components/test/data/password_manager/login_db_v21.sql

Comment 11 by bugdroid1@chromium.org, Jan 8

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1b5f7297679474ee41d6b1bca4d90f4189727fe7

commit 1b5f7297679474ee41d6b1bca4d90f4189727fe7
Author: Mohamed Amir Yosef <mamir@chromium.org>
Date: Tue Jan 08 18:20:36 2019

[Sync::USS] LoginDatabase should implement syncer::SyncMetadataStore

USS bridge require a sync metadata store to read and store the sync
metadata.
LoginDatabase is the best fit for this for Passwords data type.
This CL adds implements syncer::SyncMetadataStore in
LoginDatabase.

Bug: 902349
Change-Id: I1eef9b5aa3bbf99f8d7c5ad1fc20884781ca257b
Reviewed-on: https://chromium-review.googlesource.com/c/1371901
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620799}
[modify] https://crrev.com/1b5f7297679474ee41d6b1bca4d90f4189727fe7/components/password_manager/core/browser/login_database.cc
[modify] https://crrev.com/1b5f7297679474ee41d6b1bca4d90f4189727fe7/components/password_manager/core/browser/login_database.h
[modify] https://crrev.com/1b5f7297679474ee41d6b1bca4d90f4189727fe7/components/password_manager/core/browser/login_database_unittest.cc
[modify] https://crrev.com/1b5f7297679474ee41d6b1bca4d90f4189727fe7/components/sync/model/metadata_batch.cc

Comment 12 by bugdroid1@chromium.org, Jan 9

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0fad71db499e559926c900c6e5c2a90be5384c93

commit 0fad71db499e559926c900c6e5c2a90be5384c93
Author: Mohamed Amir Yosef <mamir@chromium.org>
Date: Wed Jan 09 14:09:05 2019

[Sync::USS] Implement PasswordSyncBridge::GetStorageKey()

Bug: 902349
Change-Id: If5d3ff4cfee29561e735af66cca2453a104dba32
Reviewed-on: https://chromium-review.googlesource.com/c/1402444
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621136}
[modify] https://crrev.com/0fad71db499e559926c900c6e5c2a90be5384c93/components/password_manager/core/browser/sync/password_sync_bridge.cc
[modify] https://crrev.com/0fad71db499e559926c900c6e5c2a90be5384c93/components/password_manager/core/browser/sync/password_sync_bridge.h

Comment 13 by bugdroid1@chromium.org, Jan 10

Project Member

Comment 14 by bugdroid1@chromium.org, Jan 14

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/991e5636b44bb07fcc67186aff6302b5b68a1798

commit 991e5636b44bb07fcc67186aff6302b5b68a1798
Author: Mohamed Amir Yosef <mamir@chromium.org>
Date: Mon Jan 14 14:43:57 2019

[Sync::USS] Add transaction support to PasswordSyncStore

USS requires atomic storage of model data and sync metadata. This CL
add transaction support to the PasswordSyncStore to fulfill this
requirement.

Bug: 902349
Change-Id: I6d564d0e579d95ca08915daf802491c69019fe74
Reviewed-on: https://chromium-review.googlesource.com/c/1396034
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622450}
[modify] https://crrev.com/991e5636b44bb07fcc67186aff6302b5b68a1798/components/password_manager/core/browser/login_database.cc
[modify] https://crrev.com/991e5636b44bb07fcc67186aff6302b5b68a1798/components/password_manager/core/browser/login_database.h
[modify] https://crrev.com/991e5636b44bb07fcc67186aff6302b5b68a1798/components/password_manager/core/browser/mock_password_store.h
[modify] https://crrev.com/991e5636b44bb07fcc67186aff6302b5b68a1798/components/password_manager/core/browser/password_store.cc
[modify] https://crrev.com/991e5636b44bb07fcc67186aff6302b5b68a1798/components/password_manager/core/browser/password_store.h
[modify] https://crrev.com/991e5636b44bb07fcc67186aff6302b5b68a1798/components/password_manager/core/browser/password_store_default.cc
[modify] https://crrev.com/991e5636b44bb07fcc67186aff6302b5b68a1798/components/password_manager/core/browser/password_store_default.h
[modify] https://crrev.com/991e5636b44bb07fcc67186aff6302b5b68a1798/components/password_manager/core/browser/password_store_sync.h
[modify] https://crrev.com/991e5636b44bb07fcc67186aff6302b5b68a1798/components/password_manager/core/browser/test_password_store.cc
[modify] https://crrev.com/991e5636b44bb07fcc67186aff6302b5b68a1798/components/password_manager/core/browser/test_password_store.h

Comment 15 by bugdroid1@chromium.org, Jan 14

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

commit fccd9c66115a1e55a3c45d081dcb3712f0c70a15
Author: Mohamed Amir Yosef <mamir@chromium.org>
Date: Mon Jan 14 20:50:17 2019

[Sync::USS] Expose the MetadataStore in PasswordStoreSync interface

This CL exposes the MetadataStore in PasswordStoreSync interface.
This will be useful later because the PasswordSyncBridge will require
a MetadataStore to persist the metadata.

Bug: 902349
Change-Id: I603299846828246a23822ca3a39126c3b4de0f61
Reviewed-on: https://chromium-review.googlesource.com/c/1401047
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622594}
[modify] https://crrev.com/fccd9c66115a1e55a3c45d081dcb3712f0c70a15/components/password_manager/core/browser/mock_password_store.h
[modify] https://crrev.com/fccd9c66115a1e55a3c45d081dcb3712f0c70a15/components/password_manager/core/browser/password_store_default.cc
[modify] https://crrev.com/fccd9c66115a1e55a3c45d081dcb3712f0c70a15/components/password_manager/core/browser/password_store_default.h
[modify] https://crrev.com/fccd9c66115a1e55a3c45d081dcb3712f0c70a15/components/password_manager/core/browser/password_store_sync.h
[modify] https://crrev.com/fccd9c66115a1e55a3c45d081dcb3712f0c70a15/components/password_manager/core/browser/test_password_store.cc
[modify] https://crrev.com/fccd9c66115a1e55a3c45d081dcb3712f0c70a15/components/password_manager/core/browser/test_password_store.h

Comment 16 by bugdroid1@chromium.org, Jan 17

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/15cf96733defc7816f5bdd495711fe1c501c2d64

commit 15cf96733defc7816f5bdd495711fe1c501c2d64
Author: Mohamed Amir Yosef <mamir@chromium.org>
Date: Thu Jan 17 15:27:18 2019

[Sync:USS] LoginDatabase::RemoveLogins*() should return a ChangeList

This is a mechanical change that only moves code around to have a more
consistent LoginDatabase API.

For consistency with AddLogin and UpdateLogin, RemoveLogin* methods
should return a PasswordStoreChangeList. This is in preparation for
a later patch that would add the primary key to the
PasswordStoreChangeList.

In addition, this changes makes GetLoginsSyncedBetween() private
and replaces push_back with emplace_back to save moving the relatively
large struct PasswordStoreChangeList as much as possible.

Bug: 902349
Change-Id: Ided5923fbd95768ae3f91179d1dc3648725cbe0b
Reviewed-on: https://chromium-review.googlesource.com/c/1409550
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623690}
[modify] https://crrev.com/15cf96733defc7816f5bdd495711fe1c501c2d64/chrome/browser/password_manager/password_store_mac.cc
[modify] https://crrev.com/15cf96733defc7816f5bdd495711fe1c501c2d64/components/password_manager/core/browser/login_database.cc
[modify] https://crrev.com/15cf96733defc7816f5bdd495711fe1c501c2d64/components/password_manager/core/browser/login_database.h
[modify] https://crrev.com/15cf96733defc7816f5bdd495711fe1c501c2d64/components/password_manager/core/browser/login_database_ios_unittest.cc
[modify] https://crrev.com/15cf96733defc7816f5bdd495711fe1c501c2d64/components/password_manager/core/browser/login_database_unittest.cc
[modify] https://crrev.com/15cf96733defc7816f5bdd495711fe1c501c2d64/components/password_manager/core/browser/password_store_default.cc

Comment 19 by mastiz@chromium.org, Jan 18

Cc: mastiz@chromium.org
 Issue 856941  has been merged into this issue.

Comment 20 by a-...@yandex-team.ru, Jan 18

Cc: a-...@yandex-team.ru

Comment 21 by mastiz@chromium.org, Jan 18

Blocking: 923287

Comment 22 by bugdroid1@chromium.org, Jan 22

Project Member

Comment 23 by bugdroid, Jan 23

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9380cfbeb8c309e5bc54eafd810689b056c793e8

commit 9380cfbeb8c309e5bc54eafd810689b056c793e8
Author: Mohamed Amir Yosef <mamir@chromium.org>
Date: Wed Jan 23 16:36:38 2019

[Sync::USS] Introduce PasswordSyncStore:ReadAllLogins()

PasswordSyncableService code used two methods in PasswordSyncStore
to retrieve all passwords FillAutofillableLogins() and
FillBlacklistLogins() and then concatenated the results.
This has three issues:
1- This unnecessarily hit the DB twice.
2- Each DB hit involved sorting over some string field that isn't
required.
3-Those calls don't expose the DB primary key of each form which
is required by the new sync architecture.

This CL is replace those APIs in the PasswordSyncStore with a single
API that returns all logins together with the corresponding DB primary
key.

This is in preparation for a later CL that would actually use this API
in the PasswordSyncBridge that belongs to the new sync architecture.

Since the methods FillAutofillableLogins() and FillBlacklistLogins()
are used in the password manager codebase, they have been moved to be
part of the PasswordStore interface rather than the PasswordStoreSync

Bug: 902349
Change-Id: Ic94b83fcc24dfc1833e07653426c064292be5ba7
Reviewed-on: https://chromium-review.googlesource.com/c/1424941
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#625225}

Comment 24 by bugdroid, Jan 23

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/541d52d86789121c42be511c5c38347bc727803e

commit 541d52d86789121c42be511c5c38347bc727803e
Author: Mohamed Amir Yosef <mamir@chromium.org>
Date: Wed Jan 23 18:25:34 2019

[Sync::USS] Introduce PasswordStoreSync::RemoveLoginByPrimaryKeySync()

This patch introduces a new method to remove logins given the DB
primary key.

This will be used in later patches.

Bug: 902349
Change-Id: I9c2bf659dbb9e4710ef1469b0296b836a0212301
Reviewed-on: https://chromium-review.googlesource.com/c/1430008
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#625267}

Comment 25 by bugdroid, Jan 25

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

commit f2bff73ff3861f8acc968a6be7c7879d51638e80
Author: Mohamed Amir Yosef <mamir@chromium.org>
Date: Fri Jan 25 13:48:17 2019

[Sync::USS] Add NOTIMPLEMENTED methods to PasswordStoreX

Recently introduced methods in PasswordStoreSync interface are
implemented in PasswordStoreDefault.
However, this implementation isn't compatible with PasswordStoreX.

This overrides such methods in PasswordStoreX with a NOTIMPLEMENTED()
implementation to make sure they aren't called by mistake.

This should be OK because those methods are only used in Sync code base
which doesn't use PasswordStoreX.
In addition, PasswordStoreX will be deprecated.

Similarly implementation in used in TestPasswordStore because it
doesn't support primary key for forms. This is also OK because  Sync
unittest is using a mock PasswordSyncStore rather than
a TestPasswordStore.

Bug: 902349
Change-Id: Id795f3f6ae6b0efa75bbb22dc1675f3c12210692
Reviewed-on: https://chromium-review.googlesource.com/c/1429964
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626067}

Comment 26 by bugdroid, Jan 25

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/771dd2a0c9992bb88fe899b1c70dc3f277499ad6

commit 771dd2a0c9992bb88fe899b1c70dc3f277499ad6
Author: Mohamed Amir Yosef <mamir@chromium.org>
Date: Fri Jan 25 16:47:00 2019

[Sync::USS] Implement PasswordSyncBridge::GetData()

Bug: 902349
Change-Id: I14ca95a063835d72ca48d9c49c4a45f816e5fb55
Reviewed-on: https://chromium-review.googlesource.com/c/1424881
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626097}

Comment 27 by bugdroid, Jan 28

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

commit c0ca7c64634075b446aa7c21e9b83d5993afe055
Author: Mohamed Amir Yosef <mamir@chromium.org>
Date: Mon Jan 28 11:47:20 2019

Implement PasswordSyncBridge::MergeSyncData()

Bug: 902349
Change-Id: I4b01ff184fc57d68a7944421aaace1d475df89a7
Reviewed-on: https://chromium-review.googlesource.com/c/1437184
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626495}

Comment 28 by bugdroid, Jan 28

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7d8f4e2b2ac2c5e7e67e88bfa92a7a6025ae9925

commit 7d8f4e2b2ac2c5e7e67e88bfa92a7a6025ae9925
Author: Mohamed Amir Yosef <mamir@chromium.org>
Date: Mon Jan 28 12:07:31 2019

[Sync::USS] Implement PasswordSyncBridge::GetAllDataForDebugging()

Bug: 902349
Change-Id: I443f2f1fde84edc612efe505d837a25144f0496e
Reviewed-on: https://chromium-review.googlesource.com/c/1437191
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626500}

Comment 29 by bugdroid, Jan 29

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/645e596ffd4956c250dfaa7ee37e5f962731c068

commit 645e596ffd4956c250dfaa7ee37e5f962731c068
Author: Mohamed Amir Yosef <mamir@chromium.org>
Date: Tue Jan 29 08:51:10 2019

[Sync::USS] Update ShouldGetAllDataForDebuggingWithHiddenPassword test

This CL updates ShouldGetAllDataForDebuggingWithHiddenPassword test
in PasswordSyncBridgeTest to make sure it doesn't pass if the bridge
returns an empty batch.

Bug: 902349
Change-Id: I089c2e6a7be3487c9b19d39896fcea5503ab74f6
Reviewed-on: https://chromium-review.googlesource.com/c/1439539
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626934}
[modify] https://crrev.com/645e596ffd4956c250dfaa7ee37e5f962731c068/components/password_manager/core/browser/sync/password_sync_bridge_unittest.cc

Comment 30 by bugdroid, Jan 30

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/57729435d4d16fa543ace2545ca24fc00c4e9bf7

commit 57729435d4d16fa543ace2545ca24fc00c4e9bf7
Author: Mohamed Amir Yosef <mamir@chromium.org>
Date: Wed Jan 30 11:19:39 2019

[Sync::USS] PasswordStoreX supports USS when there is no backend

PasswordStoreX has 3 methods that are specific to Sync USS.
Those method are currently marked as NOIMPLEMENTED because USS doesn't
support PasswordStoreX and the plan is to migrate away from
PasswordStoreX before launching USS for Passwords.

This CL changes this behavior such that in case there is no backend,
PasswordStoreX will delegate those methods to PasswordStoreDefault which
fully supports USS. This behavior is similar to other methods in
PasswordStoreX.

Same as of before this CL, it's the responsibility of Sync code base to
make sure those methods are called only when it makes sense.

This change will enable to run the sync integration tests on Linux
because they don't instantiate a backend.

Bug: 902349
Change-Id: Ie1976e3521300829d31094324832a33382f19306
Reviewed-on: https://chromium-review.googlesource.com/c/1442254
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#627386}
[modify] https://crrev.com/57729435d4d16fa543ace2545ca24fc00c4e9bf7/chrome/browser/password_manager/password_store_x.cc

Comment 31 by bugdroid, Jan 30

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/13c7cfde907f7e47ac49e4b9dc92dd904af04761

commit 13c7cfde907f7e47ac49e4b9dc92dd904af04761
Author: Mohamed Amir Yosef <mamir@chromium.org>
Date: Wed Jan 30 12:25:04 2019

[Sync::USS] Enabling Password integration test for USS

This CL enables Password browser tests for full blown USS
implementation.

Due to limitations in the test infrastructure, it's not possible to
run the integration tests against both pseudo and full-blown USS.
Therefore, this CL disables running passwords integration test against
the pseudo USS implementation.

Bug: 902349
Change-Id: If1da33e4e12b4b41a5ac97cd929d014ed5a17baf
Reviewed-on: https://chromium-review.googlesource.com/c/1439901
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#627400}
[modify] https://crrev.com/13c7cfde907f7e47ac49e4b9dc92dd904af04761/chrome/browser/sync/test/integration/single_client_passwords_sync_test.cc
[modify] https://crrev.com/13c7cfde907f7e47ac49e4b9dc92dd904af04761/chrome/browser/sync/test/integration/two_client_passwords_sync_test.cc
[modify] https://crrev.com/13c7cfde907f7e47ac49e4b9dc92dd904af04761/components/password_manager/core/browser/sync/password_sync_bridge.cc
[modify] https://crrev.com/13c7cfde907f7e47ac49e4b9dc92dd904af04761/components/sync/engine_impl/model_type_worker.cc
[modify] https://crrev.com/13c7cfde907f7e47ac49e4b9dc92dd904af04761/components/sync/engine_impl/model_type_worker_unittest.cc
[modify] https://crrev.com/13c7cfde907f7e47ac49e4b9dc92dd904af04761/components/sync/engine_impl/non_blocking_type_commit_contribution.cc
[modify] https://crrev.com/13c7cfde907f7e47ac49e4b9dc92dd904af04761/components/sync/engine_impl/non_blocking_type_commit_contribution_unittest.cc

Comment 32 by bugdroid, Jan 30

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8db656fe70ba233d370b083918c7eb4dd59bca4d

commit 8db656fe70ba233d370b083918c7eb4dd59bca4d
Author: Mohamed Amir Yosef <mamir@chromium.org>
Date: Wed Jan 30 22:14:35 2019

[Sync::USS] Introduce PasswordStoreSync::MetadataStore

This CL introduces PasswordStoreSync::MetadataStore interface.
PasswordSyncBridge requires an API to read all sync metadata from the
PasswordStore. Instead of polluting the PasswordStoreSync interface with
one method for getting the sync metadata, while all methods that handle
the sync metadata are in the SyncMetadataStore interface returned by
PasswordStoreSync::GetMetadataStore(), this CL introduce a new interface
that sub-classes the SyncMetadataStore and adds another method
ReadAllSyncMetadata().

LoginDatabase used to have the same functionally but marked ForTesting.
This CL exposes that API for the public use and changes its signature
to match the newly introduced API ReadAllSyncMetadata().

This CL also resolves the TODO that the PasswordSyncBridge upon
construction reads all stored sync metadata and pass them over to the
processor when invoking ModelReadyToSync()

Bug: 902349
Change-Id: Idad34d28cf739fd2e54a45a99881a12066953692
Reviewed-on: https://chromium-review.googlesource.com/c/1442717
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#627616}
[modify] https://crrev.com/8db656fe70ba233d370b083918c7eb4dd59bca4d/chrome/browser/password_manager/password_store_x.cc
[modify] https://crrev.com/8db656fe70ba233d370b083918c7eb4dd59bca4d/chrome/browser/password_manager/password_store_x.h
[modify] https://crrev.com/8db656fe70ba233d370b083918c7eb4dd59bca4d/components/password_manager/core/browser/DEPS
[modify] https://crrev.com/8db656fe70ba233d370b083918c7eb4dd59bca4d/components/password_manager/core/browser/login_database.cc
[modify] https://crrev.com/8db656fe70ba233d370b083918c7eb4dd59bca4d/components/password_manager/core/browser/login_database.h
[modify] https://crrev.com/8db656fe70ba233d370b083918c7eb4dd59bca4d/components/password_manager/core/browser/login_database_unittest.cc
[modify] https://crrev.com/8db656fe70ba233d370b083918c7eb4dd59bca4d/components/password_manager/core/browser/mock_password_store.h
[modify] https://crrev.com/8db656fe70ba233d370b083918c7eb4dd59bca4d/components/password_manager/core/browser/password_store_default.cc
[modify] https://crrev.com/8db656fe70ba233d370b083918c7eb4dd59bca4d/components/password_manager/core/browser/password_store_default.h
[modify] https://crrev.com/8db656fe70ba233d370b083918c7eb4dd59bca4d/components/password_manager/core/browser/password_store_sync.h
[modify] https://crrev.com/8db656fe70ba233d370b083918c7eb4dd59bca4d/components/password_manager/core/browser/sync/password_sync_bridge.cc
[modify] https://crrev.com/8db656fe70ba233d370b083918c7eb4dd59bca4d/components/password_manager/core/browser/sync/password_sync_bridge_unittest.cc
[modify] https://crrev.com/8db656fe70ba233d370b083918c7eb4dd59bca4d/components/password_manager/core/browser/test_password_store.cc
[modify] https://crrev.com/8db656fe70ba233d370b083918c7eb4dd59bca4d/components/password_manager/core/browser/test_password_store.h

Comment 33 by bugdroid, Feb 1

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/99ed662ec73f1c36b6048fc8d484a4eb8bf75534

commit 99ed662ec73f1c36b6048fc8d484a4eb8bf75534
Author: Mohamed Amir Yosef <mamir@chromium.org>
Date: Fri Feb 01 11:24:02 2019

[Sync::USS] Test that Password's progress marker is persisted

This CL adds a browser test that the progress marker is properly
persisted across restarts.

Bug: 902349
Change-Id: I6a116a132ac066fa388bd4f08eb7fe93540c0e2b
Reviewed-on: https://chromium-review.googlesource.com/c/1448330
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#628298}
[modify] https://crrev.com/99ed662ec73f1c36b6048fc8d484a4eb8bf75534/chrome/browser/sync/test/integration/single_client_passwords_sync_test.cc

Comment 34 by mamir@chromium.org, Feb 5

Blockedon: 928421

Comment 35 by bugdroid, Feb 6

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3074dfc70dcde9b0b7f3085b7cd9d60cfb6ce709

commit 3074dfc70dcde9b0b7f3085b7cd9d60cfb6ce709
Author: Mohamed Amir Yosef <mamir@chromium.org>
Date: Wed Feb 06 18:34:43 2019

[Sync::USS] Introduce about flag for USS passwords sync

This allows more easy testing of the feature under development/
experimentation.

Bug: 902349
Change-Id: I6c8c65c7c9a28ebdb0e40ed5d3a821981ad27ebb
Reviewed-on: https://chromium-review.googlesource.com/c/1454616
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Auto-Submit: Mohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#629655}
[modify] https://crrev.com/3074dfc70dcde9b0b7f3085b7cd9d60cfb6ce709/chrome/browser/about_flags.cc
[modify] https://crrev.com/3074dfc70dcde9b0b7f3085b7cd9d60cfb6ce709/chrome/browser/flag-metadata.json
[modify] https://crrev.com/3074dfc70dcde9b0b7f3085b7cd9d60cfb6ce709/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/3074dfc70dcde9b0b7f3085b7cd9d60cfb6ce709/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/3074dfc70dcde9b0b7f3085b7cd9d60cfb6ce709/tools/metrics/histograms/enums.xml

Sign in to add a comment