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};
