buffet: Fix buffet_client's use of proxies The previous code would never work, because the ObjectManager discovers the Buffet Manager asynchronously to its construction. BUG=brillo:361 TEST=buffet_client commands work again. Change-Id: I2f3f5c4c638e52e1d766a7d7ef78441ed23b7dbe Reviewed-on: https://chromium-review.googlesource.com/255575 Reviewed-by: Christopher Wiley <wiley@chromium.org> Commit-Queue: Christopher Wiley <wiley@chromium.org> Tested-by: Christopher Wiley <wiley@chromium.org>
diff --git a/buffet/buffet_client.cc b/buffet/buffet_client.cc index e630dc8..ca76a89 100644 --- a/buffet/buffet_client.cc +++ b/buffet/buffet_client.cc
@@ -6,10 +6,12 @@ #include <string> #include <sysexits.h> +#include <base/cancelable_callback.h> #include <base/command_line.h> #include <base/json/json_reader.h> #include <base/logging.h> #include <base/memory/ref_counted.h> +#include <base/memory/weak_ptr.h> #include <base/strings/stringprintf.h> #include <base/values.h> #include <chromeos/any.h> @@ -29,6 +31,7 @@ using chromeos::Error; using chromeos::ErrorPtr; +using org::chromium::Buffet::ManagerProxy; namespace { @@ -146,73 +149,11 @@ return return_code; object_manager_.reset(new org::chromium::Buffet::ObjectManagerProxy{bus_}); - manager_proxy_ = object_manager_->GetManagerProxy(); - if (!manager_proxy_) { - fprintf(stderr, "Buffet daemon was offline."); - return EX_UNAVAILABLE; - } - - auto args = CommandLine::ForCurrentProcess()->GetArgs(); - - // Pop the command off of the args list. - std::string command = args.front(); - args.erase(args.begin()); - - if (command.compare("TestMethod") == 0) { - if (args.empty() || CheckArgs(command, args, 1)) { - std::string message; - if (!args.empty()) - message = args.back(); - PostTask(&Daemon::CallTestMethod, message); - } - } else if (command.compare("StartDevice") == 0 || - command.compare("sd") == 0) { - if (CheckArgs(command, args, 0)) - PostTask(&Daemon::CallStartDevice); - } else if (command.compare("CheckDeviceRegistered") == 0 || - command.compare("cr") == 0) { - if (CheckArgs(command, args, 0)) - PostTask(&Daemon::CallCheckDeviceRegistered); - } else if (command.compare("GetDeviceInfo") == 0 || - command.compare("di") == 0) { - if (CheckArgs(command, args, 0)) - PostTask(&Daemon::CallGetDeviceInfo); - } else if (command.compare("RegisterDevice") == 0 || - command.compare("rd") == 0) { - if (args.empty() || CheckArgs(command, args, 1)) { - std::string dict; - if (!args.empty()) - dict = args.back(); - PostTask(&Daemon::CallRegisterDevice, dict); - } - } else if (command.compare("UpdateState") == 0 || - command.compare("us") == 0) { - if (CheckArgs(command, args, 2)) - PostTask(&Daemon::CallUpdateState, args.front(), args.back()); - } else if (command.compare("GetState") == 0 || - command.compare("gs") == 0) { - if (CheckArgs(command, args, 0)) - PostTask(&Daemon::CallGetState); - } else if (command.compare("AddCommand") == 0 || - command.compare("ac") == 0) { - if (CheckArgs(command, args, 1)) - PostTask(&Daemon::CallAddCommand, args.back()); - } else if (command.compare("PendingCommands") == 0 || - command.compare("pc") == 0) { - if (CheckArgs(command, args, 0)) - // CallGetPendingCommands relies on ObjectManager but it is being - // initialized asynchronously without a way to get a callback when - // it is ready to be used. So, just wait a bit before calling its - // methods. - PostDelayedTask(&Daemon::CallGetPendingCommands, - base::TimeDelta::FromMilliseconds(100)); - } else { - fprintf(stderr, "Unknown command: '%s'\n", command.c_str()); + return_code = ScheduleActions(); + if (return_code == EX_USAGE) { usage(); - Quit(); - return EX_USAGE; } - return EX_OK; + return return_code; } void OnShutdown(int* return_code) override { @@ -221,11 +162,108 @@ } private: + int ScheduleActions() { + auto args = CommandLine::ForCurrentProcess()->GetArgs(); + + // Pop the command off of the args list. + std::string command = args.front(); + args.erase(args.begin()); + base::Callback<void(ManagerProxy*)> job; + if (command.compare("TestMethod") == 0) { + if (!args.empty() && !CheckArgs(command, args, 1)) + return EX_USAGE; + std::string message; + if (!args.empty()) + message = args.back(); + job = base::Bind(&Daemon::CallTestMethod, weak_factory_.GetWeakPtr(), + message); + } else if (command.compare("StartDevice") == 0 || + command.compare("sd") == 0) { + if (!CheckArgs(command, args, 0)) + return EX_USAGE; + job = base::Bind(&Daemon::CallStartDevice, weak_factory_.GetWeakPtr()); + } else if (command.compare("CheckDeviceRegistered") == 0 || + command.compare("cr") == 0) { + if (!CheckArgs(command, args, 0)) + return EX_USAGE; + job = base::Bind(&Daemon::CallCheckDeviceRegistered, + weak_factory_.GetWeakPtr()); + } else if (command.compare("GetDeviceInfo") == 0 || + command.compare("di") == 0) { + if (!CheckArgs(command, args, 0)) + return EX_USAGE; + job = base::Bind(&Daemon::CallGetDeviceInfo, + weak_factory_.GetWeakPtr()); + } else if (command.compare("RegisterDevice") == 0 || + command.compare("rd") == 0) { + if (!args.empty() && !CheckArgs(command, args, 1)) + return EX_USAGE; + std::string dict; + if (!args.empty()) + dict = args.back(); + job = base::Bind(&Daemon::CallRegisterDevice, + weak_factory_.GetWeakPtr(), dict); + } else if (command.compare("UpdateState") == 0 || + command.compare("us") == 0) { + if (!CheckArgs(command, args, 2)) + return EX_USAGE; + job = base::Bind(&Daemon::CallUpdateState, weak_factory_.GetWeakPtr(), + args.front(), args.back()); + } else if (command.compare("GetState") == 0 || + command.compare("gs") == 0) { + if (!CheckArgs(command, args, 0)) + return EX_USAGE; + job = base::Bind(&Daemon::CallGetState, weak_factory_.GetWeakPtr()); + } else if (command.compare("AddCommand") == 0 || + command.compare("ac") == 0) { + if (!CheckArgs(command, args, 1)) + return EX_USAGE; + job = base::Bind(&Daemon::CallAddCommand, weak_factory_.GetWeakPtr(), + args.back()); + } else if (command.compare("PendingCommands") == 0 || + command.compare("pc") == 0) { + if (!CheckArgs(command, args, 0)) + return EX_USAGE; + // CallGetPendingCommands relies on ObjectManager but it is being + // initialized asynchronously without a way to get a callback when + // it is ready to be used. So, just wait a bit before calling its + // methods. + base::MessageLoop::current()->PostDelayedTask( + FROM_HERE, + base::Bind(&Daemon::CallGetPendingCommands, + weak_factory_.GetWeakPtr()), + base::TimeDelta::FromMilliseconds(100)); + } else { + fprintf(stderr, "Unknown command: '%s'\n", command.c_str()); + return EX_USAGE; + } + if (!job.is_null()) + object_manager_->SetManagerAddedCallback(job); + timeout_task_.Reset( + base::Bind(&Daemon::OnJobTimeout, weak_factory_.GetWeakPtr())); + base::MessageLoop::current()->PostDelayedTask( + FROM_HERE, + timeout_task_.callback(), + base::TimeDelta::FromSeconds(10)); + + return EX_OK; + } + + void OnJobComplete() { + timeout_task_.Cancel(); + Quit(); + } + + void OnJobTimeout() { + fprintf(stderr, "Timed out before completing request."); + Quit(); + } + void ReportError(Error* error) { fprintf(stderr, "Failed to receive a response: %s\n", error->GetMessage().c_str()); exit_code_ = EX_UNAVAILABLE; - Quit(); + OnJobComplete(); } bool CheckArgs(const std::string& command, @@ -235,69 +273,53 @@ return true; fprintf(stderr, "Invalid number of arguments for command '%s'\n", command.c_str()); - usage(); - exit_code_ = EX_USAGE; - Quit(); return false; } - template<typename... Args> - void PostTask(void (Daemon::*fn)(const Args&...), const Args&... args) { - base::MessageLoop::current()->PostTask( - FROM_HERE, base::Bind(fn, base::Unretained(this), args...)); - } - - template<typename... Args> - void PostDelayedTask(void (Daemon::*fn)(const Args&...), - const base::TimeDelta& delay, - const Args&... args) { - base::MessageLoop::current()->PostDelayedTask( - FROM_HERE, base::Bind(fn, base::Unretained(this), args...), delay); - } - - void CallTestMethod(const std::string& message) { + void CallTestMethod(const std::string& message, ManagerProxy* manager_proxy) { ErrorPtr error; std::string response_message; - if (!manager_proxy_->TestMethod(message, &response_message, &error)) { + if (!manager_proxy->TestMethod(message, &response_message, &error)) { return ReportError(error.get()); } printf("Received a response: %s\n", response_message.c_str()); - Quit(); + OnJobComplete(); } - void CallStartDevice() { + void CallStartDevice(ManagerProxy* manager_proxy) { ErrorPtr error; - if (!manager_proxy_->StartDevice(&error)) { + if (!manager_proxy->StartDevice(&error)) { return ReportError(error.get()); } - Quit(); + OnJobComplete(); } - void CallCheckDeviceRegistered() { + void CallCheckDeviceRegistered(ManagerProxy* manager_proxy) { ErrorPtr error; std::string device_id; - if (!manager_proxy_->CheckDeviceRegistered(&device_id, &error)) { + if (!manager_proxy->CheckDeviceRegistered(&device_id, &error)) { return ReportError(error.get()); } printf("Device ID: %s\n", device_id.empty() ? "<unregistered>" : device_id.c_str()); - Quit(); + OnJobComplete(); } - void CallGetDeviceInfo() { + void CallGetDeviceInfo(ManagerProxy* manager_proxy) { ErrorPtr error; std::string device_info; - if (!manager_proxy_->GetDeviceInfo(&device_info, &error)) { + if (!manager_proxy->GetDeviceInfo(&device_info, &error)) { return ReportError(error.get()); } printf("Device Info: %s\n", device_info.empty() ? "<unregistered>" : device_info.c_str()); - Quit(); + OnJobComplete(); } - void CallRegisterDevice(const std::string& args) { + void CallRegisterDevice(const std::string& args, + ManagerProxy* manager_proxy) { chromeos::VariantDictionary params; if (!args.empty()) { auto key_values = chromeos::data_encoding::WebParamsDecode(args); @@ -308,15 +330,17 @@ ErrorPtr error; std::string device_id; - if (!manager_proxy_->RegisterDevice(params, &device_id, &error)) { + if (!manager_proxy->RegisterDevice(params, &device_id, &error)) { return ReportError(error.get()); } printf("Device registered: %s\n", device_id.c_str()); - Quit(); + OnJobComplete(); } - void CallUpdateState(const std::string& prop, const std::string& value) { + void CallUpdateState(const std::string& prop, + const std::string& value, + ManagerProxy* manager_proxy) { ErrorPtr error; std::string error_message; std::unique_ptr<base::Value> json(base::JSONReader::ReadAndReturnError( @@ -328,28 +352,28 @@ } chromeos::VariantDictionary property_set{{prop, JsonToAny(*json)}}; - if (!manager_proxy_->UpdateState(property_set, &error)) { + if (!manager_proxy->UpdateState(property_set, &error)) { return ReportError(error.get()); } - Quit(); + OnJobComplete(); } - void CallGetState() { + void CallGetState(ManagerProxy* manager_proxy) { std::string json; ErrorPtr error; - if (!manager_proxy_->GetState(&json, &error)) { + if (!manager_proxy->GetState(&json, &error)) { return ReportError(error.get()); } printf("Device State: %s\n", json.c_str()); - Quit(); + OnJobComplete(); } - void CallAddCommand(const std::string& command) { + void CallAddCommand(const std::string& command, ManagerProxy* manager_proxy) { ErrorPtr error; - if (!manager_proxy_->AddCommand(command, &error)) { + if (!manager_proxy->AddCommand(command, &error)) { return ReportError(error.get()); } - Quit(); + OnJobComplete(); } void CallGetPendingCommands() { @@ -361,13 +385,14 @@ cmd->name().c_str(), cmd->id().c_str()); } - Quit(); + OnJobComplete(); } std::unique_ptr<org::chromium::Buffet::ObjectManagerProxy> object_manager_; - org::chromium::Buffet::ManagerProxy* manager_proxy_{nullptr}; int exit_code_{EX_OK}; + base::CancelableCallback<void()> timeout_task_; + base::WeakPtrFactory<Daemon> weak_factory_{this}; DISALLOW_COPY_AND_ASSIGN(Daemon); };