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

Issue 2581403002: cros: Remaining interfaces for DBUS biometrics client. (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: Remaining interfaces for DBUS biometrics client. Add the auth session, enroll session and record clients. Part 2 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/2581403002 Cr-Commit-Position: refs/heads/master@{#462171} Committed: https://chromium.googlesource.com/chromium/src/+/7b2183284d955be12c2662bbcb0752393a0861d9

Patch Set 1 #

Patch Set 2 : Updated naming. #

Patch Set 3 : Rebased to master. #

Patch Set 4 : Remove chromeos.gyp. #

Total comments: 6

Patch Set 5 : Rebased. #

Patch Set 6 : Patch set 4 fixes. #

Patch Set 7 : Rebased. #

Total comments: 10

Patch Set 8 : Fixed patch set 7 errors. #

Total comments: 2

Patch Set 9 : Fix patch set 8 errors. #

Total comments: 10

Patch Set 10 : Rebased. #

Total comments: 1

Patch Set 11 : Removed extra files. #

Total comments: 8

Patch Set 12 : Fixed patch set 11 errors. #

Patch Set 13 : Rebased. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+142 lines, -6 lines) Patch
M chromeos/dbus/biod/biod_client.h View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +23 lines, -0 lines 0 comments Download
M chromeos/dbus/biod/biod_client.cc View 1 2 3 4 5 6 7 8 9 10 11 4 chunks +81 lines, -2 lines 0 comments Download
M chromeos/dbus/biod/biod_client_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 5 chunks +38 lines, -4 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 47 (23 generated)
sammiequon
steel@ - Please take a look. Thanks!
3 years, 9 months ago (2017-03-17 17:02:53 UTC) #5
rkc
https://codereview.chromium.org/2581403002/diff/80001/chromeos/dbus/biod_auth_session_client.cc File chromeos/dbus/biod_auth_session_client.cc (right): https://codereview.chromium.org/2581403002/diff/80001/chromeos/dbus/biod_auth_session_client.cc#newcode56 chromeos/dbus/biod_auth_session_client.cc:56: DBusClientImplementationType type) { /* type */ https://codereview.chromium.org/2581403002/diff/80001/chromeos/dbus/biod_enroll_session_client.cc File chromeos/dbus/biod_enroll_session_client.cc ...
3 years, 9 months ago (2017-03-20 23:47:09 UTC) #7
rkc
BTW, with chained patches like this, it is helpful to have a list of all ...
3 years, 9 months ago (2017-03-20 23:50:51 UTC) #8
sammiequon
https://codereview.chromium.org/2581403002/diff/80001/chromeos/dbus/biod_auth_session_client.cc File chromeos/dbus/biod_auth_session_client.cc (right): https://codereview.chromium.org/2581403002/diff/80001/chromeos/dbus/biod_auth_session_client.cc#newcode56 chromeos/dbus/biod_auth_session_client.cc:56: DBusClientImplementationType type) { On 2017/03/20 23:47:09, rkc wrote: > ...
3 years, 9 months ago (2017-03-21 00:30:39 UTC) #10
sammiequon
stevenjb@ - Please take a look. Thanks!
3 years, 9 months ago (2017-03-22 00:23:23 UTC) #12
stevenjb
Please be sure to have someone familiar with the feature (rkc?) also review this. https://codereview.chromium.org/2581403002/diff/140001/chromeos/dbus/biod_auth_session_client.cc ...
3 years, 9 months ago (2017-03-22 20:57:53 UTC) #13
sammiequon
yeah, I'll be sure to wait for rkc's lgtm before proceeding https://codereview.chromium.org/2581403002/diff/140001/chromeos/dbus/biod_auth_session_client.cc File chromeos/dbus/biod_auth_session_client.cc (right): ...
3 years, 9 months ago (2017-03-23 18:30:37 UTC) #14
stevenjb
https://codereview.chromium.org/2581403002/diff/160001/chromeos/dbus/biod_record_client.cc File chromeos/dbus/biod_record_client.cc (right): https://codereview.chromium.org/2581403002/diff/160001/chromeos/dbus/biod_record_client.cc#newcode76 chromeos/dbus/biod_record_client.cc:76: LOG(ERROR) << "Error calling " << biod::kRecordLabelProperty; Shouldn't we ...
3 years, 9 months ago (2017-03-23 20:23:34 UTC) #15
sammiequon
https://codereview.chromium.org/2581403002/diff/160001/chromeos/dbus/biod_record_client.cc File chromeos/dbus/biod_record_client.cc (right): https://codereview.chromium.org/2581403002/diff/160001/chromeos/dbus/biod_record_client.cc#newcode76 chromeos/dbus/biod_record_client.cc:76: LOG(ERROR) << "Error calling " << biod::kRecordLabelProperty; On 2017/03/23 ...
3 years, 9 months ago (2017-03-23 21:12:25 UTC) #16
stevenjb
owner lgtm
3 years, 9 months ago (2017-03-23 21:14:57 UTC) #17
sammiequon
On 2017/03/23 21:14:57, stevenjb wrote: > owner lgtm Thanks! rkc@ - Please take another look. ...
3 years, 9 months ago (2017-03-23 21:45:56 UTC) #18
rkc
https://codereview.chromium.org/2581403002/diff/180001/chromeos/dbus/biod_auth_session_client.h File chromeos/dbus/biod_auth_session_client.h (right): https://codereview.chromium.org/2581403002/diff/180001/chromeos/dbus/biod_auth_session_client.h#newcode26 chromeos/dbus/biod_auth_session_client.h:26: virtual void EndAuthSession(const dbus::ObjectPath& auth_session_path) = 0; Any reason ...
3 years, 9 months ago (2017-03-25 00:41:38 UTC) #19
sammiequon
derat@ - Do you mind taking a look at the other CLs in this series? ...
3 years, 8 months ago (2017-04-03 17:52:28 UTC) #21
Daniel Erat
re the commit message, this actually patch #2 rather than #1 in the series, right? ...
3 years, 8 months ago (2017-04-03 18:02:10 UTC) #22
sammiequon
On 2017/04/03 18:02:10, Daniel Erat wrote: > re the commit message, this actually patch #2 ...
3 years, 8 months ago (2017-04-03 19:23:12 UTC) #26
Daniel Erat
https://codereview.chromium.org/2581403002/diff/240001/chromeos/dbus/biod/biod_client.cc File chromeos/dbus/biod/biod_client.cc (right): https://codereview.chromium.org/2581403002/diff/240001/chromeos/dbus/biod/biod_client.cc#newcode343 chromeos/dbus/biod/biod_client.cc:343: dbus::Bus* bus_; nit: add " = nullptr;" here https://codereview.chromium.org/2581403002/diff/240001/chromeos/dbus/biod/biod_client.cc#newcode344 ...
3 years, 8 months ago (2017-04-03 19:58:18 UTC) #27
sammiequon
https://codereview.chromium.org/2581403002/diff/240001/chromeos/dbus/biod/biod_client.cc File chromeos/dbus/biod/biod_client.cc (right): https://codereview.chromium.org/2581403002/diff/240001/chromeos/dbus/biod/biod_client.cc#newcode343 chromeos/dbus/biod/biod_client.cc:343: dbus::Bus* bus_; On 2017/04/03 19:58:18, Daniel Erat wrote: > ...
3 years, 8 months ago (2017-04-03 21:35:09 UTC) #28
Daniel Erat
lgtm
3 years, 8 months ago (2017-04-03 22:31:10 UTC) #29
sammiequon
On 2017/04/03 22:31:10, Daniel Erat wrote: > lgtm Thanks!
3 years, 8 months ago (2017-04-03 23:02:56 UTC) #30
sammiequon
On 2017/04/03 23:02:56, sammiequon wrote: > On 2017/04/03 22:31:10, Daniel Erat wrote: > > lgtm ...
3 years, 8 months ago (2017-04-04 16:43:26 UTC) #39
rkc
lgtm
3 years, 8 months ago (2017-04-05 18:16:40 UTC) #40
sammiequon
On 2017/04/05 18:16:40, rkc wrote: > lgtm Thanks!
3 years, 8 months ago (2017-04-05 18:27:54 UTC) #41
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/2581403002/280001
3 years, 8 months ago (2017-04-05 18:28:34 UTC) #44
commit-bot: I haz the power
3 years, 8 months ago (2017-04-05 19:18:23 UTC) #47
Message was sent while issue was closed.
Committed patchset #13 (id:280001) as
https://chromium.googlesource.com/chromium/src/+/7b2183284d955be12c2662bbcb07...

Powered by Google App Engine
This is Rietveld 408576698