Remove ObserverList from cloud delegate Change-Id: Icc3e7c11c4b01b1b1105a2be6c48f0ec1b7b6d92 Reviewed-on: https://weave-review.googlesource.com/2832 Reviewed-by: Alex Vakulenko <avakulenko@google.com>
diff --git a/src/privet/cloud_delegate.cc b/src/privet/cloud_delegate.cc index 65e4b87..ef9c59e 100644 --- a/src/privet/cloud_delegate.cc +++ b/src/privet/cloud_delegate.cc
@@ -49,18 +49,10 @@ device_->AddGcdStateChangedCallback(base::Bind( &CloudDelegateImpl::OnRegistrationChanged, weak_factory_.GetWeakPtr())); - component_manager_->AddTraitDefChangedCallback( - base::Bind(&CloudDelegateImpl::NotifyOnTraitDefsChanged, - weak_factory_.GetWeakPtr())); component_manager_->AddCommandAddedCallback(base::Bind( &CloudDelegateImpl::OnCommandAdded, weak_factory_.GetWeakPtr())); component_manager_->AddCommandRemovedCallback(base::Bind( &CloudDelegateImpl::OnCommandRemoved, weak_factory_.GetWeakPtr())); - component_manager_->AddStateChangedCallback(base::Bind( - &CloudDelegateImpl::NotifyOnStateChanged, weak_factory_.GetWeakPtr())); - component_manager_->AddComponentTreeChangedCallback( - base::Bind(&CloudDelegateImpl::NotifyOnComponentTreeChanged, - weak_factory_.GetWeakPtr())); } ~CloudDelegateImpl() override = default; @@ -232,6 +224,18 @@ callback.Run(commands_json, nullptr); } + void AddOnTraitsChangedCallback(const base::Closure& callback) override { + component_manager_->AddTraitDefChangedCallback(callback); + } + + void AddOnStateChangedCallback(const base::Closure& callback) override { + component_manager_->AddStateChangedCallback(callback); + } + + void AddOnComponentsChangeCallback(const base::Closure& callback) override { + component_manager_->AddComponentTreeChangedCallback(callback); + } + private: void OnCommandAdded(Command* command) { // Set to "" for any new unknown command. @@ -373,17 +377,5 @@ new CloudDelegateImpl{task_runner, device, component_manager}}; } -void CloudDelegate::NotifyOnTraitDefsChanged() { - FOR_EACH_OBSERVER(Observer, observer_list_, OnTraitDefsChanged()); -} - -void CloudDelegate::NotifyOnComponentTreeChanged() { - FOR_EACH_OBSERVER(Observer, observer_list_, OnComponentTreeChanged()); -} - -void CloudDelegate::NotifyOnStateChanged() { - FOR_EACH_OBSERVER(Observer, observer_list_, OnStateChanged()); -} - } // namespace privet } // namespace weave
diff --git a/src/privet/cloud_delegate.h b/src/privet/cloud_delegate.h index 4ded884..6d0ed48 100644 --- a/src/privet/cloud_delegate.h +++ b/src/privet/cloud_delegate.h
@@ -11,7 +11,6 @@ #include <base/callback.h> #include <base/memory/ref_counted.h> -#include <base/observer_list.h> #include <weave/device.h> #include "src/privet/privet_types.h" @@ -43,15 +42,6 @@ base::Callback<void(const base::DictionaryValue& commands, ErrorPtr error)>; - class Observer { - public: - virtual ~Observer() {} - - virtual void OnTraitDefsChanged() {} - virtual void OnStateChanged() {} - virtual void OnComponentTreeChanged() {} - }; - // Returns the ID of the device. virtual std::string GetDeviceId() const = 0; @@ -130,23 +120,15 @@ virtual void ListCommands(const UserInfo& user_info, const CommandDoneCallback& callback) = 0; - void AddObserver(Observer* observer) { observer_list_.AddObserver(observer); } - void RemoveObserver(Observer* observer) { - observer_list_.RemoveObserver(observer); - } - - void NotifyOnTraitDefsChanged(); - void NotifyOnStateChanged(); - void NotifyOnComponentTreeChanged(); + virtual void AddOnTraitsChangedCallback(const base::Closure& callback) = 0; + virtual void AddOnStateChangedCallback(const base::Closure& callback) = 0; + virtual void AddOnComponentsChangeCallback(const base::Closure& callback) = 0; // Create default instance. static std::unique_ptr<CloudDelegate> CreateDefault( provider::TaskRunner* task_runner, DeviceRegistrationInfo* device, ComponentManager* component_manager); - - private: - base::ObserverList<Observer> observer_list_; }; } // namespace privet
diff --git a/src/privet/mock_delegates.h b/src/privet/mock_delegates.h index 3fab9d0..3742494 100644 --- a/src/privet/mock_delegates.h +++ b/src/privet/mock_delegates.h
@@ -20,8 +20,10 @@ #include "src/privet/wifi_delegate.h" using testing::_; +using testing::AtLeast; using testing::Return; using testing::ReturnRef; +using testing::SaveArg; using testing::SetArgPointee; namespace weave { @@ -207,6 +209,9 @@ const UserInfo&, const CommandDoneCallback&)); MOCK_METHOD2(ListCommands, void(const UserInfo&, const CommandDoneCallback&)); + MOCK_METHOD1(AddOnTraitsChangedCallback, void(const base::Closure&)); + MOCK_METHOD1(AddOnStateChangedCallback, void(const base::Closure&)); + MOCK_METHOD1(AddOnComponentsChangeCallback, void(const base::Closure&)); MockCloudDelegate() { EXPECT_CALL(*this, GetDeviceId()).WillRepeatedly(Return("TestId")); @@ -232,12 +237,29 @@ EXPECT_CALL(*this, MockGetComponentsForUser(_)) .WillRepeatedly(ReturnRef(test_dict_)); EXPECT_CALL(*this, FindComponent(_, _)).Times(0); + + EXPECT_CALL(*this, AddOnTraitsChangedCallback(_)) + .WillRepeatedly(SaveArg<0>(&on_traits_changed_)); + EXPECT_CALL(*this, AddOnStateChangedCallback(_)) + .WillRepeatedly(SaveArg<0>(&on_state_changed_)); + EXPECT_CALL(*this, AddOnComponentsChangeCallback(_)) + .WillRepeatedly(SaveArg<0>(&on_components_changed_)); } + void NotifyOnTraitDefsChanged() { on_traits_changed_.Run(); } + + void NotifyOnComponentTreeChanged() { on_components_changed_.Run(); } + + void NotifyOnStateChanged() { on_state_changed_.Run(); } + ConnectionState connection_state_{ConnectionState::kOnline}; SetupState setup_state_{SetupState::kNone}; base::DictionaryValue test_dict_; + base::Closure on_traits_changed_; + base::Closure on_state_changed_; + base::Closure on_components_changed_; + private: std::unique_ptr<base::DictionaryValue> GetComponentsForUser( const UserInfo& user_info) const override {
diff --git a/src/privet/privet_handler.cc b/src/privet/privet_handler.cc index 3ee8211..7afeb3b 100644 --- a/src/privet/privet_handler.cc +++ b/src/privet/privet_handler.cc
@@ -398,7 +398,13 @@ CHECK(device_); CHECK(security_); CHECK(clock_); - cloud_observer_.Add(cloud_); + + cloud_->AddOnTraitsChangedCallback(base::Bind( + &PrivetHandler::OnTraitDefsChanged, weak_ptr_factory_.GetWeakPtr())); + cloud_->AddOnStateChangedCallback(base::Bind(&PrivetHandler::OnStateChanged, + weak_ptr_factory_.GetWeakPtr())); + cloud_->AddOnComponentsChangeCallback(base::Bind( + &PrivetHandler::OnComponentTreeChanged, weak_ptr_factory_.GetWeakPtr())); AddHandler("/privet/info", &PrivetHandler::HandleInfo, AuthScope::kNone); AddHandler("/privet/v3/pairing/start", &PrivetHandler::HandlePairingStart,
diff --git a/src/privet/privet_handler.h b/src/privet/privet_handler.h index 4eba329..7b212e4 100644 --- a/src/privet/privet_handler.h +++ b/src/privet/privet_handler.h
@@ -11,7 +11,6 @@ #include <base/macros.h> #include <base/memory/weak_ptr.h> -#include <base/scoped_observer.h> #include <base/time/default_clock.h> #include "src/privet/cloud_delegate.h" @@ -31,7 +30,7 @@ // Privet V3 HTTP/HTTPS requests handler. // API details at https://developers.google.com/cloud-devices/ -class PrivetHandler : public CloudDelegate::Observer { +class PrivetHandler { public: // Callback to handle requests asynchronously. // |status| is HTTP status code. @@ -45,11 +44,7 @@ SecurityDelegate* pairing, WifiDelegate* wifi, base::Clock* clock = nullptr); - ~PrivetHandler() override; - - void OnTraitDefsChanged() override; - void OnStateChanged() override; - void OnComponentTreeChanged() override; + ~PrivetHandler(); std::vector<std::string> GetHttpPaths() const; std::vector<std::string> GetHttpsPaths() const; @@ -138,6 +133,10 @@ void ReplyToUpdateRequest(const RequestCallback& callback) const; void OnUpdateRequestTimeout(int update_request_id); + void OnTraitDefsChanged(); + void OnStateChanged(); + void OnComponentTreeChanged(); + CloudDelegate* cloud_{nullptr}; DeviceDelegate* device_{nullptr}; SecurityDelegate* security_{nullptr}; @@ -165,7 +164,6 @@ uint64_t state_fingerprint_{1}; uint64_t traits_fingerprint_{1}; uint64_t components_fingerprint_{1}; - ScopedObserver<CloudDelegate, CloudDelegate::Observer> cloud_observer_{this}; base::WeakPtrFactory<PrivetHandler> weak_ptr_factory_{this};