Remove hard dependency on bundled python_26 in tests

net/test/spawned_test_server/local_test_server_win.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/test/spawned_test_server/local_test_server.h"
 
 #include <windows.h>
 #include <wincrypt.h>
 
 #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"
 #include "base/process/launch.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/test_timeouts.h"
 #include "base/threading/thread.h"
 #include "base/win/scoped_handle.h"
@@ skipping 55 matching lines @@
     return false;
   }
 
   return true;
 }
 
 }  // namespace
 
 namespace net {
 
+class AddedPythonPath {

[Paweł Hajdan Jr., 2013/08/22 18:11:53]

nit: Rename to ScopedPath, and move to anonymous namespace.

Add a TODO to share this code more broadly.

[Daniel Bratell, 2013/08/23 17:42:30]

Won't ScopedPath feel like a strange name for something that specifically adds a
python distribution to the path. Maybe ScopedPythonPath?

[Daniel Bratell, 2013/08/23 17:43:35]

Ignore this. I wrote this comment before reading your other comments and forgot
to delete it.

+public:

[Paweł Hajdan Jr., 2013/08/22 18:11:53]

nit: Indent +1.

+  AddedPythonPath();
+  ~AddedPythonPath();
+
+private:

[Paweł Hajdan Jr., 2013/08/22 18:11:53]

nit: Indent +1.

+  std::string old_path_;

[Paweł Hajdan Jr., 2013/08/22 18:11:53]

nit: Please comment all the members.

+  scoped_ptr<base::Environment> environment_;
+  bool path_modified_;
+
+  DISALLOW_COPY_AND_ASSIGN(AddedPythonPath);
+};
+
+AddedPythonPath::AddedPythonPath()
+    : environment_(base::Environment::Create()),
+      path_modified_(false) {
+  // Retrieves the old path, adds third_party/python26 to the end of it and
+  // then restores the original path in the destructor.
+
+  ignore_result(environment_->GetVar("PATH", &old_path_));
+
+  std::string new_value = old_path_;
+  if (!new_value.empty())
+    new_value += ";"; // Path seperator.

[Paweł Hajdan Jr., 2013/08/22 18:11:53]

nit: No need for the comment (by the way, we'd use two spaces between comment
and code per the style guide).

+
+  // Add new path to the end so that the system python is used if available.
+  base::FilePath python_path;
+  if (!PathService::Get(base::DIR_SOURCE_ROOT, &python_path))

[Paweł Hajdan Jr., 2013/08/22 18:11:53]

Please propagate this error as said before...

+    return;
+  python_path = python_path.AppendASCII("third_party")

[Paweł Hajdan Jr., 2013/08/22 18:11:53]

Actually it'd be better to pass this as a parameter to the ctor.

+                           .AppendASCII("python_26");

[Paweł Hajdan Jr., 2013/08/22 18:11:53]

nit: This is a bit weird wrapping - if it doesn't fit entirely on the previous
line, could you rather wrap after "=" or "("?

[Daniel Bratell, 2013/08/23 17:42:30]

It was the way the code looked before so I just assumed it was the preferred way
in this code. Takes a while to learn every owners favourite style so I don't
assume that anything is wrong just because it looks strange.

+  new_value += WideToUTF8(python_path.value());
+
+  path_modified_ = environment_->SetVar("PATH", new_value);
+}
+
+AddedPythonPath::~AddedPythonPath() {
+  if (path_modified_) {
+    // Ignoring return values since there is nothing we can do to fix
+    // any problem and leaving a test python at the end of the path is
+    // unlikely to do any harm.
+    if (old_path_.empty())
+      ignore_result(environment_->UnSetVar("PATH"));
+    else
+      ignore_result(environment_->SetVar("PATH", old_path_));
+  }
+}
+
 bool LocalTestServer::LaunchPython(const base::FilePath& testserver_path) {
   CommandLine python_command(CommandLine::NO_PROGRAM);
   if (!GetPythonCommand(&python_command))
     return false;
 
   python_command.AppendArgPath(testserver_path);
   if (!AddCommandLineArguments(&python_command))
     return false;
 
   HANDLE child_read = NULL;
@@ skipping 29 matching lines @@
   if (!job_handle_.IsValid()) {
     LOG(ERROR) << "Could not create JobObject.";
     return false;
   }
 
   if (!base::SetJobObjectAsKillOnJobClose(job_handle_.Get())) {
     LOG(ERROR) << "Could not SetInformationJobObject.";
     return false;
   }
 
+  AddedPythonPath python_path;
   base::LaunchOptions launch_options;
   launch_options.inherit_handles = true;
   launch_options.job_handle = job_handle_.Get();
   if (!base::LaunchProcess(python_command, launch_options, &process_handle_)) {
     LOG(ERROR) << "Failed to launch " << python_command.GetCommandLineString();
     return false;
   }
 
   return true;
 }
@@ skipping 19 matching lines @@
   if (!ParseServerData(server_data)) {
     LOG(ERROR) << "Could not parse server_data: " << server_data;
     return false;
   }
 
   return true;
 }
 
 }  // namespace net