Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(421)

Issue 442623002: Revert of sync: Add non-blocking type encryption support (Closed)

Created:
6 years, 4 months ago by Lei Zhang
Modified:
6 years, 4 months ago
CC:
chromium-reviews, tim+watch_chromium.org, haitaol+watch_chromium.org, zea+watch_chromium.org, maniscalco+watch_chromium.org
Project:
chromium
Visibility:
Public.

Description

Revert of sync: Add non-blocking type encryption support (https://codereview.chromium.org/423193002/) Reason for revert: Both LSAN and Valgrind complains about leaks in the tests. Original issue's description: > sync: Add non-blocking type encryption support > > Introduces the framework for dealing with sync encryption in > non-blocking types. Unlike directory sync types, non-blocking type > encryption only encrypts data before it is sent to the server. > Encrypting the data on-disk is a separate problem. > > Adds code to the ModelTypeSyncWorker so it can access the directory's > cryptographer (through a CryptographerProvider interface) and use it to > encrypt entities before it sends them to the server. If the > cryptographer is unable to encrypt with the desired key, the worker will > not commit until the cryptographer returns to a good state. > > Adds the concept of a "desired encryption key" to the data type state. > When the cryptographer key to be used to encrypt a type changes, this > will be reflected in the data type state. The ModelTypeSyncProxy is > responsible for ensuring that all items which have not yet been > encrypted with this desired key are enqueued for commit. > > Makes the ModelTypeSyncWorker, EntityTracker, and ModelTypeSyncProxy > collaborate on the management of undecryptable (inapplicable) updates. > The EntityTracker keeps track of their version numbers and content, and > prevents the committing of new items to the server until the > inapplicable update has been dealt with. The ModelTypeSyncProxy is > responsible for saving inapplicable updates across restarts. > > This CL alone is not enough to enable encryption support for > non-blocking types. It requires additional code to hook up the > ModelTypeSyncWorkers to receive cryptographer events. This will be > added in a future commit. In the meantime, this CL includes plenty > of unit tests to verify the functionality that's being added. > > BUG=351005 > > Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=287428 TBR=zea@chromium.org,stanisc@chromium.org,rlarocque@chromium.org NOTREECHECKS=true NOTRY=true BUG=351005 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=287433

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+90 lines, -1247 lines) Patch
M components/sync_driver/non_blocking_data_type_controller_unittest.cc View 1 chunk +0 lines, -1 line 0 comments Download
M sync/engine/entity_tracker.h View 3 chunks +0 lines, -21 lines 0 comments Download
M sync/engine/entity_tracker.cc View 2 chunks +2 lines, -33 lines 0 comments Download
M sync/engine/model_type_entity.h View 5 chunks +4 lines, -17 lines 0 comments Download
M sync/engine/model_type_entity.cc View 8 chunks +9 lines, -27 lines 0 comments Download
M sync/engine/model_type_entity_unittest.cc View 5 chunks +6 lines, -12 lines 0 comments Download
M sync/engine/model_type_sync_proxy.h View 1 chunk +1 line, -2 lines 0 comments Download
M sync/engine/model_type_sync_proxy_impl.h View 3 chunks +1 line, -17 lines 0 comments Download
M sync/engine/model_type_sync_proxy_impl.cc View 6 chunks +11 lines, -76 lines 0 comments Download
M sync/engine/model_type_sync_proxy_impl_unittest.cc View 9 chunks +4 lines, -230 lines 0 comments Download
M sync/engine/model_type_sync_worker_impl.h View 4 chunks +5 lines, -43 lines 0 comments Download
M sync/engine/model_type_sync_worker_impl.cc View 15 chunks +22 lines, -189 lines 0 comments Download
M sync/engine/model_type_sync_worker_impl_unittest.cc View 17 chunks +8 lines, -485 lines 0 comments Download
M sync/internal_api/public/non_blocking_sync_common.h View 3 chunks +1 line, -6 lines 0 comments Download
M sync/internal_api/public/sync_context.h View 2 chunks +1 line, -2 lines 0 comments Download
M sync/internal_api/public/sync_context_proxy.h View 2 chunks +0 lines, -2 lines 0 comments Download
M sync/internal_api/public/test/null_sync_context_proxy.h View 2 chunks +0 lines, -2 lines 0 comments Download
M sync/internal_api/sync_context_proxy_impl.h View 1 chunk +0 lines, -1 line 0 comments Download
M sync/internal_api/sync_context_proxy_impl.cc View 2 chunks +0 lines, -2 lines 0 comments Download
M sync/internal_api/sync_encryption_handler_impl.cc View 1 chunk +1 line, -1 line 0 comments Download
M sync/internal_api/test/null_sync_context_proxy.cc View 1 chunk +0 lines, -1 line 0 comments Download
M sync/sessions/model_type_registry.h View 4 chunks +1 line, -6 lines 0 comments Download
M sync/sessions/model_type_registry.cc View 6 chunks +5 lines, -16 lines 0 comments Download
M sync/sessions/model_type_registry_unittest.cc View 5 chunks +0 lines, -6 lines 0 comments Download
M sync/test/engine/injectable_sync_context_proxy.h View 2 chunks +0 lines, -2 lines 0 comments Download
M sync/test/engine/injectable_sync_context_proxy.cc View 1 chunk +0 lines, -1 line 0 comments Download
M sync/test/engine/mock_model_type_sync_proxy.h View 4 chunks +2 lines, -6 lines 0 comments Download
M sync/test/engine/mock_model_type_sync_proxy.cc View 3 chunks +3 lines, -13 lines 0 comments Download
M sync/test/engine/mock_model_type_sync_worker.h View 2 chunks +0 lines, -8 lines 0 comments Download
M sync/test/engine/mock_model_type_sync_worker.cc View 3 chunks +0 lines, -9 lines 0 comments Download
M sync/util/cryptographer.h View 1 chunk +1 line, -4 lines 0 comments Download
M sync/util/cryptographer.cc View 2 chunks +2 lines, -6 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
Lei Zhang
Created Revert of sync: Add non-blocking type encryption support
6 years, 4 months ago (2014-08-05 02:15:39 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/thestig@chromium.org/442623002/1
6 years, 4 months ago (2014-08-05 02:17:12 UTC) #2
commit-bot: I haz the power
Change committed as 287433
6 years, 4 months ago (2014-08-05 02:19:38 UTC) #3
rlarocque
6 years, 4 months ago (2014-08-05 17:22:00 UTC) #4
Message was sent while issue was closed.
On 2014/08/05 02:19:38, I haz the power (commit-bot) wrote:
> Change committed as 287433

LGTM.  Thanks for the revert.

Powered by Google App Engine
This is Rietveld 408576698