Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(422)

Unified Diff: remoting/host/setup/daemon_controller_delegate_linux.cc

Issue 1272833002: Pass error messages from native messaging to web-app. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Fixed unit tests. Created 5 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: remoting/host/setup/daemon_controller_delegate_linux.cc
diff --git a/remoting/host/setup/daemon_controller_delegate_linux.cc b/remoting/host/setup/daemon_controller_delegate_linux.cc
index 4928b8614f43d630864af29a2cd1f64b1df656c6..c249adc27fd926a7e011df370fa688e729af0008 100644
--- a/remoting/host/setup/daemon_controller_delegate_linux.cc
+++ b/remoting/host/setup/daemon_controller_delegate_linux.cc
@@ -65,20 +65,25 @@ bool GetScriptPath(base::FilePath* result) {
bool RunHostScriptWithTimeout(
const std::vector<std::string>& args,
base::TimeDelta timeout,
- int* exit_code) {
- DCHECK(exit_code);
+ const DaemonController::ErrorCallback& on_error) {
// As long as we're relying on running an external binary from the
// PATH, don't do it as root.
if (getuid() == 0) {
- LOG(ERROR) << "Refusing to run script as root.";
+ std::string error_message = "Refusing to run script as root";
+ LOG(ERROR) << error_message;
+ on_error.Run(error_message, FROM_HERE);
return false;
}
+
base::FilePath script_path;
if (!GetScriptPath(&script_path)) {
- LOG(ERROR) << "GetScriptPath() failed.";
+ std::string error_message = "GetScriptPath() failed";
+ LOG(ERROR) << error_message;
+ on_error.Run(error_message, FROM_HERE);
return false;
}
+
base::CommandLine command_line(script_path);
for (unsigned int i = 0; i < args.size(); ++i) {
command_line.AppendArg(args[i]);
@@ -98,24 +103,40 @@ bool RunHostScriptWithTimeout(
base::Process process = base::LaunchProcess(command_line, options);
if (!process.IsValid()) {
- LOG(ERROR) << "Failed to run command: "
- << command_line.GetCommandLineString();
+ std::string error_message =
+ "Failed to run command: " + command_line.GetCommandLineString();
+ LOG(ERROR) << error_message;
+ on_error.Run(error_message, FROM_HERE);
return false;
}
- if (!process.WaitForExitWithTimeout(timeout, exit_code)) {
+ int exit_code;
+ if (!process.WaitForExitWithTimeout(timeout, &exit_code)) {
process.Terminate(0, false);
- LOG(ERROR) << "Timeout exceeded for command: "
- << command_line.GetCommandLineString();
+ std::string error_message =
+ "Timeout exceeded for command: " + command_line.GetCommandLineString();
+ LOG(ERROR) << error_message;
+ on_error.Run(error_message, FROM_HERE);
return false;
}
+ if (exit_code != 0) {
+ std::ostringstream error_message;
+ error_message << "Non-zero exit code " << exit_code << " from command: "
+ << command_line.GetCommandLineString();
+ LOG(ERROR) << error_message;
+ on_error.Run(error_message.str(), FROM_HERE);
+ }
+
return true;
}
-bool RunHostScript(const std::vector<std::string>& args, int* exit_code) {
+bool RunHostScript(const std::vector<std::string>& args,
+ const DaemonController::ErrorCallback& on_error) {
return RunHostScriptWithTimeout(
- args, base::TimeDelta::FromMilliseconds(kDaemonTimeoutMs), exit_code);
+ args,
+ base::TimeDelta::FromMilliseconds(kDaemonTimeoutMs),
+ on_error);
}
} // namespace
@@ -129,7 +150,7 @@ DaemonControllerDelegateLinux::~DaemonControllerDelegateLinux() {
DaemonController::State DaemonControllerDelegateLinux::GetState() {
base::FilePath script_path;
if (!GetScriptPath(&script_path)) {
- LOG(ERROR) << "GetScriptPath() failed.";
+ LOG(ERROR) << "GetScriptPath() failed";
return DaemonController::STATE_UNKNOWN;
}
base::CommandLine command_line(script_path);
@@ -182,17 +203,14 @@ scoped_ptr<base::DictionaryValue> DaemonControllerDelegateLinux::GetConfig() {
void DaemonControllerDelegateLinux::SetConfigAndStart(
scoped_ptr<base::DictionaryValue> config,
bool consent,
- const DaemonController::CompletionCallback& done) {
+ const base::Closure& on_done,
+ const DaemonController::ErrorCallback& on_error) {
// Add the user to chrome-remote-desktop group first.
std::vector<std::string> args;
args.push_back("--add-user");
- int exit_code;
if (!RunHostScriptWithTimeout(
args, base::TimeDelta::FromSeconds(kSudoTimeoutSeconds),
- &exit_code) ||
- exit_code != 0) {
- LOG(ERROR) << "Failed to add user to chrome-remote-desktop group.";
- done.Run(DaemonController::RESULT_FAILED);
+ on_error)) {
return;
}
@@ -200,61 +218,67 @@ void DaemonControllerDelegateLinux::SetConfigAndStart(
base::FilePath config_dir = GetConfigPath().DirName();
if (!base::DirectoryExists(config_dir) &&
!base::CreateDirectory(config_dir)) {
- LOG(ERROR) << "Failed to create config directory " << config_dir.value();
- done.Run(DaemonController::RESULT_FAILED);
+ std::string error_message =
+ "Failed to create config directory " + config_dir.value();
+ LOG(ERROR) << error_message;
+ on_error.Run(error_message, FROM_HERE);
return;
}
// Write config.
if (!HostConfigToJsonFile(*config, GetConfigPath())) {
- LOG(ERROR) << "Failed to update config file.";
- done.Run(DaemonController::RESULT_FAILED);
+ std::string error_message = "Failed to update config file";
+ LOG(ERROR) << error_message;
+ on_error.Run(error_message, FROM_HERE);
return;
}
// Finally start the host.
args.clear();
args.push_back("--start");
- DaemonController::AsyncResult result = DaemonController::RESULT_FAILED;
- if (RunHostScript(args, &exit_code) && (exit_code == 0))
- result = DaemonController::RESULT_OK;
-
- done.Run(result);
+ if (RunHostScript(args, on_error)) {
+ on_done.Run();
+ }
}
void DaemonControllerDelegateLinux::UpdateConfig(
scoped_ptr<base::DictionaryValue> config,
- const DaemonController::CompletionCallback& done) {
+ const base::Closure& on_done,
+ const DaemonController::ErrorCallback& on_error) {
scoped_ptr<base::DictionaryValue> new_config(
HostConfigFromJsonFile(GetConfigPath()));
- if (new_config)
- new_config->MergeDictionary(config.get());
- if (!new_config || !HostConfigToJsonFile(*new_config, GetConfigPath())) {
- LOG(ERROR) << "Failed to update config file.";
- done.Run(DaemonController::RESULT_FAILED);
+ if (!new_config) {
+ std::string error_message =
+ "Reading config from " + GetConfigPath().value() + " failed";
+ LOG(ERROR) << error_message;
+ on_error.Run(error_message, FROM_HERE);
+ return;
+ }
+
+ new_config->MergeDictionary(config.get());
+ if (!HostConfigToJsonFile(*new_config, GetConfigPath())) {
+ std::string error_message =
+ "Writing config to " + GetConfigPath().value() + " failed";
+ LOG(ERROR) << error_message;
+ on_error.Run(error_message, FROM_HERE);
return;
}
std::vector<std::string> args;
args.push_back("--reload");
- int exit_code = 0;
- DaemonController::AsyncResult result = DaemonController::RESULT_FAILED;
- if (RunHostScript(args, &exit_code) && (exit_code == 0))
- result = DaemonController::RESULT_OK;
-
- done.Run(result);
+ if (RunHostScript(args, on_error)) {
+ on_done.Run();
+ }
}
void DaemonControllerDelegateLinux::Stop(
- const DaemonController::CompletionCallback& done) {
+ const base::Closure& on_done,
+ const DaemonController::ErrorCallback& on_error) {
std::vector<std::string> args;
args.push_back("--stop");
- int exit_code = 0;
- DaemonController::AsyncResult result = DaemonController::RESULT_FAILED;
- if (RunHostScript(args, &exit_code) && (exit_code == 0))
- result = DaemonController::RESULT_OK;
-
- done.Run(result);
+ if (RunHostScript(args, on_error)) {
+ on_done.Run();
+ }
}
DaemonController::UsageStatsConsent

Powered by Google App Engine
This is Rietveld 408576698