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