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

Issue 2934293003: The chrome.lockScreen.data API implementation (Closed)

Created:
3 years, 6 months ago by tbarzic
Modified:
3 years, 5 months ago
Reviewers:
rkc, Bernhard Bauer, Devlin
CC:
chromium-reviews, chromium-apps-reviews_chromium.org, asvitkine+watch_chromium.org, extensions-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

The chrome.lockScreen.data API implementation Introduces a data item storage for data created on lock screen. Data items are persisted on disk in per-extension value stores backed by leveldb (same backend as chrome.storage.local API). Value store is structured as follows: * registered_items -> dictionary whose keys are IDs of registered item Dictionary values are currently empty dictionaries, but could be extended in future to contain data item metadata, if required. * item ID -> base64 encoded encrypted data item content Note that data item content is encrypted because the storage is kept outside user directory. Encrypted data is base 64 encoded because value store does not currently support binary data (the DB values are preserved as JSON strings, and JSON writer does not handle binary values well). Additionally, this CL adds a dictionary to local state that maps extension IDs to number of lock screen data items registered for the extension. The purpose of this dictionary is to enable the lockScreen.data API to determine set of apps to which it should set OnDataItemsAvailable event when user session is activated without querying the values store database for each installed extension. BUG=715781 Review-Url: https://codereview.chromium.org/2934293003 Cr-Commit-Position: refs/heads/master@{#486636} Committed: https://chromium.googlesource.com/chromium/src/+/d9a88fc99f8b5465f13c6d9f8c6af42994f5335e

Patch Set 1 #

Patch Set 2 : . #

Patch Set 3 : . #

Patch Set 4 : . #

Patch Set 5 : remove FilePath*UTF8Unsafe methods #

Total comments: 13

Patch Set 6 : . #

Patch Set 7 : . #

Patch Set 8 : value store for backend #

Patch Set 9 : . #

Patch Set 10 : . #

Patch Set 11 : . #

Total comments: 8

Patch Set 12 : rkc feedback #

Patch Set 13 : few more comment updates #

Patch Set 14 : rename item_storage in browser_prefs #

Total comments: 22

Patch Set 15 : review feedback #

Patch Set 16 : switch to BackendTaskRunner #

Unified diffs Side-by-side diffs Delta from patch set Stats (+3320 lines, -21 lines) Patch
M chrome/browser/prefs/browser_prefs.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 2 chunks +3 lines, -0 lines 0 comments Download
M extensions/browser/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +2 lines, -0 lines 0 comments Download
M extensions/browser/api/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +5 lines, -0 lines 0 comments Download
A extensions/browser/api/lock_screen_data/data_item.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +144 lines, -0 lines 0 comments Download
A extensions/browser/api/lock_screen_data/data_item.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +435 lines, -0 lines 0 comments Download
A extensions/browser/api/lock_screen_data/data_item_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +733 lines, -0 lines 0 comments Download
M extensions/browser/api/lock_screen_data/lock_screen_data_api.h View 1 2 3 4 5 6 7 6 chunks +20 lines, -0 lines 0 comments Download
M extensions/browser/api/lock_screen_data/lock_screen_data_api.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 5 chunks +129 lines, -7 lines 0 comments Download
A extensions/browser/api/lock_screen_data/lock_screen_item_storage.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +267 lines, -0 lines 0 comments Download
A extensions/browser/api/lock_screen_data/lock_screen_item_storage.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +486 lines, -0 lines 0 comments Download
A extensions/browser/api/lock_screen_data/lock_screen_item_storage_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1046 lines, -0 lines 0 comments Download
A extensions/browser/api/lock_screen_data/operation_result.h View 1 2 3 4 5 6 7 1 chunk +26 lines, -0 lines 0 comments Download
M extensions/browser/test_extensions_browser_client.h View 1 2 3 4 5 3 chunks +14 lines, -5 lines 0 comments Download
M extensions/browser/test_extensions_browser_client.cc View 1 2 3 4 5 6 chunks +10 lines, -9 lines 0 comments Download

Messages

