diff options
11 files changed, 58 insertions, 96 deletions
diff --git a/chrome/browser/chromeos/login/chrome_restart_request.cc b/chrome/browser/chromeos/login/chrome_restart_request.cc index 9821fb8..e314914 100644 --- a/chrome/browser/chromeos/login/chrome_restart_request.cc +++ b/chrome/browser/chromeos/login/chrome_restart_request.cc @@ -14,6 +14,7 @@ #include "base/prefs/pref_service.h" #include "base/process/launch.h" #include "base/strings/string_split.h" +#include "base/strings/string_util.h" #include "base/strings/stringprintf.h" #include "base/sys_info.h" #include "base/timer/timer.h" @@ -55,17 +56,14 @@ namespace { // Increase logging level for Guest mode to avoid INFO messages in logs. const char kGuestModeLoggingLevel[] = "1"; -// Format of command line switch. -const char kSwitchFormatString[] = " --%s=\"%s\""; - // Derives the new command line from |base_command_line| by doing the following: // - Forward a given switches list to new command; // - Set start url if given; // - Append/override switches using |new_switches|; -std::string DeriveCommandLine(const GURL& start_url, - const base::CommandLine& base_command_line, - const base::DictionaryValue& new_switches, - base::CommandLine* command_line) { +void DeriveCommandLine(const GURL& start_url, + const base::CommandLine& base_command_line, + const base::DictionaryValue& new_switches, + base::CommandLine* command_line) { DCHECK_NE(&base_command_line, command_line); static const char* const kForwardSwitches[] = { @@ -236,30 +234,12 @@ std::string DeriveCommandLine(const GURL& start_url, CHECK(it.value().GetAsString(&value)); command_line->AppendSwitchASCII(it.key(), value); } - - std::string cmd_line_str = command_line->GetCommandLineString(); - // Special workaround for the arguments that should be quoted. - // Copying switches won't be needed when Guest mode won't need restart - // http://crosbug.com/6924 - if (base_command_line.HasSwitch(::switches::kRegisterPepperPlugins)) { - cmd_line_str += base::StringPrintf( - kSwitchFormatString, - ::switches::kRegisterPepperPlugins, - base_command_line.GetSwitchValueNative( - ::switches::kRegisterPepperPlugins).c_str()); - } - - return cmd_line_str; } // Simulates a session manager restart by launching give command line // and exit current process. -void ReLaunch(const std::string& command_line) { - // This is not a proper way to get |argv| but it's good enough for debugging. - std::vector<std::string> argv = base::SplitString( - command_line, " ", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL); - - base::LaunchProcess(argv, base::LaunchOptions()); +void ReLaunch(const base::CommandLine& command_line) { + base::LaunchProcess(command_line.argv(), base::LaunchOptions()); chrome::AttemptUserExit(); } @@ -273,7 +253,7 @@ void EnsureLocalStateIsWritten() {} class ChromeRestartRequest : public base::SupportsWeakPtr<ChromeRestartRequest> { public: - explicit ChromeRestartRequest(const std::string& command_line); + explicit ChromeRestartRequest(const std::vector<std::string>& argv); ~ChromeRestartRequest(); // Starts the request. @@ -283,22 +263,20 @@ class ChromeRestartRequest // Fires job restart request to session manager. void RestartJob(); - const int pid_; - const std::string command_line_; + const std::vector<std::string> argv_; base::OneShotTimer<ChromeRestartRequest> timer_; DISALLOW_COPY_AND_ASSIGN(ChromeRestartRequest); }; -ChromeRestartRequest::ChromeRestartRequest(const std::string& command_line) - : pid_(getpid()), - command_line_(command_line) {} +ChromeRestartRequest::ChromeRestartRequest(const std::vector<std::string>& argv) + : argv_(argv) {} ChromeRestartRequest::~ChromeRestartRequest() {} void ChromeRestartRequest::Start() { - VLOG(1) << "Requesting a restart with PID " << pid_ - << " and command line: " << command_line_; + VLOG(1) << "Requesting a restart with command line: " + << base::JoinString(argv_, " "); // Session Manager may kill the chrome anytime after this point. // Write exit_cleanly and other stuff to the disk here. @@ -333,19 +311,17 @@ void ChromeRestartRequest::Start() { void ChromeRestartRequest::RestartJob() { DCHECK_CURRENTLY_ON(BrowserThread::UI); - DBusThreadManager::Get()->GetSessionManagerClient()->RestartJob( - pid_, command_line_); + DBusThreadManager::Get()->GetSessionManagerClient()->RestartJob(argv_); delete this; } } // namespace -std::string GetOffTheRecordCommandLine( - const GURL& start_url, - bool is_oobe_completed, - const base::CommandLine& base_command_line, - base::CommandLine* command_line) { +void GetOffTheRecordCommandLine(const GURL& start_url, + bool is_oobe_completed, + const base::CommandLine& base_command_line, + base::CommandLine* command_line) { base::DictionaryValue otr_switches; otr_switches.SetString(switches::kGuestSession, std::string()); otr_switches.SetString(::switches::kIncognito, std::string()); @@ -362,13 +338,10 @@ std::string GetOffTheRecordCommandLine( if (!is_oobe_completed) otr_switches.SetString(switches::kOobeGuestSession, std::string()); - return DeriveCommandLine(start_url, - base_command_line, - otr_switches, - command_line); + DeriveCommandLine(start_url, base_command_line, otr_switches, command_line); } -void RestartChrome(const std::string& command_line) { +void RestartChrome(const base::CommandLine& command_line) { DCHECK_CURRENTLY_ON(BrowserThread::UI); BootTimesRecorder::Get()->set_restart_requested(); @@ -385,7 +358,7 @@ void RestartChrome(const std::string& command_line) { } // ChromeRestartRequest deletes itself after request sent to session manager. - (new ChromeRestartRequest(command_line))->Start(); + (new ChromeRestartRequest(command_line.argv()))->Start(); } } // namespace chromeos diff --git a/chrome/browser/chromeos/login/chrome_restart_request.h b/chrome/browser/chromeos/login/chrome_restart_request.h index 788edb1..576b5fa 100644 --- a/chrome/browser/chromeos/login/chrome_restart_request.h +++ b/chrome/browser/chromeos/login/chrome_restart_request.h @@ -15,16 +15,14 @@ class CommandLine; namespace chromeos { -// Returns the command line string to be used for the OTR process. Also modifies -// the given command line. -std::string GetOffTheRecordCommandLine( - const GURL& start_url, - bool is_oobe_completed, - const base::CommandLine& base_command_line, - base::CommandLine* command_line); +// Determines the |command_line| to be used for the OTR process. +void GetOffTheRecordCommandLine(const GURL& start_url, + bool is_oobe_completed, + const base::CommandLine& base_command_line, + base::CommandLine* command_line); // Request session manager to restart chrome with a new command line. -void RestartChrome(const std::string& command_line); +void RestartChrome(const base::CommandLine& command_line); } // namespace chromeos diff --git a/chrome/browser/chromeos/login/screens/error_screen.cc b/chrome/browser/chromeos/login/screens/error_screen.cc index 3b43664..5b36bad 100644 --- a/chrome/browser/chromeos/login/screens/error_screen.cc +++ b/chrome/browser/chromeos/login/screens/error_screen.cc @@ -229,13 +229,9 @@ void ErrorScreen::OnOffTheRecordAuthSuccess() { const base::CommandLine& browser_command_line = *base::CommandLine::ForCurrentProcess(); base::CommandLine command_line(browser_command_line.GetProgram()); - std::string cmd_line_str = - GetOffTheRecordCommandLine(GURL(), - StartupUtils::IsOobeCompleted(), - browser_command_line, - &command_line); - - RestartChrome(cmd_line_str); + GetOffTheRecordCommandLine(GURL(), StartupUtils::IsOobeCompleted(), + browser_command_line, &command_line); + RestartChrome(command_line); } void ErrorScreen::OnPasswordChangeDetected() { diff --git a/chrome/browser/chromeos/login/session/user_session_manager.cc b/chrome/browser/chromeos/login/session/user_session_manager.cc index 2675739..926d0d51 100644 --- a/chrome/browser/chromeos/login/session/user_session_manager.cc +++ b/chrome/browser/chromeos/login/session/user_session_manager.cc @@ -372,11 +372,8 @@ void UserSessionManager::CompleteGuestSessionLogin(const GURL& start_url) { const base::CommandLine& browser_command_line = *base::CommandLine::ForCurrentProcess(); base::CommandLine command_line(browser_command_line.GetProgram()); - std::string cmd_line_str = - GetOffTheRecordCommandLine(start_url, - StartupUtils::IsOobeCompleted(), - browser_command_line, - &command_line); + GetOffTheRecordCommandLine(start_url, StartupUtils::IsOobeCompleted(), + browser_command_line, &command_line); // This makes sure that Chrome restarts with no per-session flags. The guest // profile will always have empty set of per-session flags. If this is not @@ -391,7 +388,7 @@ void UserSessionManager::CompleteGuestSessionLogin(const GURL& start_url) { chromeos::login::kGuestUserName, base::CommandLine::StringVector()); } - RestartChrome(cmd_line_str); + RestartChrome(command_line); } scoped_refptr<Authenticator> UserSessionManager::CreateAuthenticator( diff --git a/chrome/browser/chromeos/settings/device_settings_test_helper.cc b/chrome/browser/chromeos/settings/device_settings_test_helper.cc index abc8caf..45236d8 100644 --- a/chrome/browser/chromeos/settings/device_settings_test_helper.cc +++ b/chrome/browser/chromeos/settings/device_settings_test_helper.cc @@ -108,8 +108,8 @@ bool DeviceSettingsTestHelper::IsScreenLocked() const { return false; } void DeviceSettingsTestHelper::EmitLoginPromptVisible() {} -void DeviceSettingsTestHelper::RestartJob(int pid, - const std::string& command_line) {} +void DeviceSettingsTestHelper::RestartJob( + const std::vector<std::string>& argv) {} void DeviceSettingsTestHelper::StartSession(const std::string& user_email) {} diff --git a/chrome/browser/chromeos/settings/device_settings_test_helper.h b/chrome/browser/chromeos/settings/device_settings_test_helper.h index 0736342..823c088 100644 --- a/chrome/browser/chromeos/settings/device_settings_test_helper.h +++ b/chrome/browser/chromeos/settings/device_settings_test_helper.h @@ -88,7 +88,7 @@ class DeviceSettingsTestHelper : 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/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; |