Chromium Code Reviews| Index: net/test/spawned_test_server/local_test_server_win.cc |
| diff --git a/net/test/spawned_test_server/local_test_server_win.cc b/net/test/spawned_test_server/local_test_server_win.cc |
| index fd26483ed477b37698538f1f2b609e7d79b343f2..d15f0c037e8b94b7307692473fac7d975df8907b 100644 |
| --- a/net/test/spawned_test_server/local_test_server_win.cc |
| +++ b/net/test/spawned_test_server/local_test_server_win.cc |
| @@ -10,6 +10,7 @@ |
| #include "base/base_paths.h" |
| #include "base/bind.h" |
| #include "base/command_line.h" |
| +#include "base/environment.h" |
| #include "base/files/file_path.h" |
| #include "base/message_loop/message_loop.h" |
| #include "base/path_service.h" |
| @@ -81,10 +82,69 @@ bool ReadData(HANDLE read_fd, HANDLE write_fd, |
| return true; |
| } |
| +// Class that sets up a temporary path that includes the supplied path |
| +// at the end. |
| +class ScopedPath { |
| + // TODO: By making this more generic we can possibly reused it at |
|
Paweł Hajdan Jr.
2013/08/26 21:39:49
nit: TODO should have a username in parentheses. I
|
| + // other places such as chrome/common/multi_process_lock_unittest.cc. |
| + public: |
| + // Constructor which sets up the environment to include the path to |
| + // third_party/python_26. |
|
Paweł Hajdan Jr.
2013/08/26 21:39:49
nit: Don't mention python_26 here as it's supposed
|
| + ScopedPath(const base::FilePath& path_to_add); |
|
Paweł Hajdan Jr.
2013/08/26 21:39:49
nit: One-parameter ctros should be marked explicit
|
| + |
| + // Destructor that restores the path that were active when the |
| + // object was constructed. |
| + ~ScopedPath(); |
| + |
| + private: |
| + // The PATH environment variable before it was changed or an empty |
| + // string if there was no PATH environment variable. |
| + std::string old_path_; |
| + |
| + // The helper object that allows us to read and set environment |
| + // variables more easily. |
| + scoped_ptr<base::Environment> environment_; |
| + |
| + // A flag saying if we have actually modified the environment. If we |
| + // haven't then we should also not try to "restore" it in the |
|
Paweł Hajdan Jr.
2013/08/26 21:39:49
nit: No need for the "If we haven't" part (up to y
|
| + // destructor. |
| + bool path_modified_; |
| + |
| + // This is a local stack based helper object. Copying it would make |
|
Paweł Hajdan Jr.
2013/08/26 21:39:49
nit: DISALLOW_COPY_AND_ASSIGN is a common enough m
|
| + // no sense and would cause bugs. |
| + DISALLOW_COPY_AND_ASSIGN(ScopedPath); |
| +}; |
| + |
| +ScopedPath::ScopedPath(const base::FilePath& path_to_add) |
| + : environment_(base::Environment::Create()), |
| + path_modified_(false) { |
| + // Retrieves the old PATH, adds the new path to the end of it and |
|
Paweł Hajdan Jr.
2013/08/26 21:39:49
nit: No need for this comment.
|
| + // then restores the original PATH in the destructor. |
| + environment_->GetVar("PATH", &old_path_); |
| + |
| + std::string new_value = old_path_; |
| + if (!new_value.empty()) |
| + new_value += ";"; |
| + |
| + new_value += WideToUTF8(path_to_add.value()); |
| + |
| + path_modified_ = environment_->SetVar("PATH", new_value); |
| +} |
| + |
| +ScopedPath::~ScopedPath() { |
| + if (path_modified_) { |
|
Paweł Hajdan Jr.
2013/08/26 21:39:49
nit: I prefer an early return, like:
if (!path_mo
|
| + if (old_path_.empty()) |
| + environment_->UnSetVar("PATH"); |
| + else |
| + environment_->SetVar("PATH", old_path_); |
| + } |
| +} |
| + |
| } // namespace |
| namespace net { |
| + |
|
Paweł Hajdan Jr.
2013/08/26 21:39:49
nit: No need for this empty line.
|
| bool LocalTestServer::LaunchPython(const base::FilePath& testserver_path) { |
| CommandLine python_command(CommandLine::NO_PROGRAM); |
| if (!GetPythonCommand(&python_command)) |
| @@ -134,6 +194,15 @@ bool LocalTestServer::LaunchPython(const base::FilePath& testserver_path) { |
| return false; |
| } |
| + // Add our internal python to the path so it can be used if there is |
| + // no system python. |
| + base::FilePath python_dir; |
| + if (!PathService::Get(base::DIR_SOURCE_ROOT, &python_dir)) { |
| + LOG(ERROR) << "Could not locate source root directory."; |
| + return false; |
| + } |
| + python_dir = python_dir.AppendASCII("third_party").AppendASCII("python_26"); |
| + ScopedPath python_path(python_dir); |
| base::LaunchOptions launch_options; |
| launch_options.inherit_handles = true; |
| launch_options.job_handle = job_handle_.Get(); |