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

Issue 2567813002: cros: DBUS client to interact with fingerprint DBUS API. (Closed)

Created:
4 years ago by sammiequon
Modified:
3 years, 8 months ago
CC:
chromium-reviews, hashimoto+watch_chromium.org, oshima+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

cros: DBUS client to interact with fingerprint DBUS API. Skeleton of DBUS client to interact with fingerprint DBUS API. Fake client and rest of methods/properties to come shortly after. Part 1 of a 5 patch series: https://codereview.chromium.org/2567813002/ Adds biometrics manager interface <<< https://codereview.chromium.org/2581403002/ Adds other interfaces https://codereview.chromium.org/2578323004/ Adds fake clients https://codereview.chromium.org/2646793003/ Hook up to chromeos https://codereview.chromium.org/2644233002/ Add a fake implemntation TEST=chromeos_unittest --gtest_filter="BiodClientTest.*" BUG=702675 Review-Url: https://codereview.chromium.org/2567813002 Cr-Commit-Position: refs/heads/master@{#461500} Committed: https://chromium.googlesource.com/chromium/src/+/0958b7f56e08e90312288a938205dc33fd69d56b

Patch Set 1 #

Patch Set 2 : Updated API and names. #

Patch Set 3 : Some nits. #

Patch Set 4 : Rebased to master. #

Total comments: 4

Patch Set 5 : Fixed patch set 4 errors. #

Total comments: 24

Patch Set 6 : Fixed patch set 5 errors. #

Patch Set 7 : ScanFailed -> SessionFailed. #

Total comments: 24

Patch Set 8 : Fixed patch set 7 errors. #

Total comments: 34

Patch Set 9 : Fixed patch set 8 errors. #

Patch Set 10 : Addressed comments. #

Patch Set 11 : s/BiodBiometricsManagerClient/BiodClient #

Total comments: 53

Patch Set 12 : Fixed patch set 11 errors. #

Total comments: 16

Patch Set 13 : Fixed patch set 12 errors. #

Total comments: 4

Patch Set 14 : Fixed patch set 13 errors. #

Total comments: 2

