Implement Physical Web listener registration

components/physical_web/data_source/physical_web_data_source_impl_unittest.cc

Index: components/physical_web/data_source/physical_web_data_source_impl_unittest.cc
diff --git a/components/physical_web/data_source/physical_web_data_source_impl_unittest.cc b/components/physical_web/data_source/physical_web_data_source_impl_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..fa3bb0d6fa0f3fc79c1fec102cbaf4010f63d891
--- /dev/null
+++ b/components/physical_web/data_source/physical_web_data_source_impl_unittest.cc
@@ -0,0 +1,174 @@
+// Copyright 2016 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 "base/values.h"
+#include "components/physical_web/data_source/physical_web_data_source_impl.h"
+#include "components/physical_web/data_source/physical_web_listener.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace physical_web {
+
+// TestPhysicalWebDataSource --------------------------------------------------
+
+class TestPhysicalWebDataSource : public PhysicalWebDataSourceImpl {
+ public:
+  TestPhysicalWebDataSource() {}
+  ~TestPhysicalWebDataSource() override {}
+
+  void StartDiscovery(bool network_request_enabled) override;
+  void StopDiscovery() override;
+  std::unique_ptr<base::ListValue> GetMetadata() override;
+  bool HasUnresolvedDiscoveries() override;
+};
+void TestPhysicalWebDataSource::StartDiscovery(bool network_request_enabled) {}
+
+void TestPhysicalWebDataSource::StopDiscovery() {}
+
+std::unique_ptr<base::ListValue> TestPhysicalWebDataSource::GetMetadata() {
+  return NULL;
+}
+
+bool TestPhysicalWebDataSource::HasUnresolvedDiscoveries() {
+  return false;
+}
+
+// TestPhysicalWebListener ----------------------------------------------------
+
+class TestPhysicalWebListener : public PhysicalWebListener {
+ public:
+  TestPhysicalWebListener() {}
+  ~TestPhysicalWebListener() {}
+
+  void OnFound(const std::string& url) override;
+
+  void OnLost(const std::string& url) override;
+
+  void OnDistanceChanged(const std::string& url,
+                         double distance_estimate) override;
+
+  bool OnFoundNotified() { return on_found_notified_; }
+
+  bool OnLostNotified() { return on_lost_notified_; }
+
+  bool OnDistanceChangedNotified() { return on_distance_changed_notified_; }
+
+  std::string LastEventUrl() { return last_event_url_; }
+
+  void ClearNotified();
+
+ private:
+  bool on_found_notified_;
+  bool on_lost_notified_;
+  bool on_distance_changed_notified_;
+  std::string last_event_url_;
+};
+
+void TestPhysicalWebListener::OnFound(const std::string& url) {

[mattreynolds, 2016/09/20 22:49:33]

These methods are pretty short, I think it makes sense to move them into the
class definition

[hayesjordan, 2016/09/21 20:47:44]

Done.

+  on_found_notified_ = true;
+  last_event_url_ = url;
+}
+
+void TestPhysicalWebListener::OnLost(const std::string& url) {
+  on_lost_notified_ = true;
+  last_event_url_ = url;
+}
+
+void TestPhysicalWebListener::OnDistanceChanged(const std::string& url,
+                                                double distance_estimate) {
+  on_distance_changed_notified_ = true;
+  last_event_url_ = url;
+}
+
+void TestPhysicalWebListener::ClearNotified() {

[mattreynolds, 2016/09/20 22:49:33]

Why not initialize these in the TestPhysicalWebListener constructor?

[hayesjordan, 2016/09/21 20:47:44]

Done.

+  on_found_notified_ = false;
+  on_lost_notified_ = false;
+  on_distance_changed_notified_ = false;
+  last_event_url_ = "";

[mattreynolds, 2016/09/20 22:49:34]

last_event_url_.clear();

[hayesjordan, 2016/09/21 20:47:44]

Done.

+}
+
+// PhysicalWebDataSourceImplTest ----------------------------------------------
+
+class PhysicalWebDataSourceImplTest : public ::testing::Test {
+ public:
+  PhysicalWebDataSourceImplTest() {}
+  ~PhysicalWebDataSourceImplTest() override {}
+
+  // testing::Test
+  void SetUp() override;
+  void TearDown() override;
+
+ protected:
+  TestPhysicalWebDataSource data_source_;
+  TestPhysicalWebListener listener_;
+};
+
+void PhysicalWebDataSourceImplTest::SetUp() {
+  listener_.ClearNotified();
+  data_source_.RegisterListener(&listener_);
+}
+
+void PhysicalWebDataSourceImplTest::TearDown() {
+  data_source_.UnregisterListener(&listener_);
+}
+
+// Tests ----------------------------------------------------------------------
+
+TEST_F(PhysicalWebDataSourceImplTest, OnFound) {
+  std::string url = "https://www.google.com";

[mattreynolds, 2016/09/20 22:49:33]

Let's pull out this constant and share it between tests.

[hayesjordan, 2016/09/21 20:47:44]

Done.

+  data_source_.NotifyOnFound(url);
+  EXPECT_TRUE(listener_.OnFoundNotified());
+  EXPECT_FALSE(listener_.OnLostNotified());
+  EXPECT_FALSE(listener_.OnDistanceChangedNotified());
+  EXPECT_EQ(url, listener_.LastEventUrl());
+}
+
+TEST_F(PhysicalWebDataSourceImplTest, OnLost) {
+  std::string url = "https://www.google.com";
+  data_source_.NotifyOnLost(url);
+  EXPECT_FALSE(listener_.OnFoundNotified());
+  EXPECT_TRUE(listener_.OnLostNotified());
+  EXPECT_FALSE(listener_.OnDistanceChangedNotified());
+  EXPECT_EQ(url, listener_.LastEventUrl());
+}
+
+TEST_F(PhysicalWebDataSourceImplTest, OnDistanceChanged) {
+  std::string url = "https://www.google.com";
+  data_source_.NotifyOnDistanceChanged(url, 0.0);
+  EXPECT_FALSE(listener_.OnFoundNotified());
+  EXPECT_FALSE(listener_.OnLostNotified());
+  EXPECT_TRUE(listener_.OnDistanceChangedNotified());
+  EXPECT_EQ(url, listener_.LastEventUrl());
+}
+
+TEST_F(PhysicalWebDataSourceImplTest, OnFoundNotRegistered) {
+  std::string url = "https://www.google.com";
+  data_source_.UnregisterListener(&listener_);
+  data_source_.NotifyOnFound(url);
+  EXPECT_FALSE(listener_.OnFoundNotified());
+  EXPECT_FALSE(listener_.OnLostNotified());
+  EXPECT_FALSE(listener_.OnDistanceChangedNotified());
+  EXPECT_EQ("", listener_.LastEventUrl());

[mattreynolds, 2016/09/20 22:49:33]

Consider changing to EXPECT_TRUE(listener_.LastEventUrl().empty())

[hayesjordan, 2016/09/21 20:47:44]

Done.

+}
+
+TEST_F(PhysicalWebDataSourceImplTest, OnLostNotRegistered) {
+  std::string url = "https://www.google.com";
+  data_source_.UnregisterListener(&listener_);
+  data_source_.NotifyOnLost(url);
+  EXPECT_FALSE(listener_.OnFoundNotified());
+  EXPECT_FALSE(listener_.OnLostNotified());
+  EXPECT_FALSE(listener_.OnDistanceChangedNotified());
+  EXPECT_EQ("", listener_.LastEventUrl());
+}
+
+TEST_F(PhysicalWebDataSourceImplTest, OnDistanceChangedNotregistered) {

[mattreynolds, 2016/09/20 22:49:33]

OnDistanceChangedNotregistered -> OnDistanceChangedNotRegistered

[hayesjordan, 2016/09/21 20:47:44]

Done.

+  std::string url = "https://www.google.com";
+  data_source_.UnregisterListener(&listener_);
+  data_source_.NotifyOnDistanceChanged(url, 0.0);
+  EXPECT_FALSE(listener_.OnFoundNotified());
+  EXPECT_FALSE(listener_.OnLostNotified());
+  EXPECT_FALSE(listener_.OnDistanceChangedNotified());
+  EXPECT_EQ("", listener_.LastEventUrl());
+}
+}

[mattreynolds, 2016/09/20 22:49:33]

Add a comment for the end of the namespace, eg:

}  // namespace physical_web

[hayesjordan, 2016/09/21 20:47:44]

Done.