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

Issue 442053002: sync: Add non-blocking type encryption (retry) (Closed)

Created:
6 years, 4 months ago by rlarocque
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

sync: Add non-blocking type encryption (retry) Fixes some memory leak issues that were present in the first instance of this CL. Original description follows: 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=287849

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1250 lines, -87 lines) Patch
M components/sync_driver/non_blocking_data_type_controller_unittest.cc View 1 chunk +1 line, -0 lines 0 comments Download
M sync/engine/entity_tracker.h View 3 chunks +21 lines, -0 lines 0 comments Download
M sync/engine/entity_tracker.cc View 2 chunks +33 lines, -2 lines 0 comments Download
M sync/engine/model_type_entity.h View 5 chunks +17 lines, -4 lines 0 comments Download
M sync/engine/model_type_entity.cc View 8 chunks +27 lines, -9 lines 0 comments Download
M sync/engine/model_type_entity_unittest.cc View 5 chunks +12 lines, -6 lines 0 comments Download
M sync/engine/model_type_sync_proxy.h View 1 chunk +2 lines, -1 line 0 comments Download
M sync/engine/model_type_sync_proxy_impl.h View 3 chunks +17 lines, -1 line 0 comments Download
M sync/engine/model_type_sync_proxy_impl.cc View 6 chunks +74 lines, -9 lines 0 comments Download
M sync/engine/model_type_sync_proxy_impl_unittest.cc View 9 chunks +236 lines, -4 lines 0 comments Download
M sync/engine/model_type_sync_worker_impl.h View 4 chunks +43 lines, -5 lines 0 comments Download
M sync/engine/model_type_sync_worker_impl.cc View 15 chunks +188 lines, -21 lines 0 comments Download
M sync/engine/model_type_sync_worker_impl_unittest.cc View 17 chunks +485 lines, -8 lines 0 comments Download
M sync/internal_api/public/non_blocking_sync_common.h View 3 chunks +6 lines, -1 line 0 comments Download
M sync/internal_api/public/sync_context.h View 2 chunks +2 lines, -1 line 0 comments Download
M sync/internal_api/public/sync_context_proxy.h View 2 chunks +2 lines, -0 lines 0 comments Download
M sync/internal_api/public/test/null_sync_context_proxy.h View 2 chunks +2 lines, -0 lines 0 comments Download
M sync/internal_api/sync_context_proxy_impl.h View 1 chunk +1 line, -0 lines 0 comments Download
M sync/internal_api/sync_context_proxy_impl.cc View 2 chunks +2 lines, -0 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 +1 line, -0 lines 0 comments Download
M sync/sessions/model_type_registry.h View 4 chunks +6 lines, -1 line 0 comments Download
M sync/sessions/model_type_registry.cc View 6 chunks +16 lines, -5 lines 0 comments Download
M sync/sessions/model_type_registry_unittest.cc View 5 chunks +6 lines, -0 lines 0 comments Download
M sync/test/engine/injectable_sync_context_proxy.h View 2 chunks +2 lines, -0 lines 0 comments Download
M sync/test/engine/injectable_sync_context_proxy.cc View 1 chunk +1 line, -0 lines 0 comments Download
M sync/test/engine/mock_model_type_sync_proxy.h View 4 chunks +6 lines, -2 lines 0 comments Download
M sync/test/engine/mock_model_type_sync_proxy.cc View 3 chunks +13 lines, -3 lines 0 comments Download
M sync/test/engine/mock_model_type_sync_worker.h View 2 chunks +8 lines, -0 lines 0 comments Download
M sync/test/engine/mock_model_type_sync_worker.cc View 3 chunks +9 lines, -0 lines 0 comments Download
M sync/util/cryptographer.h View 1 chunk +4 lines, -1 line 0 comments Download
M sync/util/cryptographer.cc View 2 chunks +6 lines, -2 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
rlarocque
I forgot to upload the original to highlight the fixes. Fortunately, the diff is small, ...
6 years, 4 months ago (2014-08-05 18:58:01 UTC) #1
rlarocque
First attempt got reverted. Here's the retry. See previous message in this CL for the ...
6 years, 4 months ago (2014-08-05 19:40:22 UTC) #2
Nicolas Zea
lgtm
6 years, 4 months ago (2014-08-06 18:58:41 UTC) #3
rlarocque
The CQ bit was checked by rlarocque@chromium.org
6 years, 4 months ago (2014-08-06 18:59:21 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rlarocque@chromium.org/442053002/1
6 years, 4 months ago (2014-08-06 19:01:12 UTC) #5
commit-bot: I haz the power
6 years, 4 months ago (2014-08-06 20:46:21 UTC) #6
Message was sent while issue was closed.
Change committed as 287849

Powered by Google App Engine
This is Rietveld 408576698