Patch Set 15 : Fixed patch set 14 errors. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+782 lines, -0 lines) Patch
M chromeos/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +3 lines, -0 lines 0 comments Download
A chromeos/dbus/biod/biod_client.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +121 lines, -0 lines 0 comments Download
A chromeos/dbus/biod/biod_client.cc View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +284 lines, -0 lines 0 comments Download
A chromeos/dbus/biod/biod_client_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +374 lines, -0 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 86 (48 generated)
sammiequon
On 2016/12/13 01:40:01, sammiequon wrote: > mailto:sammiequon@chromium.org changed reviewers: > + mailto:steel@chromium.org steel@ - Please ...
4 years ago (2016-12-13 01:40:17 UTC) #4
sammiequon
On 2016/12/13 01:40:17, sammiequon wrote: > On 2016/12/13 01:40:01, sammiequon wrote: > > mailto:sammiequon@chromium.org changed ...
3 years, 9 months ago (2017-03-14 19:06:01 UTC) #9
rkc
lgtm % comments https://codereview.chromium.org/2567813002/diff/120001/chromeos/dbus/biod_biometrics_manager_client.cc File chromeos/dbus/biod_biometrics_manager_client.cc (right): https://codereview.chromium.org/2567813002/diff/120001/chromeos/dbus/biod_biometrics_manager_client.cc#newcode274 chromeos/dbus/biod_biometrics_manager_client.cc:274: DBusClientImplementationType type) { Unused? https://codereview.chromium.org/2567813002/diff/120001/chromeos/dbus/biod_biometrics_manager_client_unittest.cc File ...
3 years, 9 months ago (2017-03-20 23:24:34 UTC) #20
sammiequon
Thanks! https://codereview.chromium.org/2567813002/diff/120001/chromeos/dbus/biod_biometrics_manager_client.cc File chromeos/dbus/biod_biometrics_manager_client.cc (right): https://codereview.chromium.org/2567813002/diff/120001/chromeos/dbus/biod_biometrics_manager_client.cc#newcode274 chromeos/dbus/biod_biometrics_manager_client.cc:274: DBusClientImplementationType type) { On 2017/03/20 23:24:34, rkc wrote: ...
3 years, 9 months ago (2017-03-21 00:13:36 UTC) #22
sammiequon
stevenjb@ - Please take a look. Thanks!
3 years, 9 months ago (2017-03-21 01:11:09 UTC) #24
stevenjb
https://codereview.chromium.org/2567813002/diff/140001/chromeos/dbus/biod_biometrics_manager_client.cc File chromeos/dbus/biod_biometrics_manager_client.cc (right): https://codereview.chromium.org/2567813002/diff/140001/chromeos/dbus/biod_biometrics_manager_client.cc#newcode23 chromeos/dbus/biod_biometrics_manager_client.cc:23: : biometrics_manager_proxy_(NULL), weak_ptr_factory_(this) {} s/NULL/nullptr throughout https://codereview.chromium.org/2567813002/diff/140001/chromeos/dbus/biod_biometrics_manager_client.cc#newcode195 chromeos/dbus/biod_biometrics_manager_client.cc:195: void ...
3 years, 9 months ago (2017-03-21 18:46:17 UTC) #25
sammiequon
https://codereview.chromium.org/2567813002/diff/140001/chromeos/dbus/biod_biometrics_manager_client.cc File chromeos/dbus/biod_biometrics_manager_client.cc (right): https://codereview.chromium.org/2567813002/diff/140001/chromeos/dbus/biod_biometrics_manager_client.cc#newcode23 chromeos/dbus/biod_biometrics_manager_client.cc:23: : biometrics_manager_proxy_(NULL), weak_ptr_factory_(this) {} On 2017/03/21 18:46:16, stevenjb wrote: ...
3 years, 9 months ago (2017-03-21 20:50:19 UTC) #27
stevenjb
lgtm
3 years, 9 months ago (2017-03-21 21:12:01 UTC) #28
sammiequon
On 2017/03/21 21:12:01, stevenjb wrote: > lgtm Thanks! Changed some naming to match the changes ...
3 years, 9 months ago (2017-03-21 23:26:06 UTC) #29
rkc
mostly lg, just a couple of comments. https://codereview.chromium.org/2567813002/diff/180001/chromeos/dbus/biod_biometrics_manager_client.cc File chromeos/dbus/biod_biometrics_manager_client.cc (right): https://codereview.chromium.org/2567813002/diff/180001/chromeos/dbus/biod_biometrics_manager_client.cc#newcode183 chromeos/dbus/biod_biometrics_manager_client.cc:183: void NameOwnerChangedReceived(const ...
3 years, 9 months ago (2017-03-25 00:28:20 UTC) #30
rkc
Can we move this to a //chromeos/dbus/biod directory? It is probably time that we start ...
3 years, 9 months ago (2017-03-25 00:56:04 UTC) #31
stevenjb
No reason not to create sub directories under chromeos/dbus that I can think of. derat@, ...
3 years, 9 months ago (2017-03-27 16:45:11 UTC) #33
Daniel Erat
i didn't look at the test code but have comments. do you plan to add ...
3 years, 9 months ago (2017-03-27 18:18:26 UTC) #34
rkc
I specifically want to move everything that I makes sense into sub-directories. I can take ...
3 years, 9 months ago (2017-03-27 21:37:14 UTC) #35
sammiequon
I moved the files to a biod directory since there will a couple more joining ...
3 years, 9 months ago (2017-03-27 23:21:49 UTC) #36
rkc
lgtm, but please wait for Dan's lgtm also.
3 years, 9 months ago (2017-03-27 23:30:42 UTC) #37
sammiequon
On 2017/03/27 23:30:42, rkc wrote: > lgtm, but please wait for Dan's lgtm also. Thanks!
3 years, 9 months ago (2017-03-27 23:33:59 UTC) #38
Daniel Erat
https://codereview.chromium.org/2567813002/diff/200001/chromeos/dbus/biod/biod_biometrics_manager_client.cc File chromeos/dbus/biod/biod_biometrics_manager_client.cc (right): https://codereview.chromium.org/2567813002/diff/200001/chromeos/dbus/biod/biod_biometrics_manager_client.cc#newcode143 chromeos/dbus/biod/biod_biometrics_manager_client.cc:143: LOG(ERROR) << "Failed to get response for " you ...
3 years, 9 months ago (2017-03-28 00:14:35 UTC) #39
sammiequon
https://codereview.chromium.org/2567813002/diff/200001/chromeos/dbus/biod/biod_biometrics_manager_client.cc File chromeos/dbus/biod/biod_biometrics_manager_client.cc (right): https://codereview.chromium.org/2567813002/diff/200001/chromeos/dbus/biod/biod_biometrics_manager_client.cc#newcode143 chromeos/dbus/biod/biod_biometrics_manager_client.cc:143: LOG(ERROR) << "Failed to get response for " On ...
3 years, 9 months ago (2017-03-28 01:31:06 UTC) #41
Daniel Erat
https://codereview.chromium.org/2567813002/diff/200001/chromeos/dbus/biod/biod_biometrics_manager_client.h File chromeos/dbus/biod/biod_biometrics_manager_client.h (right): https://codereview.chromium.org/2567813002/diff/200001/chromeos/dbus/biod/biod_biometrics_manager_client.h#newcode30 chromeos/dbus/biod/biod_biometrics_manager_client.h:30: // interface. On 2017/03/28 01:31:05, sammiequon wrote: > On ...
3 years, 9 months ago (2017-03-28 01:44:30 UTC) #42
sammiequon
https://codereview.chromium.org/2567813002/diff/200001/chromeos/dbus/biod/biod_biometrics_manager_client.h File chromeos/dbus/biod/biod_biometrics_manager_client.h (right): https://codereview.chromium.org/2567813002/diff/200001/chromeos/dbus/biod/biod_biometrics_manager_client.h#newcode30 chromeos/dbus/biod/biod_biometrics_manager_client.h:30: // interface. On 2017/03/28 01:44:30, Daniel Erat wrote: > ...
3 years, 9 months ago (2017-03-28 03:17:54 UTC) #43
Daniel Erat
https://codereview.chromium.org/2567813002/diff/200001/chromeos/dbus/biod/biod_biometrics_manager_client.cc File chromeos/dbus/biod/biod_biometrics_manager_client.cc (right): https://codereview.chromium.org/2567813002/diff/200001/chromeos/dbus/biod/biod_biometrics_manager_client.cc#newcode107 chromeos/dbus/biod/biod_biometrics_manager_client.cc:107: biod::kBiodServiceName, dbus::ObjectPath(biod::kBiodServicePath)); i think that this part is wrong. ...
3 years, 8 months ago (2017-03-29 22:11:58 UTC) #44
sammiequon
On 2017/03/29 22:11:58, Daniel Erat wrote: > this is why i suggested not having biod ...
3 years, 8 months ago (2017-03-29 22:30:45 UTC) #45
Daniel Erat
On 2017/03/29 22:30:45, sammiequon wrote: > On 2017/03/29 22:11:58, Daniel Erat wrote: > > this ...
3 years, 8 months ago (2017-03-29 22:36:04 UTC) #46
sammiequon
https://codereview.chromium.org/2567813002/diff/200001/chromeos/dbus/biod/biod_biometrics_manager_client.cc File chromeos/dbus/biod/biod_biometrics_manager_client.cc (right): https://codereview.chromium.org/2567813002/diff/200001/chromeos/dbus/biod/biod_biometrics_manager_client.cc#newcode107 chromeos/dbus/biod/biod_biometrics_manager_client.cc:107: biod::kBiodServiceName, dbus::ObjectPath(biod::kBiodServicePath)); On 2017/03/29 22:11:57, Daniel Erat wrote: > ...
3 years, 8 months ago (2017-03-30 00:22:27 UTC) #47
Daniel Erat
https://codereview.chromium.org/2567813002/diff/200001/chromeos/dbus/biod/biod_biometrics_manager_client.cc File chromeos/dbus/biod/biod_biometrics_manager_client.cc (right): https://codereview.chromium.org/2567813002/diff/200001/chromeos/dbus/biod/biod_biometrics_manager_client.cc#newcode107 chromeos/dbus/biod/biod_biometrics_manager_client.cc:107: biod::kBiodServiceName, dbus::ObjectPath(biod::kBiodServicePath)); On 2017/03/30 00:22:27, sammiequon wrote: > On ...
3 years, 8 months ago (2017-03-30 00:25:38 UTC) #48
sammiequon
On 2017/03/30 00:25:38, Daniel Erat wrote: > https://codereview.chromium.org/2567813002/diff/200001/chromeos/dbus/biod/biod_biometrics_manager_client.cc > File chromeos/dbus/biod/biod_biometrics_manager_client.cc (right): > > https://codereview.chromium.org/2567813002/diff/200001/chromeos/dbus/biod/biod_biometrics_manager_client.cc#newcode107 ...
3 years, 8 months ago (2017-03-30 18:14:42 UTC) #49
sammiequon
On 2017/03/30 18:14:42, sammiequon wrote: > On 2017/03/30 00:25:38, Daniel Erat wrote: > > > ...
3 years, 8 months ago (2017-03-31 00:55:15 UTC) #50
Daniel Erat
https://codereview.chromium.org/2567813002/diff/280001/chromeos/dbus/biod/biod_client.cc File chromeos/dbus/biod/biod_client.cc (right): https://codereview.chromium.org/2567813002/diff/280001/chromeos/dbus/biod/biod_client.cc#newcode139 chromeos/dbus/biod/biod_client.cc:139: dbus::ObjectPath result; you could make all of these methods ...
3 years, 8 months ago (2017-03-31 04:30:11 UTC) #51
sammiequon
https://codereview.chromium.org/2567813002/diff/280001/chromeos/dbus/biod/biod_client.cc File chromeos/dbus/biod/biod_client.cc (right): https://codereview.chromium.org/2567813002/diff/280001/chromeos/dbus/biod/biod_client.cc#newcode139 chromeos/dbus/biod/biod_client.cc:139: dbus::ObjectPath result; On 2017/03/31 04:30:09, Daniel Erat wrote: > ...
3 years, 8 months ago (2017-03-31 22:52:13 UTC) #55
Daniel Erat
https://codereview.chromium.org/2567813002/diff/280001/chromeos/dbus/biod/biod_client.h File chromeos/dbus/biod/biod_client.h (right): https://codereview.chromium.org/2567813002/diff/280001/chromeos/dbus/biod/biod_client.h#newcode25 chromeos/dbus/biod/biod_client.h:25: // with the labels of all the matched stored ...
3 years, 8 months ago (2017-04-01 00:46:27 UTC) #57
sammiequon
https://codereview.chromium.org/2567813002/diff/280001/chromeos/dbus/biod/biod_client.h File chromeos/dbus/biod/biod_client.h (right): https://codereview.chromium.org/2567813002/diff/280001/chromeos/dbus/biod/biod_client.h#newcode25 chromeos/dbus/biod/biod_client.h:25: // with the labels of all the matched stored ...
3 years, 8 months ago (2017-04-01 01:22:44 UTC) #60
Daniel Erat
thanks, lgtm with a few comments https://codereview.chromium.org/2567813002/diff/340001/chromeos/dbus/biod/biod_client_unittest.cc File chromeos/dbus/biod/biod_client_unittest.cc (right): https://codereview.chromium.org/2567813002/diff/340001/chromeos/dbus/biod/biod_client_unittest.cc#newcode29 chromeos/dbus/biod/biod_client_unittest.cc:29: const dbus::ObjectPath kInvalidTestPath ...
3 years, 8 months ago (2017-04-01 01:29:24 UTC) #61
sammiequon
Thanks Dan! https://codereview.chromium.org/2567813002/diff/340001/chromeos/dbus/biod/biod_client_unittest.cc File chromeos/dbus/biod/biod_client_unittest.cc (right): https://codereview.chromium.org/2567813002/diff/340001/chromeos/dbus/biod/biod_client_unittest.cc#newcode29 chromeos/dbus/biod/biod_client_unittest.cc:29: const dbus::ObjectPath kInvalidTestPath = On 2017/04/01 01:29:24, ...
3 years, 8 months ago (2017-04-03 04:59:19 UTC) #70
Daniel Erat
lgtm https://codereview.chromium.org/2567813002/diff/380001/chromeos/dbus/biod/biod_client_unittest.cc File chromeos/dbus/biod/biod_client_unittest.cc (right): https://codereview.chromium.org/2567813002/diff/380001/chromeos/dbus/biod/biod_client_unittest.cc#newcode31 chromeos/dbus/biod/biod_client_unittest.cc:31: const char* kInvalidTestPath = "/invalid/test/path"; one more nit: ...
3 years, 8 months ago (2017-04-03 13:52:34 UTC) #75
sammiequon
Thanks! https://codereview.chromium.org/2567813002/diff/380001/chromeos/dbus/biod/biod_client_unittest.cc File chromeos/dbus/biod/biod_client_unittest.cc (right): https://codereview.chromium.org/2567813002/diff/380001/chromeos/dbus/biod/biod_client_unittest.cc#newcode31 chromeos/dbus/biod/biod_client_unittest.cc:31: const char* kInvalidTestPath = "/invalid/test/path"; On 2017/04/03 13:52:34, ...
3 years, 8 months ago (2017-04-03 16:40:42 UTC) #78
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/2567813002/400001
3 years, 8 months ago (2017-04-03 19:05:39 UTC) #83
commit-bot: I haz the power
3 years, 8 months ago (2017-04-03 19:25:16 UTC) #86
Message was sent while issue was closed.
Committed patchset #15 (id:400001) as
https://chromium.googlesource.com/chromium/src/+/0958b7f56e08e90312288a938205...

Powered by Google App Engine
This is Rietveld 408576698