libweave: Remove ownership of CommandProxyInterface by CommandInstance
Future CommandProxyInterface will be in external code, we don't want
to control their life-cycle.
BUG=brillo:1245
TEST='FEATURES=test emerge-gizmo buffet'
Change-Id: Ife0fd402ee911e3caeb20570f4b4001ebeb9dc58
Reviewed-on: https://chromium-review.googlesource.com/287194
Tested-by: Vitaly Buka <vitalybuka@chromium.org>
Reviewed-by: Alex Vakulenko <avakulenko@chromium.org>
Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
diff --git a/libweave/src/commands/cloud_command_proxy.cc b/libweave/src/commands/cloud_command_proxy.cc
index 1bc4007..cc4d64f 100644
--- a/libweave/src/commands/cloud_command_proxy.cc
+++ b/libweave/src/commands/cloud_command_proxy.cc
@@ -50,6 +50,10 @@
QueueCommandUpdate(std::move(patch));
}
+void CloudCommandProxy::OnCommandDestroyed() {
+ delete this;
+}
+
void CloudCommandProxy::QueueCommandUpdate(
std::unique_ptr<base::DictionaryValue> patch) {
UpdateID id = state_change_queue_->GetLastStateChangeId();
diff --git a/libweave/src/commands/cloud_command_proxy.h b/libweave/src/commands/cloud_command_proxy.h
index 4bacdc9..5e3d42c 100644
--- a/libweave/src/commands/cloud_command_proxy.h
+++ b/libweave/src/commands/cloud_command_proxy.h
@@ -38,6 +38,7 @@
void OnResultsChanged() override;
void OnStatusChanged() override;
void OnProgressChanged() override;
+ void OnCommandDestroyed() override;
private:
using UpdateID = StateChangeQueueInterface::UpdateID;
diff --git a/libweave/src/commands/cloud_command_proxy_unittest.cc b/libweave/src/commands/cloud_command_proxy_unittest.cc
index 501c873..69e0b4b 100644
--- a/libweave/src/commands/cloud_command_proxy_unittest.cc
+++ b/libweave/src/commands/cloud_command_proxy_unittest.cc
@@ -156,7 +156,7 @@
&state_change_queue_,
std::move(backoff),
task_runner_}};
- command_instance_->AddProxy(std::move(proxy));
+ command_instance_->AddProxy(proxy.release());
}
StateChangeQueueInterface::UpdateID current_state_update_id_{0};
diff --git a/libweave/src/commands/command_instance.cc b/libweave/src/commands/command_instance.cc
index f836a64..04401d1 100644
--- a/libweave/src/commands/command_instance.cc
+++ b/libweave/src/commands/command_instance.cc
@@ -38,7 +38,11 @@
CHECK(command_definition_);
}
-CommandInstance::~CommandInstance() = default;
+CommandInstance::~CommandInstance() {
+ for (auto& proxy : proxies_) {
+ proxy->OnCommandDestroyed();
+ }
+}
const std::string& CommandInstance::GetCategory() const {
return command_definition_->GetCategory();
@@ -169,8 +173,8 @@
return json;
}
-void CommandInstance::AddProxy(std::unique_ptr<CommandProxyInterface> proxy) {
- proxies_.push_back(std::move(proxy));
+void CommandInstance::AddProxy(CommandProxyInterface* proxy) {
+ proxies_.push_back(proxy);
}
bool CommandInstance::SetResults(const native_types::Object& results) {
diff --git a/libweave/src/commands/command_instance.h b/libweave/src/commands/command_instance.h
index 5142d98..ba11985 100644
--- a/libweave/src/commands/command_instance.h
+++ b/libweave/src/commands/command_instance.h
@@ -83,7 +83,7 @@
void SetID(const std::string& id) { id_ = id; }
// Adds a proxy for this command.
// The proxy object is not owned by this class.
- void AddProxy(std::unique_ptr<CommandProxyInterface> proxy);
+ void AddProxy(CommandProxyInterface* proxy);
// Sets the pointer to queue this command is part of.
void SetCommandQueue(CommandQueue* queue) { queue_ = queue; }
@@ -142,7 +142,7 @@
// Current command status.
std::string status_ = kStatusQueued;
// Command proxies for the command.
- std::vector<std::unique_ptr<CommandProxyInterface>> proxies_;
+ std::vector<CommandProxyInterface*> proxies_;
// Pointer to the command queue this command instance is added to.
// The queue owns the command instance, so it outlives this object.
CommandQueue* queue_ = nullptr;
diff --git a/libweave/src/commands/command_proxy_interface.h b/libweave/src/commands/command_proxy_interface.h
index 9f4df61..bf03f34 100644
--- a/libweave/src/commands/command_proxy_interface.h
+++ b/libweave/src/commands/command_proxy_interface.h
@@ -21,6 +21,7 @@
virtual void OnResultsChanged() = 0;
virtual void OnStatusChanged() = 0;
virtual void OnProgressChanged() = 0;
+ virtual void OnCommandDestroyed() = 0;
};
} // namespace weave
diff --git a/libweave/src/commands/dbus_command_dispatcher.cc b/libweave/src/commands/dbus_command_dispatcher.cc
index 65684d3..6800660 100644
--- a/libweave/src/commands/dbus_command_dispatcher.cc
+++ b/libweave/src/commands/dbus_command_dispatcher.cc
@@ -27,7 +27,7 @@
object_manager_.get(), object_manager_->GetBus(), command_instance,
buffet::kCommandServicePathPrefix + std::to_string(++next_id_))};
proxy->RegisterAsync(AsyncEventSequencer::GetDefaultCompletionAction());
- command_instance->AddProxy(std::move(proxy));
+ command_instance->AddProxy(proxy.release());
}
} // namespace weave
diff --git a/libweave/src/commands/dbus_command_dispatcher_unittest.cc b/libweave/src/commands/dbus_command_dispatcher_unittest.cc
index 8b5412b..e97aa53 100644
--- a/libweave/src/commands/dbus_command_dispatcher_unittest.cc
+++ b/libweave/src/commands/dbus_command_dispatcher_unittest.cc
@@ -116,7 +116,7 @@
DBusCommandProxy* FindProxy(CommandInstance* command_instance) {
CHECK_EQ(command_instance->proxies_.size(), 1U);
- return static_cast<DBusCommandProxy*>(command_instance->proxies_[0].get());
+ return static_cast<DBusCommandProxy*>(command_instance->proxies_[0]);
}
void FinishCommand(DBusCommandProxy* proxy) { proxy->Done(); }
diff --git a/libweave/src/commands/dbus_command_proxy.cc b/libweave/src/commands/dbus_command_proxy.cc
index d2fea64..743bc73 100644
--- a/libweave/src/commands/dbus_command_proxy.cc
+++ b/libweave/src/commands/dbus_command_proxy.cc
@@ -62,6 +62,10 @@
ObjectToDBusVariant(command_instance_->GetProgress()));
}
+void DBusCommandProxy::OnCommandDestroyed() {
+ delete this;
+}
+
bool DBusCommandProxy::SetProgress(
chromeos::ErrorPtr* error,
const chromeos::VariantDictionary& progress) {
diff --git a/libweave/src/commands/dbus_command_proxy.h b/libweave/src/commands/dbus_command_proxy.h
index 5ea5ad4..6780766 100644
--- a/libweave/src/commands/dbus_command_proxy.h
+++ b/libweave/src/commands/dbus_command_proxy.h
@@ -41,6 +41,7 @@
void OnResultsChanged() override;
void OnStatusChanged() override;
void OnProgressChanged() override;
+ void OnCommandDestroyed() override;
private:
// Handles calls to org.chromium.Buffet.Command.SetProgress(progress).
diff --git a/libweave/src/commands/dbus_command_proxy_unittest.cc b/libweave/src/commands/dbus_command_proxy_unittest.cc
index e1685ea..7ae20ce 100644
--- a/libweave/src/commands/dbus_command_proxy_unittest.cc
+++ b/libweave/src/commands/dbus_command_proxy_unittest.cc
@@ -111,7 +111,7 @@
std::unique_ptr<CommandProxyInterface> command_proxy(
new DBusCommandProxy(nullptr, bus_, command_instance_.get(), cmd_path));
- command_instance_->AddProxy(std::move(command_proxy));
+ command_instance_->AddProxy(command_proxy.release());
GetCommandProxy()->RegisterAsync(
AsyncEventSequencer::GetDefaultCompletionAction());
}
@@ -125,7 +125,7 @@
DBusCommandProxy* GetCommandProxy() const {
CHECK_EQ(command_instance_->proxies_.size(), 1U);
- return static_cast<DBusCommandProxy*>(command_instance_->proxies_[0].get());
+ return static_cast<DBusCommandProxy*>(command_instance_->proxies_[0]);
}
org::chromium::Buffet::CommandAdaptor* GetCommandAdaptor() const {
diff --git a/libweave/src/device_registration_info.cc b/libweave/src/device_registration_info.cc
index 1b5a07d..340fb40 100644
--- a/libweave/src/device_registration_info.cc
+++ b/libweave/src/device_registration_info.cc
@@ -944,7 +944,7 @@
state_manager_->GetStateChangeQueue(),
std::move(backoff_entry),
task_runner_}};
- command_instance->AddProxy(std::move(cloud_proxy));
+ command_instance->AddProxy(cloud_proxy.release());
command_manager_->AddCommand(std::move(command_instance));
}
}
