cros: DBUS client to interact with fingerprint DBUS API.

chromeos/dbus/biod_biometrics_manager_client.cc

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chromeos/dbus/biod_biometrics_manager_client.h"
+
+#include <stdint.h>
+
+#include "base/bind.h"
+#include "base/macros.h"
+#include "dbus/bus.h"
+#include "dbus/message.h"
+#include "dbus/object_path.h"
+#include "dbus/object_proxy.h"
+#include "third_party/cros_system_api/dbus/service_constants.h"
+
+namespace chromeos {
+
+// The BiodBiometricsManagerClient implementation used in production.
+class BiodBiometricsManagerClientImpl : public BiodBiometricsManagerClient {
+ public:
+  BiodBiometricsManagerClientImpl()
+      : biometrics_manager_proxy_(nullptr), weak_ptr_factory_(this) {}
+
+  ~BiodBiometricsManagerClientImpl() override {}
+
+  // BiodBiometricsManagerClient overrides:
+  void AddObserver(Observer* observer) override {
+    observers_.AddObserver(observer);
+  }
+
+  void RemoveObserver(Observer* observer) override {
+    observers_.RemoveObserver(observer);
+  }
+
+  bool HasObserver(const Observer* observer) const override {
+    return observers_.HasObserver(observer);
+  }
+
+  void StartEnrollSession(const std::string& user_id,
+                          const std::string& label,
+                          const ObjectPathCallback& callback) override {
+    dbus::MethodCall method_call(
+        biod::kBiometricsManagerInterface,
+        biod::kBiometricsManagerStartEnrollSessionMethod);
+    dbus::MessageWriter writer(&method_call);
+    writer.AppendString(user_id);
+    writer.AppendString(label);
+
+    biometrics_manager_proxy_->CallMethod(
+        &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+        base::Bind(&BiodBiometricsManagerClientImpl::OnStartEnrollSession,
+                   weak_ptr_factory_.GetWeakPtr(), callback));
+  }
+
+  void GetRecordsForUser(const std::string& user_id,
+                         const UserRecordsCallback& callback) override {
+    dbus::MethodCall method_call(
+        biod::kBiometricsManagerInterface,
+        biod::kBiometricsManagerGetRecordsForUserMethod);
+    dbus::MessageWriter writer(&method_call);
+    writer.AppendString(user_id);
+
+    biometrics_manager_proxy_->CallMethod(
+        &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+        base::Bind(&BiodBiometricsManagerClientImpl::OnGetRecordsForUser,
+                   weak_ptr_factory_.GetWeakPtr(), callback));
+  }
+
+  void DestroyAllRecords() override {
+    dbus::MethodCall method_call(
+        biod::kBiometricsManagerInterface,
+        biod::kBiometricsManagerDestroyAllRecordsMethod);
+
+    biometrics_manager_proxy_->CallMethod(
+        &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+        dbus::ObjectProxy::EmptyResponseCallback());
+  }
+
+  void StartAuthSession(const ObjectPathCallback& callback) override {
+    dbus::MethodCall method_call(
+        biod::kBiometricsManagerInterface,
+        biod::kBiometricsManagerStartAuthSessionMethod);
+
+    biometrics_manager_proxy_->CallMethod(
+        &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+        base::Bind(&BiodBiometricsManagerClientImpl::OnStartAuthSession,
+                   weak_ptr_factory_.GetWeakPtr(), callback));
+  }
+
+  void RequestType(const BiometricTypeCallback& callback) override {
+    dbus::MethodCall method_call(dbus::kDBusPropertiesInterface,
+                                 dbus::kDBusPropertiesGet);
+    dbus::MessageWriter writer(&method_call);
+    writer.AppendString(biod::kBiometricsManagerInterface);
+    writer.AppendString(biod::kBiometricsManagerBiometricTypeProperty);
+
+    biometrics_manager_proxy_->CallMethod(
+        &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+        base::Bind(&BiodBiometricsManagerClientImpl::OnRequestType,
+                   weak_ptr_factory_.GetWeakPtr(), callback));
+  }
+
+ protected:
+  void Init(dbus::Bus* bus) override {
+    biometrics_manager_proxy_ = bus->GetObjectProxy(
+        biod::kBiodServiceName, dbus::ObjectPath(biod::kBiodServicePath));
+
+    // Monitor the NameOwnerChanged signal.

[Daniel Erat, 2017/03/27 18:18:25]

these comments don't add much; i'd remove them

[sammiequon, 2017/03/27 23:21:48]

Done.

+    biometrics_manager_proxy_->SetNameOwnerChangedCallback(
+        base::Bind(&BiodBiometricsManagerClientImpl::NameOwnerChangedReceived,
+                   weak_ptr_factory_.GetWeakPtr()));
+
+    // Monitor the D-Bus signal for enroll scans.
+    biometrics_manager_proxy_->ConnectToSignal(
+        biod::kBiometricsManagerInterface,
+        biod::kBiometricsManagerEnrollScanDoneSignal,
+        base::Bind(&BiodBiometricsManagerClientImpl::EnrollScanDoneReceived,
+                   weak_ptr_factory_.GetWeakPtr()),
+        base::Bind(&BiodBiometricsManagerClientImpl::OnSignalConnected,
+                   weak_ptr_factory_.GetWeakPtr()));
+
+    // Monitor the D-Bus signal for auth scans.
+    biometrics_manager_proxy_->ConnectToSignal(
+        biod::kBiometricsManagerInterface,
+        biod::kBiometricsManagerAuthScanDoneSignal,
+        base::Bind(&BiodBiometricsManagerClientImpl::AuthScanDoneReceived,
+                   weak_ptr_factory_.GetWeakPtr()),
+        base::Bind(&BiodBiometricsManagerClientImpl::OnSignalConnected,
+                   weak_ptr_factory_.GetWeakPtr()));
+
+    // Monitor the D-Bus signal for session failures.
+    biometrics_manager_proxy_->ConnectToSignal(
+        biod::kBiometricsManagerInterface,
+        biod::kBiometricsManagerSessionFailedSignal,
+        base::Bind(&BiodBiometricsManagerClientImpl::SessionFailedReceived,
+                   weak_ptr_factory_.GetWeakPtr()),
+        base::Bind(&BiodBiometricsManagerClientImpl::OnSignalConnected,
+                   weak_ptr_factory_.GetWeakPtr()));
+  }
+
+ private:
+  void OnStartEnrollSession(const ObjectPathCallback& callback,
+                            dbus::Response* response) {
+    dbus::MessageReader reader(response);

[Daniel Erat, 2017/03/27 18:18:25]

i think that all of these methods will segfault if there's an error due to e.g.
biod not running. you need to check that |response| is non-null. did you test
this case?

[sammiequon, 2017/03/27 23:21:48]

Done.

+    dbus::ObjectPath result;
+    if (reader.PopObjectPath(&result))
+      callback.Run(result);

[Daniel Erat, 2017/03/27 18:18:25]

please make all of these methods log errors if incorrect arguments are supplied.
otherwise, all of these methods will silently fail if there's a bug in biod.

[sammiequon, 2017/03/27 23:21:48]

Done.

+  }
+
+  void OnGetRecordsForUser(const UserRecordsCallback& callback,
+                           dbus::Response* response) {
+    dbus::MessageReader reader(response);
+    std::vector<dbus::ObjectPath> result;
+    if (reader.PopArrayOfObjectPaths(&result))
+      callback.Run(result);
+  }
+
+  void OnStartAuthSession(const ObjectPathCallback& callback,
+                          dbus::Response* response) {
+    dbus::MessageReader reader(response);
+    dbus::ObjectPath result;
+    if (reader.PopObjectPath(&result))
+      callback.Run(result);
+  }
+
+  void OnRequestType(const BiometricTypeCallback& callback,
+                     dbus::Response* response) {
+    dbus::MessageReader reader(response);
+    uint32_t result;
+    if (reader.PopVariantOfUint32(&result))
+      callback.Run(static_cast<biod::BiometricType>(result));
+  }
+
+  // Called when the biometrics signal is initially connected.
+  void OnSignalConnected(const std::string& interface_name,
+                         const std::string& signal_name,
+                         bool success) {
+    LOG_IF(ERROR, !success)
+        << "Failed to connect to biometrics signal:" << signal_name;
+  }
+
+  void NameOwnerChangedReceived(const std::string& old_owner,

[rkc, 2017/03/25 00:28:20]

nit: /* old_owner */
/* new_owner */

[sammiequon, 2017/03/27 23:21:48]

Done.

+                                const std::string& new_owner) {
+    for (auto& observer : observers_)
+      observer.ClientRestarted();

[Daniel Erat, 2017/03/27 18:18:26]

this seems incorrect. i think you'll see this method get called twice when biod
gets restarted, once with the old client and "", and once with "" and the new
client. if biod dies and isn't restarted, you'll see it called as well. calling
the "restarted" method when new_owner is empty is misleading.

[sammiequon, 2017/03/27 23:21:48]

Done.

+  }
+
+  void EnrollScanDoneReceived(dbus::Signal* signal) {
+    dbus::MessageReader reader(signal);
+    uint32_t scan_result;
+    bool is_complete;
+    if (!reader.PopUint32(&scan_result) || !reader.PopBool(&is_complete)) {
+      LOG(ERROR) << "Error reading signal from biometrics:"
+                 << signal->ToString();
+      return;
+    }
+
+    for (auto& observer : observers_)
+      observer.EnrollScanDoneReceived(scan_result, is_complete);
+  }
+
+  void AuthScanDoneReceived(dbus::Signal* signal) {
+    dbus::MessageReader signal_reader(signal);
+    dbus::MessageReader array_reader(NULL);

[Daniel Erat, 2017/03/27 18:18:25]

use nullptr instead of NULL in c++ code (also below)

[sammiequon, 2017/03/27 23:21:48]

Done.

+    uint32_t scan_result;
+    AuthScanMatches matches;
+    if (!signal_reader.PopUint32(&scan_result) ||
+        !signal_reader.PopArray(&array_reader)) {
+      LOG(ERROR) << "Error reading signal from biometrics:"
+                 << signal->ToString();
+      return;
+    }
+
+    while (array_reader.HasMoreData()) {
+      dbus::MessageReader entry_reader(NULL);
+      std::string user_id;
+      std::vector<std::string> labels;
+      if (!array_reader.PopDictEntry(&entry_reader) ||
+          !entry_reader.PopString(&user_id) ||
+          !entry_reader.PopArrayOfStrings(&labels)) {
+        LOG(ERROR) << "Error reading signal from biometrics:"
+                   << signal->ToString();
+        return;
+      }
+
+      matches[user_id] = std::move(labels);
+    }
+
+    for (auto& observer : observers_)
+      observer.AuthScanDoneReceived(scan_result, matches);
+  }
+
+  void SessionFailedReceived(dbus::Signal* signal) {
+    for (auto& observer : observers_)
+      observer.SessionFailedReceived();
+  }
+
+  dbus::ObjectProxy* biometrics_manager_proxy_;
+  base::ObserverList<Observer> observers_;
+
+  // Note: This should remain the last member so it'll be destroyed and
+  // invalidate its weak pointers before any other members are destroyed.
+  base::WeakPtrFactory<BiodBiometricsManagerClientImpl> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(BiodBiometricsManagerClientImpl);
+};
+
+BiodBiometricsManagerClient::BiodBiometricsManagerClient() {}
+
+BiodBiometricsManagerClient::~BiodBiometricsManagerClient() {}
+
+// static
+BiodBiometricsManagerClient* BiodBiometricsManagerClient::Create(
+    DBusClientImplementationType /* type */) {
+  return new BiodBiometricsManagerClientImpl();
+}
+
+}  // namespace chromeos