diff options
author | mnissler <mnissler@chromium.org> | 2015-08-05 02:20:09 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-08-05 09:22:07 +0000 |
commit | fb4a4c60596d52a2f3ab26ab94399d3ebd5a64c1 (patch) | |
tree | f398017f1630d0d6923b3b1a5e136527051a503e /chromeos | |
parent | 7ae8d9c314c283e9d2d8b72e106d6625a009e7bc (diff) | |
download | chromium_src-fb4a4c60596d52a2f3ab26ab94399d3ebd5a64c1.zip chromium_src-fb4a4c60596d52a2f3ab26ab94399d3ebd5a64c1.tar.gz chromium_src-fb4a4c60596d52a2f3ab26ab94399d3ebd5a64c1.tar.bz2 |
SessionManagerClient::RestartJob: Use argv instead of string.
This changes the interface to session_manager to pass the command line
as an array instead of a string. The previous code path would
(incorrectly) quote arguments and combine them into a string, and then
again parse and unquote them on the session_manager side, which is
unnecessary and error-prone. This changes the code to pass an array of
strings in the DBus call, which simplifies things considerably.
BUG=chromium:433172
TEST=Guest session starts successfully.
Review URL: https://codereview.chromium.org/1268973004
Cr-Commit-Position: refs/heads/master@{#341873}
Diffstat (limited to 'chromeos')
-rw-r--r-- | chromeos/dbus/fake_session_manager_client.cc | 5 | ||||
-rw-r--r-- | chromeos/dbus/fake_session_manager_client.h | 2 | ||||
-rw-r--r-- | chromeos/dbus/mock_session_manager_client.h | 2 | ||||
-rw-r--r-- | chromeos/dbus/session_manager_client.cc | 33 | ||||
-rw-r--r-- | chromeos/dbus/session_manager_client.h | 4 |
5 files changed, 22 insertions, 24 deletions
diff --git a/chromeos/dbus/fake_session_manager_client.cc b/chromeos/dbus/fake_session_manager_client.cc index c1745cf..89261ea 100644 --- a/chromeos/dbus/fake_session_manager_client.cc +++ b/chromeos/dbus/fake_session_manager_client.cc @@ -47,9 +47,8 @@ bool FakeSessionManagerClient::IsScreenLocked() const { void FakeSessionManagerClient::EmitLoginPromptVisible() { } -void FakeSessionManagerClient::RestartJob(int pid, - const std::string& command_line) { -} +void FakeSessionManagerClient::RestartJob( + const std::vector<std::string>& argv) {} void FakeSessionManagerClient::StartSession(const std::string& user_email) { DCHECK_EQ(0UL, user_sessions_.count(user_email)); diff --git a/chromeos/dbus/fake_session_manager_client.h b/chromeos/dbus/fake_session_manager_client.h index ed4c5c1..e2ff7cc 100644 --- a/chromeos/dbus/fake_session_manager_client.h +++ b/chromeos/dbus/fake_session_manager_client.h @@ -31,7 +31,7 @@ class FakeSessionManagerClient : public SessionManagerClient { bool HasObserver(const Observer* observer) const override; bool IsScreenLocked() const override; void EmitLoginPromptVisible() override; - void RestartJob(int pid, const std::string& command_line) override; + void RestartJob(const std::vector<std::string>& argv) override; void StartSession(const std::string& user_email) override; void StopSession() override; void NotifySupervisedUserCreationStarted() override; diff --git a/chromeos/dbus/mock_session_manager_client.h b/chromeos/dbus/mock_session_manager_client.h index 3a88e52..17f0caf 100644 --- a/chromeos/dbus/mock_session_manager_client.h +++ b/chromeos/dbus/mock_session_manager_client.h @@ -24,7 +24,7 @@ class MockSessionManagerClient : public SessionManagerClient { MOCK_CONST_METHOD1(HasObserver, bool(const Observer*)); MOCK_CONST_METHOD0(IsScreenLocked, bool(void)); MOCK_METHOD0(EmitLoginPromptVisible, void(void)); - MOCK_METHOD2(RestartJob, void(int, const std::string&)); + MOCK_METHOD1(RestartJob, void(const std::vector<std::string>&)); MOCK_METHOD1(StartSession, void(const std::string&)); MOCK_METHOD0(StopSession, void(void)); MOCK_METHOD0(NotifySupervisedUserCreationStarted, void(void)); diff --git a/chromeos/dbus/session_manager_client.cc b/chromeos/dbus/session_manager_client.cc index 438ec05..36a4e28 100644 --- a/chromeos/dbus/session_manager_client.cc +++ b/chromeos/dbus/session_manager_client.cc @@ -115,15 +115,15 @@ class SessionManagerClientImpl : public SessionManagerClient { FOR_EACH_OBSERVER(Observer, observers_, EmitLoginPromptVisibleCalled()); } - void RestartJob(int pid, const std::string& command_line) override { + void RestartJob(const std::vector<std::string>& argv) override { dbus::ScopedFileDescriptor local_auth_fd(new dbus::FileDescriptor); dbus::ScopedFileDescriptor remote_auth_fd(new dbus::FileDescriptor); - // The session_manager provides a new method to replace RestartJob, called - // RestartJobWithAuth, that is able to be used correctly within a PID - // namespace. To use it, the caller must create a unix domain socket pair - // and pass one end over dbus while holding the local end open for the - // duration of the call. + // session_manager's RestartJob call requires the caller to open a socket + // pair and pass one end over dbus while holding the local end open for the + // duration of the call. session_manager uses this to determine whether the + // PID the restart request originates from belongs to the browser itself. + // // Here, we call CreateValidCredConduit() to create the socket pair, // and then pass both ends along to CallRestartJobWithValidFd(), which // takes care of them from there. @@ -146,7 +146,7 @@ class SessionManagerClientImpl : public SessionManagerClient { FROM_HERE, create_credentials_conduit_closure, base::Bind(&SessionManagerClientImpl::CallRestartJobWithValidFd, weak_ptr_factory_.GetWeakPtr(), base::Passed(&local_auth_fd), - base::Passed(&remote_auth_fd), command_line), + base::Passed(&remote_auth_fd), argv), false); } @@ -421,19 +421,18 @@ class SessionManagerClientImpl : public SessionManagerClient { callback)); } - // Calls RestartJobWithAuth to tell the session manager to restart the - // browser using the contents of command_line, authorizing the call - // using credentials acquired via remote_auth_fd. - // Ownership of local_auth_fd is held for the duration of the dbus call. + // Calls RestartJob to tell the session manager to restart the browser using + // the contents of |argv| as the command line, authorizing the call using + // credentials acquired via |remote_auth_fd|. Ownership of |local_auth_fd| is + // held for the duration of the dbus call. void CallRestartJobWithValidFd(dbus::ScopedFileDescriptor local_auth_fd, dbus::ScopedFileDescriptor remote_auth_fd, - const std::string& command_line) { - dbus::MethodCall method_call( - login_manager::kSessionManagerInterface, - login_manager::kSessionManagerRestartJobWithAuth); + const std::vector<std::string>& argv) { + dbus::MethodCall method_call(login_manager::kSessionManagerInterface, + login_manager::kSessionManagerRestartJob); dbus::MessageWriter writer(&method_call); writer.AppendFileDescriptor(*remote_auth_fd); - writer.AppendString(command_line); + writer.AppendArrayOfStrings(argv); // Ownership of local_auth_fd is passed to the callback that is to be // called on completion of this method call. This keeps the browser end @@ -670,7 +669,7 @@ class SessionManagerClientStubImpl : public SessionManagerClient { } bool IsScreenLocked() const override { return screen_is_locked_; } void EmitLoginPromptVisible() override {} - void RestartJob(int pid, const std::string& command_line) override {} + void RestartJob(const std::vector<std::string>& argv) override {} void StartSession(const std::string& user_email) override {} void StopSession() override {} void NotifySupervisedUserCreationStarted() override {} diff --git a/chromeos/dbus/session_manager_client.h b/chromeos/dbus/session_manager_client.h index 254e762..ba21997 100644 --- a/chromeos/dbus/session_manager_client.h +++ b/chromeos/dbus/session_manager_client.h @@ -73,8 +73,8 @@ class CHROMEOS_EXPORT SessionManagerClient : public DBusClient { // Kicks off an attempt to emit the "login-prompt-visible" upstart signal. virtual void EmitLoginPromptVisible() = 0; - // Restarts a job referenced by |pid| with the provided command line. - virtual void RestartJob(int pid, const std::string& command_line) = 0; + // Restarts the browser job, passing |argv| as the updated command line. + virtual void RestartJob(const std::vector<std::string>& argv) = 0; // Starts the session for the user. virtual void StartSession(const std::string& user_email) = 0; |