From e7e3802f6dc5ac415ef926ee022d1e26f10a50b9 Mon Sep 17 00:00:00 2001 From: "armansito@chromium.org" Date: Fri, 1 Nov 2013 17:20:51 +0000 Subject: nfc: Move object proxy update logic based on properties to DBusObjectMap. Added nfc_client_helpers::DBusObjectMap::UpdateObjects. This simplifies the NFC client classes in the way they keep track of added and removed remote objects based on D-Bus properties that they keep track of. This also reduces the size of changes that need to be made once we transition to neard 0.14, which will use DBus.ObjectManager. BUG=304979 Review URL: https://codereview.chromium.org/45333006 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@232421 0039d316-1c4b-4281-b951-d872f2087c98 --- chromeos/dbus/nfc_adapter_client.cc | 73 +++++++++---------------- chromeos/dbus/nfc_client_helpers.cc | 49 ++++++++++++++++- chromeos/dbus/nfc_client_helpers.h | 22 +++++--- chromeos/dbus/nfc_client_unittest.cc | 102 +++++++++++++++++------------------ chromeos/dbus/nfc_device_client.cc | 55 +++++-------------- chromeos/dbus/nfc_device_client.h | 14 ++--- chromeos/dbus/nfc_tag_client.cc | 55 +++++-------------- chromeos/dbus/nfc_tag_client.h | 12 ++--- 8 files changed, 176 insertions(+), 206 deletions(-) (limited to 'chromeos') diff --git a/chromeos/dbus/nfc_adapter_client.cc b/chromeos/dbus/nfc_adapter_client.cc index 040db2a..476aaf2 100644 --- a/chromeos/dbus/nfc_adapter_client.cc +++ b/chromeos/dbus/nfc_adapter_client.cc @@ -43,8 +43,7 @@ class NfcAdapterClientImpl public nfc_client_helpers::DBusObjectMap::Delegate { public: explicit NfcAdapterClientImpl(NfcManagerClient* manager_client) - : initial_adapters_received_(false), - bus_(NULL), + : bus_(NULL), manager_client_(manager_client), weak_ptr_factory_(this) { DCHECK(manager_client); @@ -138,51 +137,23 @@ class NfcAdapterClientImpl private: // NfcManagerClient::Observer override. - virtual void AdapterAdded(const dbus::ObjectPath& object_path) OVERRIDE { - VLOG(1) << "AdapterAdded: " << object_path.value(); - // Initialize the object proxy here, so that observers can start receiving - // notifications for it and it is cached for reuse. Note that, even if we - // miss this signal, a proxy will be created on demand for any object paths - // that are passed to the public methods later. - object_map_->AddObject(object_path); - FOR_EACH_OBSERVER(NfcAdapterClient::Observer, observers_, - AdapterAdded(object_path)); - } - - // NfcManagerClient::Observer override. - virtual void AdapterRemoved(const dbus::ObjectPath& object_path) OVERRIDE { - VLOG(1) << "AdapterRemoved: " << object_path.value(); - // Remove the object proxy, as we know that the adapter no longer exists. - // Note that this doesn't prevent a client from recreating a proxy for an - // object path that is no longer valid, as proxies are created on demand as - // necessary by the public methods. - object_map_->RemoveObject(object_path); - FOR_EACH_OBSERVER(NfcAdapterClient::Observer, observers_, - AdapterRemoved(object_path)); - } - - // NfcManagerClient::Observer override. virtual void ManagerPropertyChanged( const std::string& property_name) OVERRIDE { + // Update the adapter proxies. + DCHECK(manager_client_); NfcManagerClient::Properties* manager_properties = manager_client_->GetProperties(); - if (!initial_adapters_received_ && - property_name == manager_properties->adapters.name()) { - initial_adapters_received_ = true; - VLOG(1) << "Initial set of adapters received."; - // Create proxies for all adapters that are known to the manager, so that - // observers can start getting notified for any signals emitted by - // adapters. We use the PropertyChanged signal from manager only to - // create proxies for the initial fetch. We rely on the AdapterAdded and - // AdapterRemoved signals after that. - std::vector adapters = - manager_properties->adapters.value(); - for (std::vector::iterator iter = adapters.begin(); - iter != adapters.end(); ++iter) { - VLOG(1) << "Creating proxy for: " << iter->value(); - object_map_->AddObject(*iter); - } - } + + // Ignore changes to properties other than "Adapters". + if (property_name != manager_properties->adapters.name()) + return; + + VLOG(1) << "NFC adapters changed."; + + // Update the known adapters. + const std::vector& received_adapters = + manager_properties->adapters.value(); + object_map_->UpdateObjects(received_adapters); } // nfc_client_helpers::DBusObjectMap::Delegate override. @@ -195,6 +166,17 @@ class NfcAdapterClientImpl object_proxy->object_path())); } + // nfc_client_helpers::DBusObjectMap::Delegate override. + virtual void ObjectAdded(const dbus::ObjectPath& object_path) OVERRIDE { + FOR_EACH_OBSERVER(NfcAdapterClient::Observer, observers_, + AdapterAdded(object_path)); + } + + virtual void ObjectRemoved(const dbus::ObjectPath& object_path) OVERRIDE { + FOR_EACH_OBSERVER(NfcAdapterClient::Observer, observers_, + AdapterRemoved(object_path)); + } + // Called by NfcPropertySet when a property value is changed, either by // result of a signal or response to a GetAll() or Get() call. void OnPropertyChanged(const dbus::ObjectPath& object_path, @@ -205,11 +187,6 @@ class NfcAdapterClientImpl AdapterPropertyChanged(object_path, property_name)); } - // This variable stores whether or not we have ever been notified of - // ManagerPropertiesChanged. This is used to bootstrap adapter proxies - // after receiving the initial set of properties from the NFC manager. - bool initial_adapters_received_; - // We maintain a pointer to the bus to be able to request proxies for // new NFC adapters that appear. dbus::Bus* bus_; diff --git a/chromeos/dbus/nfc_client_helpers.cc b/chromeos/dbus/nfc_client_helpers.cc index 51889e81..0e5add2 100644 --- a/chromeos/dbus/nfc_client_helpers.cc +++ b/chromeos/dbus/nfc_client_helpers.cc @@ -4,6 +4,8 @@ #include "chromeos/dbus/nfc_client_helpers.h" +#include "base/stl_util.h" + namespace chromeos { namespace nfc_client_helpers { @@ -61,12 +63,50 @@ NfcPropertySet* DBusObjectMap::GetObjectProperties( return GetObjectPropertyPair(object_path).second; } +void DBusObjectMap::UpdateObjects( + const std::vector& object_paths) { + // This set is used to query if an object path was removed, while updating + // the removed paths below. The iterator is used as a hint to accelerate + // insertion. + std::set object_path_set; + std::set::iterator object_path_set_iter = + object_path_set.begin(); + + // Add all objects. + for (std::vector::const_iterator iter = + object_paths.begin(); + iter != object_paths.end(); ++iter) { + const dbus::ObjectPath &object_path = *iter; + AddObject(object_path); + // Neard usually returns object paths in ascending order. Give a hint here + // to make insertion amortized constant. + object_path_set_iter = + object_path_set.insert(object_path_set_iter, object_path); + } + + // Remove all objects that are not in |object_paths|. + ObjectMap::const_iterator iter = object_map_.begin(); + while (iter != object_map_.end()) { + // Make a copy here, as the iterator will be invalidated by + // DBusObjectMap::RemoveObject below, but |object_path| is needed to + // notify observers right after. We want to make sure that we notify + // the observers AFTER we remove the object, so that method calls + // to the removed object paths in observer implementations fail + // gracefully. + dbus::ObjectPath object_path = iter->first; + ++iter; + if (!ContainsKey(object_path_set, object_path)) + RemoveObject(object_path); + } +} + bool DBusObjectMap::AddObject(const dbus::ObjectPath& object_path) { ObjectMap::iterator iter = object_map_.find(object_path); if (iter != object_map_.end()) return false; DCHECK(bus_); + DCHECK(delegate_); dbus::ObjectProxy* object_proxy = bus_->GetObjectProxy(service_name_, object_path); @@ -74,15 +114,17 @@ bool DBusObjectMap::AddObject(const dbus::ObjectPath& object_path) { NfcPropertySet* properties = delegate_->CreateProperties(object_proxy); properties->ConnectSignals(); properties->GetAll(); - VLOG(1) << "Created proxy for object with Path: " << object_path.value() - << ", Service: " << service_name_; ObjectPropertyPair object = std::make_pair(object_proxy, properties); object_map_[object_path] = object; + VLOG(1) << "Created proxy for object with Path: " << object_path.value() + << ", Service: " << service_name_; + delegate_->ObjectAdded(object_path); return true; } void DBusObjectMap::RemoveObject(const dbus::ObjectPath& object_path) { DCHECK(bus_); + DCHECK(delegate_); ObjectMap::iterator iter = object_map_.find(object_path); if (iter == object_map_.end()) return; @@ -90,6 +132,9 @@ void DBusObjectMap::RemoveObject(const dbus::ObjectPath& object_path) { ObjectPropertyPair pair = iter->second; CleanUpObjectPropertyPair(pair); object_map_.erase(iter); + VLOG(1) << "Object proxy removed for object path: " + << object_path.value(); + delegate_->ObjectRemoved(object_path); } DBusObjectMap::ObjectPropertyPair DBusObjectMap::GetObjectPropertyPair( diff --git a/chromeos/dbus/nfc_client_helpers.h b/chromeos/dbus/nfc_client_helpers.h index ed344db..2d579f2c 100644 --- a/chromeos/dbus/nfc_client_helpers.h +++ b/chromeos/dbus/nfc_client_helpers.h @@ -62,10 +62,15 @@ class CHROMEOS_EXPORT DBusObjectMap { // signals and update the properties. virtual NfcPropertySet* CreateProperties( dbus::ObjectProxy* object_proxy) = 0; - }; - typedef std::pair ObjectPropertyPair; - typedef std::map ObjectMap; + // Notifies the delegate that an object was added with object path + // |object_path|. + virtual void ObjectAdded(const dbus::ObjectPath& object_path) {} + + // Notifies the delegate that an object was removed with object path + // |object_path|. + virtual void ObjectRemoved(const dbus::ObjectPath& object_path) {} + }; // Constructor takes in the D-Bus service name the proxies belong to and // the delegate which will be used to construct properties structures. @@ -85,6 +90,11 @@ class CHROMEOS_EXPORT DBusObjectMap { // returns NULL. NfcPropertySet* GetObjectProperties(const dbus::ObjectPath& object_path); + // Updates the object proxies from the given list of object paths + // |object_paths|. It notifies the delegate of each added and removed object + // via |Delegate::ObjectAdded| and |Delegate::ObjectRemoved|. + void UpdateObjects(const std::vector& object_paths); + // Creates and stores an object proxy and properties structure for a remote // object with object path |object_path|. If an object proxy was already // created, this operation returns false; returns true otherwise. @@ -94,10 +104,10 @@ class CHROMEOS_EXPORT DBusObjectMap { // remote object with object path |object_path|. void RemoveObject(const dbus::ObjectPath& object_path); - // Returns the underlying map. - const ObjectMap& object_map() const { return object_map_; } - private: + typedef std::pair ObjectPropertyPair; + typedef std::map ObjectMap; + // Returns an instance of ObjectPropertyPair by looking up |object_path| in // |object_map_|. If no entry is found, returns an instance that contains // NULL pointers. diff --git a/chromeos/dbus/nfc_client_unittest.cc b/chromeos/dbus/nfc_client_unittest.cc index 7327d9e..512b7de 100644 --- a/chromeos/dbus/nfc_client_unittest.cc +++ b/chromeos/dbus/nfc_client_unittest.cc @@ -53,20 +53,16 @@ class MockNfcAdapterObserver : public NfcAdapterClient::Observer { class MockNfcDeviceObserver : public NfcDeviceClient::Observer { public: - MOCK_METHOD2(DeviceFound, void(const dbus::ObjectPath&, - const dbus::ObjectPath&)); - MOCK_METHOD2(DeviceLost, void(const dbus::ObjectPath&, - const dbus::ObjectPath&)); + MOCK_METHOD1(DeviceFound, void(const dbus::ObjectPath&)); + MOCK_METHOD1(DeviceLost, void(const dbus::ObjectPath&)); MOCK_METHOD2(DevicePropertyChanged, void(const dbus::ObjectPath&, const std::string&)); }; class MockNfcTagObserver : public NfcTagClient::Observer { public: - MOCK_METHOD2(TagFound, void(const dbus::ObjectPath&, - const dbus::ObjectPath&)); - MOCK_METHOD2(TagLost, void(const dbus::ObjectPath&, - const dbus::ObjectPath&)); + MOCK_METHOD1(TagFound, void(const dbus::ObjectPath&)); + MOCK_METHOD1(TagLost, void(const dbus::ObjectPath&)); MOCK_METHOD2(TagPropertyChanged, void(const dbus::ObjectPath&, const std::string&)); }; @@ -189,28 +185,19 @@ class NfcClientTest : public testing::Test { mock_bus_->ShutdownAndBlock(); } - void SimulateAdapterAdded(const dbus::ObjectPath& object_path) { - EXPECT_CALL(mock_manager_observer_, AdapterAdded(object_path)); - EXPECT_CALL(mock_adapter_observer_, AdapterAdded(object_path)); - dbus::Signal signal(nfc_manager::kNfcManagerInterface, - nfc_manager::kAdapterAddedSignal); - dbus::MessageWriter writer(&signal); - writer.AppendObjectPath(object_path); - ASSERT_FALSE(manager_adapter_added_signal_callback_.is_null()); - manager_adapter_added_signal_callback_.Run(&signal); - } - - void SimulateAdapterRemoved(const dbus::ObjectPath& object_path) { - EXPECT_CALL(mock_manager_observer_, AdapterRemoved(object_path)); - EXPECT_CALL(mock_adapter_observer_, AdapterRemoved(object_path)); - dbus::Signal signal(nfc_manager::kNfcManagerInterface, - nfc_manager::kAdapterRemovedSignal); - dbus::MessageWriter writer(&signal); - writer.AppendObjectPath(object_path); - ASSERT_FALSE(manager_adapter_removed_signal_callback_.is_null()); - manager_adapter_removed_signal_callback_.Run(&signal); + void SimulateAdaptersChanged( + const std::vector& adapter_paths) { + NfcManagerClient::Properties* properties = + manager_client_->GetProperties(); + ASSERT_TRUE(properties); + EXPECT_CALL(mock_manager_observer_, + ManagerPropertyChanged(nfc_manager::kAdaptersProperty)); + SendArrayPropertyChangedSignal( + properties, + nfc_manager::kNfcManagerInterface, + nfc_manager::kAdaptersProperty, + adapter_paths); Mock::VerifyAndClearExpectations(&mock_manager_observer_); - Mock::VerifyAndClearExpectations(&mock_adapter_observer_); } void SimulateTagsChanged(const std::vector& tag_paths, @@ -328,7 +315,11 @@ TEST_F(NfcClientTest, AdaptersAddedAndRemoved) { Mock::VerifyAndClearExpectations(this); // Add adapter 0. - SimulateAdapterAdded(dbus::ObjectPath(kTestAdapterPath0)); + std::vector adapter_paths; + adapter_paths.push_back(dbus::ObjectPath(kTestAdapterPath0)); + EXPECT_CALL(mock_adapter_observer_, + AdapterAdded(dbus::ObjectPath(kTestAdapterPath0))); + SimulateAdaptersChanged(adapter_paths); // Invoking methods should succeed on adapter 0 but fail on adapter 1. EXPECT_CALL(*mock_adapter0_proxy_, CallMethodWithErrorCallback(_, _, _, _)); @@ -351,7 +342,10 @@ TEST_F(NfcClientTest, AdaptersAddedAndRemoved) { Mock::VerifyAndClearExpectations(&mock_adapter1_proxy_); // Add adapter 1. - SimulateAdapterAdded(dbus::ObjectPath(kTestAdapterPath1)); + adapter_paths.push_back(dbus::ObjectPath(kTestAdapterPath1)); + EXPECT_CALL(mock_adapter_observer_, + AdapterAdded(dbus::ObjectPath(kTestAdapterPath1))); + SimulateAdaptersChanged(adapter_paths); // Invoking methods should succeed on both adapters. EXPECT_CALL(*mock_adapter0_proxy_, CallMethodWithErrorCallback(_, _, _, _)); @@ -370,7 +364,10 @@ TEST_F(NfcClientTest, AdaptersAddedAndRemoved) { Mock::VerifyAndClearExpectations(&mock_adapter1_proxy_); // Remove adapter 0. - SimulateAdapterRemoved(dbus::ObjectPath(kTestAdapterPath0)); + adapter_paths.erase(adapter_paths.begin()); + EXPECT_CALL(mock_adapter_observer_, + AdapterRemoved(dbus::ObjectPath(kTestAdapterPath0))); + SimulateAdaptersChanged(adapter_paths); // Invoking methods should succeed on adapter 1 but fail on adapter 0. EXPECT_CALL(*this, @@ -394,7 +391,10 @@ TEST_F(NfcClientTest, AdaptersAddedAndRemoved) { Mock::VerifyAndClearExpectations(&mock_adapter1_proxy_); // Remove adapter 1. - SimulateAdapterRemoved(dbus::ObjectPath(kTestAdapterPath1)); + adapter_paths.clear(); + EXPECT_CALL(mock_adapter_observer_, + AdapterRemoved(dbus::ObjectPath(kTestAdapterPath1))); + SimulateAdaptersChanged(adapter_paths); // Invoking methods should fail on both adapters. EXPECT_CALL(*this, @@ -433,14 +433,17 @@ TEST_F(NfcClientTest, TagsAddedAndRemoved) { Mock::VerifyAndClearExpectations(this); // Add adapter 0. - SimulateAdapterAdded(dbus::ObjectPath(kTestAdapterPath0)); + std::vector adapter_paths; + adapter_paths.push_back(dbus::ObjectPath(kTestAdapterPath0)); + EXPECT_CALL(mock_adapter_observer_, + AdapterAdded(dbus::ObjectPath(kTestAdapterPath0))); + SimulateAdaptersChanged(adapter_paths); // Add tag 0. std::vector tag_paths; tag_paths.push_back(dbus::ObjectPath(kTestTagPath0)); EXPECT_CALL(mock_tag_observer_, - TagFound(dbus::ObjectPath(kTestTagPath0), - dbus::ObjectPath(kTestAdapterPath0))); + TagFound(dbus::ObjectPath(kTestTagPath0))); SimulateTagsChanged(tag_paths, dbus::ObjectPath(kTestAdapterPath0)); Mock::VerifyAndClearExpectations(&mock_tag_observer_); @@ -467,8 +470,7 @@ TEST_F(NfcClientTest, TagsAddedAndRemoved) { // Add tag 1. tag_paths.push_back(dbus::ObjectPath(kTestTagPath1)); EXPECT_CALL(mock_tag_observer_, - TagFound(dbus::ObjectPath(kTestTagPath1), - dbus::ObjectPath(kTestAdapterPath0))); + TagFound(dbus::ObjectPath(kTestTagPath1))); SimulateTagsChanged(tag_paths, dbus::ObjectPath(kTestAdapterPath0)); Mock::VerifyAndClearExpectations(&mock_tag_observer_); @@ -491,8 +493,7 @@ TEST_F(NfcClientTest, TagsAddedAndRemoved) { // Remove tag 0. tag_paths.erase(tag_paths.begin()); EXPECT_CALL(mock_tag_observer_, - TagLost(dbus::ObjectPath(kTestTagPath0), - dbus::ObjectPath(kTestAdapterPath0))); + TagLost(dbus::ObjectPath(kTestTagPath0))); SimulateTagsChanged(tag_paths, dbus::ObjectPath(kTestAdapterPath0)); Mock::VerifyAndClearExpectations(&mock_tag_observer_); @@ -519,8 +520,7 @@ TEST_F(NfcClientTest, TagsAddedAndRemoved) { // Remove tag 1. tag_paths.clear(); EXPECT_CALL(mock_tag_observer_, - TagLost(dbus::ObjectPath(kTestTagPath1), - dbus::ObjectPath(kTestAdapterPath0))); + TagLost(dbus::ObjectPath(kTestTagPath1))); SimulateTagsChanged(tag_paths, dbus::ObjectPath(kTestAdapterPath0)); Mock::VerifyAndClearExpectations(&mock_tag_observer_); @@ -561,14 +561,17 @@ TEST_F(NfcClientTest, DevicesAddedAndRemoved) { Mock::VerifyAndClearExpectations(this); // Add adapter 0. - SimulateAdapterAdded(dbus::ObjectPath(kTestAdapterPath0)); + std::vector adapter_paths; + adapter_paths.push_back(dbus::ObjectPath(kTestAdapterPath0)); + EXPECT_CALL(mock_adapter_observer_, + AdapterAdded(dbus::ObjectPath(kTestAdapterPath0))); + SimulateAdaptersChanged(adapter_paths); // Add device 0. std::vector device_paths; device_paths.push_back(dbus::ObjectPath(kTestDevicePath0)); EXPECT_CALL(mock_device_observer_, - DeviceFound(dbus::ObjectPath(kTestDevicePath0), - dbus::ObjectPath(kTestAdapterPath0))); + DeviceFound(dbus::ObjectPath(kTestDevicePath0))); SimulateDevicesChanged(device_paths, dbus::ObjectPath(kTestAdapterPath0)); Mock::VerifyAndClearExpectations(&mock_device_observer_); @@ -595,8 +598,7 @@ TEST_F(NfcClientTest, DevicesAddedAndRemoved) { // Add device 1. device_paths.push_back(dbus::ObjectPath(kTestDevicePath1)); EXPECT_CALL(mock_device_observer_, - DeviceFound(dbus::ObjectPath(kTestDevicePath1), - dbus::ObjectPath(kTestAdapterPath0))); + DeviceFound(dbus::ObjectPath(kTestDevicePath1))); SimulateDevicesChanged(device_paths, dbus::ObjectPath(kTestAdapterPath0)); Mock::VerifyAndClearExpectations(&mock_device_observer_); @@ -619,8 +621,7 @@ TEST_F(NfcClientTest, DevicesAddedAndRemoved) { // Remove device 0. device_paths.erase(device_paths.begin()); EXPECT_CALL(mock_device_observer_, - DeviceLost(dbus::ObjectPath(kTestDevicePath0), - dbus::ObjectPath(kTestAdapterPath0))); + DeviceLost(dbus::ObjectPath(kTestDevicePath0))); SimulateDevicesChanged(device_paths, dbus::ObjectPath(kTestAdapterPath0)); Mock::VerifyAndClearExpectations(&mock_device_observer_); @@ -647,8 +648,7 @@ TEST_F(NfcClientTest, DevicesAddedAndRemoved) { // Remove device 1. device_paths.clear(); EXPECT_CALL(mock_device_observer_, - DeviceLost(dbus::ObjectPath(kTestDevicePath1), - dbus::ObjectPath(kTestAdapterPath0))); + DeviceLost(dbus::ObjectPath(kTestDevicePath1))); SimulateDevicesChanged(device_paths, dbus::ObjectPath(kTestAdapterPath0)); Mock::VerifyAndClearExpectations(&mock_device_observer_); diff --git a/chromeos/dbus/nfc_device_client.cc b/chromeos/dbus/nfc_device_client.cc index 83076d2..c1a212a 100644 --- a/chromeos/dbus/nfc_device_client.cc +++ b/chromeos/dbus/nfc_device_client.cc @@ -9,7 +9,6 @@ #include "base/bind.h" #include "base/memory/weak_ptr.h" #include "base/observer_list.h" -#include "base/stl_util.h" #include "base/strings/stringprintf.h" #include "chromeos/dbus/fake_nfc_device_client.h" #include "chromeos/dbus/nfc_adapter_client.h" @@ -21,12 +20,6 @@ using chromeos::nfc_client_helpers::DBusObjectMap; namespace chromeos { -namespace { - -typedef std::vector ObjectPathVector; - -} // namespace - NfcDeviceClient::Properties::Properties( dbus::ObjectProxy* object_proxy, const PropertyChangedCallback& callback) @@ -150,41 +143,10 @@ class NfcDeviceClientImpl : public NfcDeviceClient, VLOG(1) << "NFC devices changed."; - // Add all the new devices. - const ObjectPathVector& received_devices = + // Update the known devices. + const std::vector& received_devices = adapter_properties->devices.value(); - for (ObjectPathVector::const_iterator iter = received_devices.begin(); - iter != received_devices.end(); ++iter) { - const dbus::ObjectPath &device_path = *iter; - if (object_map_->AddObject(device_path)) { - VLOG(1) << "Found NFC device: " << device_path.value(); - FOR_EACH_OBSERVER(NfcDeviceClient::Observer, observers_, - DeviceFound(device_path, object_path)); - } - } - - // Remove all devices that were lost. - const DBusObjectMap::ObjectMap& known_devices = - object_map_->object_map(); - std::set devices_set(received_devices.begin(), - received_devices.end()); - DBusObjectMap::ObjectMap::const_iterator iter = known_devices.begin(); - while (iter != known_devices.end()) { - // Make a copy here, as the iterator will be invalidated by - // DBusObjectMap::RemoveObject below, but |device_path| is needed to - // notify observers right after. We want to make sure that we notify - // the observers AFTER we remove the object, so that method calls - // to the removed object paths in observer implementations fail - // gracefully. - dbus::ObjectPath device_path = iter->first; - ++iter; - if (!ContainsKey(devices_set, device_path)) { - VLOG(1) << "Lost NFC device: " << device_path.value(); - object_map_->RemoveObject(device_path); - FOR_EACH_OBSERVER(NfcDeviceClient::Observer, observers_, - DeviceLost(device_path, object_path)); - } - } + object_map_->UpdateObjects(received_devices); } // nfc_client_helpers::DBusObjectMap::Delegate override. @@ -197,6 +159,17 @@ class NfcDeviceClientImpl : public NfcDeviceClient, object_proxy->object_path())); } + // nfc_client_helpers::DBusObjectMap::Delegate override. + virtual void ObjectAdded(const dbus::ObjectPath& object_path) OVERRIDE { + FOR_EACH_OBSERVER(NfcDeviceClient::Observer, observers_, + DeviceFound(object_path)); + } + + virtual void ObjectRemoved(const dbus::ObjectPath& object_path) OVERRIDE { + FOR_EACH_OBSERVER(NfcDeviceClient::Observer, observers_, + DeviceLost(object_path)); + } + // Called by NfcPropertySet when a property value is changed, either by // result of a signal or response to a GetAll() or Get() call. void OnPropertyChanged(const dbus::ObjectPath& object_path, diff --git a/chromeos/dbus/nfc_device_client.h b/chromeos/dbus/nfc_device_client.h index bbabf7e..314a70a 100644 --- a/chromeos/dbus/nfc_device_client.h +++ b/chromeos/dbus/nfc_device_client.h @@ -23,7 +23,7 @@ namespace chromeos { class NfcAdapterClient; // NfcDeviceClient is used to communicate with objects representing remote NFC -// adapters that can be communicated with. +// devices that can be communicated with. class CHROMEOS_EXPORT NfcDeviceClient : public DBusClient { public: // Structure of properties associated with an NFC device. @@ -42,16 +42,12 @@ class CHROMEOS_EXPORT NfcDeviceClient : public DBusClient { virtual ~Observer() {} // Called when a remote NFC device with the object |object_path| is added - // to the set of known devices associated with the adapter with object path - // |adapter_path|. - virtual void DeviceFound(const dbus::ObjectPath& object_path, - const dbus::ObjectPath& adapter_path) {} + // to the set of known devices. + virtual void DeviceFound(const dbus::ObjectPath& object_path) {} // Called when a remote NFC device with the object path |object_path| is - // removed from the set of known devices associated with the adapter with - // object path |adapter_path|. - virtual void DeviceLost(const dbus::ObjectPath& object_path, - const dbus::ObjectPath& adapter_path) {} + // removed from the set of known devices. + virtual void DeviceLost(const dbus::ObjectPath& object_path) {} // Called when the device property with the name |property_name| on device // with object path |object_path| has acquired a new value. diff --git a/chromeos/dbus/nfc_tag_client.cc b/chromeos/dbus/nfc_tag_client.cc index 37f3c0a..0f44791 100644 --- a/chromeos/dbus/nfc_tag_client.cc +++ b/chromeos/dbus/nfc_tag_client.cc @@ -9,7 +9,6 @@ #include "base/bind.h" #include "base/memory/weak_ptr.h" #include "base/observer_list.h" -#include "base/stl_util.h" #include "base/strings/stringprintf.h" #include "chromeos/dbus/fake_nfc_tag_client.h" #include "chromeos/dbus/nfc_adapter_client.h" @@ -21,12 +20,6 @@ using chromeos::nfc_client_helpers::DBusObjectMap; namespace chromeos { -namespace { - -typedef std::vector ObjectPathVector; - -} // namespace - NfcTagClient::Properties::Properties( dbus::ObjectProxy* object_proxy, const PropertyChangedCallback& callback) @@ -153,41 +146,10 @@ class NfcTagClientImpl : public NfcTagClient, VLOG(1) << "NFC tags changed."; - // Add all the new tags. - const ObjectPathVector& received_tags = + // Update the known tags. + const std::vector& received_tags = adapter_properties->tags.value(); - for (ObjectPathVector::const_iterator iter = received_tags.begin(); - iter != received_tags.end(); ++iter) { - const dbus::ObjectPath &tag_path = *iter; - if (object_map_->AddObject(tag_path)) { - VLOG(1) << "Found NFC tag: " << tag_path.value(); - FOR_EACH_OBSERVER(NfcTagClient::Observer, observers_, - TagFound(tag_path, object_path)); - } - } - - // Remove all tags that were lost. - const DBusObjectMap::ObjectMap& known_tags = - object_map_->object_map(); - std::set tags_set(received_tags.begin(), - received_tags.end()); - DBusObjectMap::ObjectMap::const_iterator iter = known_tags.begin(); - while (iter != known_tags.end()) { - // Make a copy here, as the iterator will be invalidated by - // DBusObjectMap::RemoveObject below, but |device_path| is needed to - // notify observers right after. We want to make sure that we notify - // the observers AFTER we remove the object, so that method calls - // to the removed object paths in observer implementations fail - // gracefully. - dbus::ObjectPath tag_path = iter->first; - ++iter; - if (!ContainsKey(tags_set, tag_path)) { - VLOG(1) << "Lost NFC tag: " << tag_path.value(); - object_map_->RemoveObject(tag_path); - FOR_EACH_OBSERVER(NfcTagClient::Observer, observers_, - TagLost(tag_path, object_path)); - } - } + object_map_->UpdateObjects(received_tags); } // nfc_client_helpers::DBusObjectMap::Delegate override. @@ -200,6 +162,17 @@ class NfcTagClientImpl : public NfcTagClient, object_proxy->object_path())); } + // nfc_client_helpers::DBusObjectMap::Delegate override. + virtual void ObjectAdded(const dbus::ObjectPath& object_path) OVERRIDE { + FOR_EACH_OBSERVER(NfcTagClient::Observer, observers_, + TagFound(object_path)); + } + + virtual void ObjectRemoved(const dbus::ObjectPath& object_path) OVERRIDE { + FOR_EACH_OBSERVER(NfcTagClient::Observer, observers_, + TagLost(object_path)); + } + // Called by NfcPropertySet when a property value is changed, either by // result of a signal or response to a GetAll() or Get() call. void OnPropertyChanged(const dbus::ObjectPath& object_path, diff --git a/chromeos/dbus/nfc_tag_client.h b/chromeos/dbus/nfc_tag_client.h index 5d7e1e0..a719e5a 100644 --- a/chromeos/dbus/nfc_tag_client.h +++ b/chromeos/dbus/nfc_tag_client.h @@ -53,16 +53,12 @@ class CHROMEOS_EXPORT NfcTagClient : public DBusClient { virtual ~Observer() {} // Called when a remote NFC tag with the object path |object_path| is added - // to the set of known tags associated with the adapter with object path - // |adapter_path|. - virtual void TagFound(const dbus::ObjectPath& object_path, - const dbus::ObjectPath& adapter_path) {} + // to the set of known tags. + virtual void TagFound(const dbus::ObjectPath& object_path) {} // Called when a remote NFC tag with the object path |object_path| is - // removed from the set of known tags associated with the adapter with the - // object path |adapter_path|. - virtual void TagLost(const dbus::ObjectPath& object_path, - const dbus::ObjectPath& adapter_path) {} + // removed from the set of known tags. + virtual void TagLost(const dbus::ObjectPath& object_path) {} // Called when the tag property with the name |property_name| on tag with // object path |object_path| has acquired a new value. -- cgit v1.1