Properly handle handler registering and unregistering
In addition to now supporting unregistering, we now correctly support
multiple handlers under DBus, and correctly send initial events under
binder.
TEST=Verified update_engine_client still works as expected
Bug: None
Change-Id: I60955d4d9ca61dfe7857b9fc78f77fa4292ed218
diff --git a/client_library/client_binder.cc b/client_library/client_binder.cc
index 969c5e9..6993be1 100644
--- a/client_library/client_binder.cc
+++ b/client_library/client_binder.cc
@@ -154,9 +154,38 @@
}
handlers_.push_back(handler);
+
+ int64_t last_checked_time;
+ double progress;
+ UpdateStatus update_status;
+ string new_version;
+ int64_t new_size;
+
+ if (!GetStatus(&last_checked_time, &progress, &update_status,
+ &new_version, &new_size)) {
+ handler->IPCError("Could not get status from binder service");
+ }
+
+ handler->HandleStatusUpdate(last_checked_time, progress, update_status,
+ new_version, new_size);
+
return true;
}
+bool BinderUpdateEngineClient::UnregisterStatusUpdateHandler(
+ StatusUpdateHandler* handler) {
+ auto it = handlers_.begin();
+
+ for (; *it != handler && it != handlers_.end(); it++);
+
+ if (it != handlers_.end()) {
+ handlers_.erase(it);
+ return true;
+ }
+
+ return false;
+}
+
bool BinderUpdateEngineClient::SetTargetChannel(const string& in_target_channel,
bool allow_powerwash) {
return service_->SetChannel(String16{in_target_channel.c_str()},
diff --git a/client_library/client_binder.h b/client_library/client_binder.h
index 744b238..562cee4 100644
--- a/client_library/client_binder.h
+++ b/client_library/client_binder.h
@@ -78,6 +78,7 @@
bool GetChannel(std::string* out_channel) const override;
bool RegisterStatusUpdateHandler(StatusUpdateHandler* handler) override;
+ bool UnregisterStatusUpdateHandler(StatusUpdateHandler* handler) override;
private:
class StatusUpdateCallback :
diff --git a/client_library/client_dbus.cc b/client_library/client_dbus.cc
index d9b4a05..2579506 100644
--- a/client_library/client_dbus.cc
+++ b/client_library/client_dbus.cc
@@ -116,37 +116,44 @@
return proxy_->ResetStatus(nullptr);
}
-void DBusUpdateEngineClient::StatusUpdateHandlerRegistered(
- StatusUpdateHandler* handler,
+void DBusUpdateEngineClient::DBusStatusHandlerRegistered(
const string& interface,
const string& signal_name,
bool success) const {
if (!success) {
- handler->IPCError("Could not connect to" + signal_name);
- return;
+ for (auto handler : handlers_) {
+ handler->IPCError("Could not connect to" + signal_name +
+ " on " + interface);
+ }
+ } else {
+ StatusUpdateHandlersRegistered(nullptr);
}
+}
+void DBusUpdateEngineClient::StatusUpdateHandlersRegistered(
+ StatusUpdateHandler* handler) const {
int64_t last_checked_time;
double progress;
UpdateStatus update_status;
string new_version;
int64_t new_size;
- if (GetStatus(&last_checked_time,
- &progress,
- &update_status,
- &new_version,
- &new_size)) {
- handler->HandleStatusUpdate(
- last_checked_time, progress, update_status, new_version, new_size);
+ if (!GetStatus(&last_checked_time,
+ &progress,
+ &update_status,
+ &new_version,
+ &new_size)) {
+ handler->IPCError("Could not query current status");
return;
}
- handler->IPCError("Could not query current status");
+ for (auto h : handler ? {handler} : handlers_) {
+ h->HandleStatusUpdate(
+ last_checked_time, progress, update_status, new_version, new_size);
+ }
}
-void DBusUpdateEngineClient::RunStatusUpdateHandler(
- StatusUpdateHandler* h,
+void DBusUpdateEngineClient::RunStatusUpdateHandlers(
int64_t last_checked_time,
double progress,
const string& current_operation,
@@ -155,8 +162,24 @@
UpdateStatus status;
StringToUpdateStatus(current_operation, &status);
- h->HandleStatusUpdate(
- last_checked_time, progress, status, new_version, new_size);
+ for (auto handler : handlers_) {
+ handler->HandleStatusUpdate(
+ last_checked_time, progress, status, new_version, new_size);
+ }
+}
+
+bool DBusUpdateEngineClient::UnregisterStatusUpdateHandler(
+ StatusUpdateHandler* handler) {
+ auto it = handlers_.begin();
+
+ for (; *it != handler && it != handlers_.end(); it++);
+
+ if (it != handlers_.end()) {
+ handlers_.erase(it);
+ return true;
+ }
+
+ return false;
}
bool DBusUpdateEngineClient::RegisterStatusUpdateHandler(
@@ -166,11 +189,20 @@
return false;
}
+ handlers_.push_back(handler);
+
+ if (dbus_handler_registered_) {
+ StatusUpdateHandlersRegistered(handler);
+ return true;
+ }
+
proxy_->RegisterStatusUpdateSignalHandler(
- base::Bind(&DBusUpdateEngineClient::RunStatusUpdateHandler,
- base::Unretained(this), base::Unretained(handler)),
- base::Bind(&DBusUpdateEngineClient::StatusUpdateHandlerRegistered,
- base::Unretained(this), base::Unretained(handler)));
+ base::Bind(&DBusUpdateEngineClient::RunStatusUpdateHandlers,
+ base::Unretained(this)),
+ base::Bind(&DBusUpdateEngineClient::StatusUpdateHandlersRegistered,
+ base::Unretained(this), base::Unretained(nullptr)));
+
+ dbus_handler_registered_ = true;
return true;
}
diff --git a/client_library/client_dbus.h b/client_library/client_dbus.h
index e6bbe36..9e125ba 100644
--- a/client_library/client_dbus.h
+++ b/client_library/client_dbus.h
@@ -20,6 +20,7 @@
#include <cstdint>
#include <memory>
#include <string>
+#include <vector>
#include <base/macros.h>
@@ -70,21 +71,30 @@
bool GetChannel(std::string* out_channel) const override;
bool RegisterStatusUpdateHandler(StatusUpdateHandler* handler) override;
+ bool UnregisterStatusUpdateHandler(StatusUpdateHandler* handler) override;
private:
std::unique_ptr<org::chromium::UpdateEngineInterfaceProxy> proxy_;
+ std::vector<update_engine::StatusUpdateHandler*> handlers_;
- void StatusUpdateHandlerRegistered(StatusUpdateHandler* handler,
- const std::string& interface,
- const std::string& signal_name,
- bool success) const;
+ void DBusStatusHandlersRegistered(StatusUpdateHandler* handler,
+ const std::string& interface,
+ const std::string& signal_name,
+ bool success) const;
- void RunStatusUpdateHandler(StatusUpdateHandler* handler,
- int64_t last_checked_time,
- double progress,
- const std::string& current_operation,
- const std::string& new_version,
- int64_t new_size);
+ // Send an initial event to new StatusUpdateHandlers. If the handler argument
+ // is not nullptr, only that handler receives the event. Otherwise all
+ // registered handlers receive the event.
+ void StatusUpdateHandlersRegistered(StatusUpdateHandler* handler) const;
+
+ void RunStatusUpdateHandlers(int64_t last_checked_time,
+ double progress,
+ const std::string& current_operation,
+ const std::string& new_version,
+ int64_t new_size);
+
+ std::vector<update_engine::StatusUpdateHandler*> handlers_;
+ bool dbus_handler_registered_ = false;
DISALLOW_COPY_AND_ASSIGN(DBusUpdateEngineClient);
}; // class DBusUpdateEngineClient
diff --git a/client_library/include/update_engine/client.h b/client_library/include/update_engine/client.h
index f2af453..dc0fb8c 100644
--- a/client_library/include/update_engine/client.h
+++ b/client_library/include/update_engine/client.h
@@ -102,12 +102,16 @@
virtual bool GetChannel(std::string* out_channel) const = 0;
// Handle status updates. The handler must exist until the client is
- // destroyed. Its IPCError method will be called if the handler could
- // not be registered. Otherwise its HandleStatusUpdate method will be called
- // every time update_engine's status changes. Will always report the status
- // on registration to prevent race conditions.
+ // destroyed or UnregisterStatusUpdateHandler is called for it. Its IPCError
+ // method will be called if the handler could not be registered. Otherwise
+ // its HandleStatusUpdate method will be called every time update_engine's
+ // status changes. Will always report the status on registration to prevent
+ // race conditions.
virtual bool RegisterStatusUpdateHandler(StatusUpdateHandler* handler) = 0;
+ // Unregister a status update handler
+ virtual bool UnregisterStatusUpdateHandler(StatusUpdateHandler* handler) = 0;
+
protected:
// Use CreateInstance().
UpdateEngineClient() = default;