cros: DBUS client to interact with fingerprint DBUS API.

chromeos/dbus/biod/biod_client_unittest.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/biod_client.h"
+
+#include <map>
+#include <string>
+
+#include "base/bind.h"
+#include "base/run_loop.h"
+#include "dbus/mock_bus.h"
+#include "dbus/mock_object_proxy.h"
+#include "dbus/object_path.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using ::testing::_;
+using ::testing::Invoke;
+using ::testing::Return;
+
+namespace chromeos {
+
+namespace {
+
+// Shorthand for a commonly-used constant.
+const char* kInterface = biod::kBiometricsManagerInterface;
+
+void TestObjectPathCallback(dbus::ObjectPath** expected_object_path_out,
+                            const dbus::ObjectPath& object_path) {

[Daniel Erat, 2017/04/01 00:46:27]

you're leaking memory in tests now, i think.

just do this:

  void CopyObjectPath(dbus::ObjectPath* dest_path,
                      const dbus::ObjectPath& src_path) {
    CHECK(dest_path);
    *dest_path = src_path;
  }

and then in tests:

  dbus::ObjectPath returned_path("invalid");
  // pass base::Bind(&CopyObjectPath, &returned_path)
  EXPECT_EQ(kFakeObjectPath, returned_path);

then you don't have any new-allocated memory that can be leaked.

i think i'd recommend initializing it to "invalid" or something similar just so
you can ensure that CopyObjectPath was actually called -- otherwise, you don't
have any way to distinguish between not being called or being called with an
empty path.

[sammiequon, 2017/04/01 01:22:44]

Done. Yeah i thought of this but for some reason I thought using nullptr was the
way to go.

+  ASSERT_FALSE(*expected_object_path_out);
+  *expected_object_path_out = new dbus::ObjectPath(object_path);
+}
+
+void TestObjectPathArrayCallback(
+    std::vector<dbus::ObjectPath>** expected_object_paths_out,

[Daniel Erat, 2017/04/01 00:46:27]

same comment applies here. pass a std::vector<dbus::ObjectPath>* to this method
and then change the body to just be:

  CHECK(dest_object_paths);
  *dest_object_paths = src_object_paths;

[sammiequon, 2017/04/01 01:22:44]

Done.

+    const std::vector<dbus::ObjectPath>& object_paths) {
+  ASSERT_FALSE(*expected_object_paths_out);
+  *expected_object_paths_out = new std::vector<dbus::ObjectPath>();
+  for (size_t i = 0; i < object_paths.size(); ++i)
+    (*expected_object_paths_out)->push_back(object_paths[i]);
+}
+
+// Matcher that verifies that a dbus::Message has member |name|.
+MATCHER_P(HasMember, name, "") {
+  if (arg->GetMember() != name) {
+    *result_listener << "has member " << arg->GetMember();
+    return false;
+  }
+  return true;
+}
+
+// Runs |callback| with |response|. Needed due to ResponseCallback expecting a
+// bare pointer rather than an std::unique_ptr.
+void RunResponseCallback(dbus::ObjectProxy::ResponseCallback callback,
+                         std::unique_ptr<dbus::Response> response) {
+  callback.Run(response.get());
+}
+
+// Implementation of BiodClient::Observer.
+class TestBiodObserver : public BiodClient::Observer {
+ public:
+  TestBiodObserver() {}
+  ~TestBiodObserver() override {}
+
+  int num_enroll_scans_received() const { return num_enroll_scans_received_; }
+  int num_auth_scans_received() const { return num_auth_scans_received_; }
+  int num_failures_received() const { return num_failures_received_; }
+
+  // BiodClient::Observer:
+  void BiodServiceRestarted() override {}
+
+  void BiodEnrollScanDoneReceived(biod::ScanResult scan_result,
+                                  bool enroll_sesion_complete) override {
+    num_enroll_scans_received_++;
+  }
+
+  void BiodAuthScanDoneReceived(biod::ScanResult scan_result,
+                                const AuthScanMatches& matches) override {
+    num_auth_scans_received_++;
+  }
+
+  void BiodSessionFailedReceived() override { num_failures_received_++; }
+
+ private:
+  int num_enroll_scans_received_ = 0;
+  int num_auth_scans_received_ = 0;
+  int num_failures_received_ = 0;
+
+  DISALLOW_COPY_AND_ASSIGN(TestBiodObserver);
+};
+
+}  // namespace
+
+class BiodClientTest : public testing::Test {
+ public:
+  BiodClientTest() {}
+  ~BiodClientTest() override {}
+
+  void SetUp() override {
+    dbus::Bus::Options options;
+    options.bus_type = dbus::Bus::SYSTEM;
+    bus_ = new dbus::MockBus(options);
+
+    proxy_ =
+        new dbus::MockObjectProxy(bus_.get(), biod::kBiodServiceName,
+                                  dbus::ObjectPath(biod::kBiodServicePath));
+
+    // |client_|'s Init() method should request a proxy for communicating with
+    // biometrics api.
+    EXPECT_CALL(*bus_.get(),
+                GetObjectProxy(biod::kBiodServiceName,
+                               dbus::ObjectPath(biod::kBiodServicePath)))
+        .WillRepeatedly(Return(proxy_.get()));
+
+    // Save |client_|'s signal callback.
+    EXPECT_CALL(*proxy_.get(), ConnectToSignal(kInterface, _, _, _))
+        .WillRepeatedly(Invoke(this, &BiodClientTest::ConnectToSignal));
+
+    client_.reset(BiodClient::Create(REAL_DBUS_CLIENT_IMPLEMENTATION));
+    client_->Init(bus_.get());
+
+    // Execute callbacks posted by Init().
+    base::RunLoop().RunUntilIdle();
+  }
+
+ protected:
+  // Add an expectation for method with |method_name| to be called. When the
+  // method is called the response shoudl match |response|.
+  void AddMethodExpectation(const std::string& method_name,
+                            std::unique_ptr<dbus::Response> response) {
+    ASSERT_FALSE(pending_method_calls_.count(method_name));
+    pending_method_calls_[method_name] = std::move(response);
+    EXPECT_CALL(*proxy_.get(), CallMethod(HasMember(method_name), _, _))
+        .WillOnce(Invoke(this, &BiodClientTest::OnCallMethod));
+  }
+
+  // Synchronously passes |signal| to |client_|'s handler, simulating the signal
+  // from biometrics.
+  void EmitSignal(dbus::Signal* signal) {
+    const std::string signal_name = signal->GetMember();
+    const auto it = signal_callbacks_.find(signal_name);
+    ASSERT_TRUE(it != signal_callbacks_.end())
+        << "Client didn't register for signal " << signal_name;
+    it->second.Run(signal);
+  }
+
+  // Passes a enroll scan done signal to |client_|.
+  void EmitEnrollScanDoneSignal(biod::ScanResult scan_result,
+                                bool enroll_session_complete) {
+    dbus::Signal signal(kInterface,
+                        biod::kBiometricsManagerEnrollScanDoneSignal);
+    dbus::MessageWriter writer(&signal);
+    writer.AppendUint32(uint32_t{scan_result});
+    writer.AppendBool(enroll_session_complete);
+    EmitSignal(&signal);
+  }
+
+  // Passes a auth scan done signal to |client_|.
+  void EmitAuthScanDoneSignal(biod::ScanResult scan_result,
+                              const AuthScanMatches& matches) {
+    dbus::Signal signal(kInterface, biod::kBiometricsManagerAuthScanDoneSignal);
+    dbus::MessageWriter writer(&signal);
+    writer.AppendUint32(uint32_t{scan_result});
+
+    dbus::MessageWriter array_writer(nullptr);
+    writer.OpenArray("{sx}", &array_writer);
+    for (auto& match : matches) {
+      dbus::MessageWriter entry_writer(nullptr);
+      array_writer.OpenDictEntry(&entry_writer);
+      entry_writer.AppendString(match.first);
+      entry_writer.AppendArrayOfStrings(match.second);
+      array_writer.CloseContainer(&entry_writer);
+    }
+    writer.CloseContainer(&array_writer);
+    EmitSignal(&signal);
+  }
+
+  // Passes a scan failed signal to |client_|.
+  void EmitScanFailedSignal() {
+    dbus::Signal signal(kInterface,
+                        biod::kBiometricsManagerSessionFailedSignal);
+    EmitSignal(&signal);
+  }
+
+  std::map<std::string, std::unique_ptr<dbus::Response>> pending_method_calls_;
+
+  base::MessageLoop message_loop_;
+
+  // Mock bus and proxy for simulating calls.
+  scoped_refptr<dbus::MockBus> bus_;
+  scoped_refptr<dbus::MockObjectProxy> proxy_;
+
+  std::unique_ptr<BiodClient> client_;
+
+  // Maps from biod signal name to the corresponding callback provided by
+  // |client_|.
+  std::map<std::string, dbus::ObjectProxy::SignalCallback> signal_callbacks_;
+
+ private:
+  // Handles calls to |proxy_|'s ConnectToSignal() method.
+  void ConnectToSignal(
+      const std::string& interface_name,
+      const std::string& signal_name,
+      dbus::ObjectProxy::SignalCallback signal_callback,
+      dbus::ObjectProxy::OnConnectedCallback on_connected_callback) {
+    EXPECT_EQ(interface_name, kInterface);
+    signal_callbacks_[signal_name] = signal_callback;
+    message_loop_.task_runner()->PostTask(
+        FROM_HERE, base::Bind(on_connected_callback, interface_name,
+                              signal_name, true /* success */));
+  }
+
+  // Handles calls to |proxy_|'s CallMethod().
+  void OnCallMethod(dbus::MethodCall* method_call,
+                    int timeout_ms,
+                    const dbus::ObjectProxy::ResponseCallback& callback) {
+    auto it = pending_method_calls_.find(method_call->GetMember());
+    ASSERT_TRUE(it != pending_method_calls_.end());
+    auto pending_response = std::move(it->second);
+    pending_method_calls_.erase(it);
+
+    message_loop_.task_runner()->PostTask(
+        FROM_HERE, base::Bind(&RunResponseCallback, callback,
+                              base::Passed(&pending_response)));
+  }
+
+  DISALLOW_COPY_AND_ASSIGN(BiodClientTest);
+};
+
+TEST_F(BiodClientTest, TestStartEnrollSession) {
+  const std::string kFakeId("fakeId");
+  const std::string kFakeLabel("fakeLabel");
+  const dbus::ObjectPath kFakeObjectPath(std::string("/fake/object/path"));
+  const dbus::ObjectPath kFakeObjectPath2(std::string("/fake/object/path2"));
+
+  std::unique_ptr<dbus::Response> response(dbus::Response::CreateEmpty());
+  dbus::MessageWriter writer(response.get());
+  writer.AppendObjectPath(kFakeObjectPath);
+
+  // Create a fake response with a fake object path. The start enroll
+  // call should return this object path.
+  AddMethodExpectation(biod::kBiometricsManagerStartEnrollSessionMethod,
+                       std::move(response));
+  dbus::ObjectPath* callback_result = nullptr;
+  client_->StartEnrollSession(
+      kFakeId, kFakeLabel,
+      base::Bind(&TestObjectPathCallback, &callback_result));
+  base::RunLoop().RunUntilIdle();
+  ASSERT_TRUE(callback_result);
+  EXPECT_EQ(kFakeObjectPath, *callback_result);
+
+  // Verify that by sending a empty reponse or a improperly formatted one, the
+  // response is an empty object path. Also, logs will get printed.
+  AddMethodExpectation(biod::kBiometricsManagerStartEnrollSessionMethod,
+                       nullptr);
+  callback_result = nullptr;

[Daniel Erat, 2017/04/01 00:46:27]

here's one of the places where memory is being leaked, for example. you'd need
to call delete in each of these places instead of just at the end of the test,
but see the earlier comment about how to avoid new/delete entirely.

[sammiequon, 2017/04/01 01:22:44]

Oops. Yeah.

+  client_->StartEnrollSession(
+      kFakeId, kFakeLabel,
+      base::Bind(&TestObjectPathCallback, &callback_result));
+  base::RunLoop().RunUntilIdle();
+  ASSERT_TRUE(callback_result);

[Daniel Erat, 2017/04/01 00:46:27]

after getting rid of the new calls, you won't need these null checks either

[sammiequon, 2017/04/01 01:22:44]

Done.

+  EXPECT_EQ(dbus::ObjectPath(), *callback_result);
+
+  std::unique_ptr<dbus::Response> bad_response(dbus::Response::CreateEmpty());
+  dbus::MessageWriter bad_writer(bad_response.get());
+  bad_writer.AppendString("");
+  AddMethodExpectation(biod::kBiometricsManagerStartEnrollSessionMethod,
+                       std::move(bad_response));
+  callback_result = nullptr;
+  client_->StartEnrollSession(
+      kFakeId, kFakeLabel,
+      base::Bind(&TestObjectPathCallback, &callback_result));
+  base::RunLoop().RunUntilIdle();
+  ASSERT_TRUE(callback_result);
+  EXPECT_EQ(dbus::ObjectPath(), *callback_result);
+
+  delete callback_result;
+}
+
+TEST_F(BiodClientTest, TestGetRecordsForUser) {
+  const std::string kFakeId("fakeId");
+  const dbus::ObjectPath kFakeObjectPath(std::string("/fake/object/path"));
+  const dbus::ObjectPath kFakeObjectPath2(std::string("/fake/object/path2"));
+  const std::vector<dbus::ObjectPath> kFakeObjectPaths = {kFakeObjectPath,
+                                                          kFakeObjectPath2};
+
+  std::unique_ptr<dbus::Response> response(dbus::Response::CreateEmpty());
+  dbus::MessageWriter writer(response.get());
+  writer.AppendArrayOfObjectPaths(kFakeObjectPaths);
+
+  // Create a fake response with an array of fake object paths. The get
+  // enrollments call should return this array of object paths.
+  AddMethodExpectation(biod::kBiometricsManagerGetRecordsForUserMethod,
+                       std::move(response));
+  std::vector<dbus::ObjectPath>* callback_result = nullptr;
+  client_->GetRecordsForUser(
+      kFakeId, base::Bind(&TestObjectPathArrayCallback, &callback_result));
+  base::RunLoop().RunUntilIdle();
+  ASSERT_TRUE(callback_result);
+  EXPECT_EQ(kFakeObjectPaths, *callback_result);
+
+  // Verify that by sending a empty reponse the response is an empty array of
+  // object paths. Also, logs will get printed.
+  AddMethodExpectation(biod::kBiometricsManagerGetRecordsForUserMethod,
+                       nullptr);
+  callback_result = nullptr;
+  client_->GetRecordsForUser(
+      kFakeId, base::Bind(&TestObjectPathArrayCallback, &callback_result));
+  base::RunLoop().RunUntilIdle();
+  ASSERT_TRUE(callback_result);
+  EXPECT_EQ(std::vector<dbus::ObjectPath>(), *callback_result);
+
+  delete callback_result;
+}
+
+TEST_F(BiodClientTest, TestStartAuthentication) {
+  const dbus::ObjectPath kFakeObjectPath(std::string("/fake/object/path"));
+
+  // Create a fake response with a fake object path. The start authentication
+  // call should return this object path.
+  std::unique_ptr<dbus::Response> response(dbus::Response::CreateEmpty());
+  dbus::MessageWriter writer(response.get());
+  writer.AppendObjectPath(kFakeObjectPath);
+
+  AddMethodExpectation(biod::kBiometricsManagerStartAuthSessionMethod,
+                       std::move(response));
+  dbus::ObjectPath* callback_result = nullptr;
+  client_->StartAuthSession(
+      base::Bind(&TestObjectPathCallback, &callback_result));
+  base::RunLoop().RunUntilIdle();
+  ASSERT_TRUE(callback_result);
+  EXPECT_EQ(kFakeObjectPath, *callback_result);
+
+  // Verify that by sending a empty reponse or a improperly formatted one, the
+  // response is an empty object path. Also, logs will get printed.
+  AddMethodExpectation(biod::kBiometricsManagerStartAuthSessionMethod, nullptr);
+  callback_result = nullptr;
+  client_->StartAuthSession(
+      base::Bind(&TestObjectPathCallback, &callback_result));
+  base::RunLoop().RunUntilIdle();
+  ASSERT_TRUE(callback_result);
+  EXPECT_EQ(dbus::ObjectPath(), *callback_result);
+
+  std::unique_ptr<dbus::Response> bad_response(dbus::Response::CreateEmpty());
+  dbus::MessageWriter bad_writer(bad_response.get());
+  bad_writer.AppendString("");
+  AddMethodExpectation(biod::kBiometricsManagerStartAuthSessionMethod,
+                       std::move(bad_response));
+  callback_result = nullptr;
+  client_->StartAuthSession(
+      base::Bind(&TestObjectPathCallback, &callback_result));
+  base::RunLoop().RunUntilIdle();
+  ASSERT_TRUE(callback_result);
+  EXPECT_EQ(dbus::ObjectPath(), *callback_result);
+
+  delete callback_result;
+}
+
+// Verify when signals are mocked, an observer will catch the signals as
+// expected.
+TEST_F(BiodClientTest, TestEnrollScanDoneObserver) {

[Daniel Erat, 2017/04/01 00:46:27]

nit: TestNotifyObserverAboutScan?

[sammiequon, 2017/04/01 01:22:44]

Done. I opted to skip the AboutScan since I put the failures testing in here
too.

+  TestBiodObserver observer;
+  client_->AddObserver(&observer);
+  EXPECT_TRUE(client_->HasObserver(&observer));
+
+  const biod::ScanResult scan_signal = biod::ScanResult::SCAN_RESULT_SUCCESS;
+  const bool enroll_session_complete = false;
+  const AuthScanMatches test_attempt;
+  EXPECT_EQ(0, observer.num_enroll_scans_received());
+  EXPECT_EQ(0, observer.num_auth_scans_received());
+  EXPECT_EQ(0, observer.num_failures_received());
+
+  EmitEnrollScanDoneSignal(scan_signal, enroll_session_complete);
+  EXPECT_EQ(1, observer.num_enroll_scans_received());
+
+  EmitAuthScanDoneSignal(scan_signal, test_attempt);
+  EXPECT_EQ(1, observer.num_auth_scans_received());
+
+  EmitScanFailedSignal();
+  EXPECT_EQ(1, observer.num_failures_received());
+
+  client_->RemoveObserver(&observer);
+
+  EmitEnrollScanDoneSignal(scan_signal, enroll_session_complete);
+  EmitAuthScanDoneSignal(scan_signal, test_attempt);
+  EXPECT_EQ(1, observer.num_enroll_scans_received());
+  EXPECT_EQ(1, observer.num_auth_scans_received());
+  EXPECT_EQ(1, observer.num_failures_received());
+}
+}  // namespace chromeos