Total messages: 47 (29 generated)
tbarzic
3 years, 6 months ago (2017-06-15 04:58:59 UTC) #2
Devlin
Since rkc@ will be an owner here, I'll let him review first. One high level ...
3 years, 6 months ago (2017-06-20 17:38:15 UTC) #3
tbarzic
On 2017/06/20 17:38:15, Devlin wrote: > Since rkc@ will be an owner here, I'll let ...
3 years, 6 months ago (2017-06-20 19:14:34 UTC) #4
rkc
On 2017/06/20 19:14:34, tbarzic wrote: > On 2017/06/20 17:38:15, Devlin wrote: > > Since rkc@ ...
3 years, 6 months ago (2017-06-22 18:35:36 UTC) #5
rkc
https://codereview.chromium.org/2934293003/diff/80001/extensions/browser/api/lock_screen_data/data_item.h File extensions/browser/api/lock_screen_data/data_item.h (right): https://codereview.chromium.org/2934293003/diff/80001/extensions/browser/api/lock_screen_data/data_item.h#newcode52 extensions/browser/api/lock_screen_data/data_item.h:52: virtual OperationResult Write( Why return a value at all? ...
3 years, 6 months ago (2017-06-22 18:35:48 UTC) #6
tbarzic
Switched this to use value store to persist lock screen data. let me know what ...
3 years, 5 months ago (2017-06-26 22:21:42 UTC) #16
rkc
mostly lg, minor comments. https://codereview.chromium.org/2934293003/diff/200001/extensions/browser/api/lock_screen_data/data_item.cc File extensions/browser/api/lock_screen_data/data_item.cc (right): https://codereview.chromium.org/2934293003/diff/200001/extensions/browser/api/lock_screen_data/data_item.cc#newcode40 extensions/browser/api/lock_screen_data/data_item.cc:40: std::string iv(16, ' '); I ...
3 years, 5 months ago (2017-07-10 20:04:00 UTC) #17
tbarzic
https://codereview.chromium.org/2934293003/diff/200001/extensions/browser/api/lock_screen_data/data_item.cc File extensions/browser/api/lock_screen_data/data_item.cc (right): https://codereview.chromium.org/2934293003/diff/200001/extensions/browser/api/lock_screen_data/data_item.cc#newcode40 extensions/browser/api/lock_screen_data/data_item.cc:40: std::string iv(16, ' '); On 2017/07/10 20:04:00, rkc wrote: ...
3 years, 5 months ago (2017-07-10 22:29:50 UTC) #18
rkc
lgtm
3 years, 5 months ago (2017-07-10 22:33:09 UTC) #19
tbarzic
updating reviewer list for owner approvals: Devlin (already cced): extensions/browser/test_extensions_browser_client.* bauerb: chrome/browser/prefs/browser_prefs.cc
3 years, 5 months ago (2017-07-11 01:05:39 UTC) #25
Bernhard Bauer
prefs LGTM
3 years, 5 months ago (2017-07-11 09:47:47 UTC) #26
Devlin
Wow, this is a massive CL... First pass. https://codereview.chromium.org/2934293003/diff/260001/extensions/browser/api/lock_screen_data/data_item.cc File extensions/browser/api/lock_screen_data/data_item.cc (right): https://codereview.chromium.org/2934293003/diff/260001/extensions/browser/api/lock_screen_data/data_item.cc#newcode42 extensions/browser/api/lock_screen_data/data_item.cc:42: std::string ...
3 years, 5 months ago (2017-07-12 02:13:41 UTC) #27
tbarzic
https://codereview.chromium.org/2934293003/diff/260001/extensions/browser/api/lock_screen_data/data_item.cc File extensions/browser/api/lock_screen_data/data_item.cc (right): https://codereview.chromium.org/2934293003/diff/260001/extensions/browser/api/lock_screen_data/data_item.cc#newcode42 extensions/browser/api/lock_screen_data/data_item.cc:42: std::string iv(kAesInitializationVectorLength, ' '); On 2017/07/12 02:13:40, Devlin wrote: ...
3 years, 5 months ago (2017-07-12 04:02:11 UTC) #28
Devlin
https://codereview.chromium.org/2934293003/diff/260001/extensions/browser/api/lock_screen_data/lock_screen_item_storage.h File extensions/browser/api/lock_screen_data/lock_screen_item_storage.h (right): https://codereview.chromium.org/2934293003/diff/260001/extensions/browser/api/lock_screen_data/lock_screen_item_storage.h#newcode241 extensions/browser/api/lock_screen_data/lock_screen_item_storage.h:241: const std::string user_id_; On 2017/07/12 04:02:11, tbarzic wrote: > ...
3 years, 5 months ago (2017-07-12 22:42:57 UTC) #29
Devlin
On 2017/07/12 22:42:57, Devlin wrote: > https://codereview.chromium.org/2934293003/diff/260001/extensions/browser/api/lock_screen_data/lock_screen_item_storage.h > File extensions/browser/api/lock_screen_data/lock_screen_item_storage.h (right): > > https://codereview.chromium.org/2934293003/diff/260001/extensions/browser/api/lock_screen_data/lock_screen_item_storage.h#newcode241 > ...
3 years, 5 months ago (2017-07-12 22:43:58 UTC) #30
tbarzic
https://codereview.chromium.org/2934293003/diff/260001/extensions/browser/api/lock_screen_data/lock_screen_item_storage.h File extensions/browser/api/lock_screen_data/lock_screen_item_storage.h (right): https://codereview.chromium.org/2934293003/diff/260001/extensions/browser/api/lock_screen_data/lock_screen_item_storage.h#newcode241 extensions/browser/api/lock_screen_data/lock_screen_item_storage.h:241: const std::string user_id_; On 2017/07/12 22:42:57, Devlin wrote: > ...
3 years, 5 months ago (2017-07-12 23:52:41 UTC) #31
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2934293003/290001
3 years, 5 months ago (2017-07-14 00:23:56 UTC) #44
commit-bot: I haz the power
3 years, 5 months ago (2017-07-14 02:18:33 UTC) #47
Message was sent while issue was closed.
Committed patchset #16 (id:290001) as
https://chromium.googlesource.com/chromium/src/+/d9a88fc99f8b5465f13c6d9f8c6a...

Powered by Google App Engine
This is Rietveld 408576698