Reland r54418 - base: Add UnSetEnv function to EnvVarGetter API.

This reverts commit 7ed083f6d2b1b0a09c8bf9470386ba3e38f2feed.

It was reverted due to crbug.com/50663 started failing when I land this patch,
so I'm relanding it now again.

BUG=None
TEST=trybots

TBR=thestig@chromium.org

Review URL: http://codereview.chromium.org/3043018

Review URL: http://codereview.chromium.org/3076020

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@54451 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 47 insertions, 10 deletions

diff --git a/base/env_var.cc b/base/env_var.cc
index d0eaa0c..71d9c53 100644
--- a/base/env_var.cc
+++ b/base/env_var.cc
@@ -44,6 +44,10 @@ class EnvVarGetterImpl : public base::EnvVarGetter {
     return SetEnvImpl(variable_name, new_value);
   }
 
+  virtual bool UnSetEnv(const char* variable_name) {
+    return UnSetEnvImpl(variable_name);
+  }
+
  private:
   bool GetEnvImpl(const char* variable_name, std::string* result) {
 #if defined(OS_POSIX)
@@ -81,6 +85,17 @@ class EnvVarGetterImpl : public base::EnvVarGetter {
                                     ASCIIToWide(new_value).c_str()) != 0;
 #endif
   }
+
+  bool UnSetEnvImpl(const char* variable_name) {
+#if defined(OS_POSIX)
+    // On success, zero is returned.
+    return unsetenv(variable_name) == 0;
+#elif defined(OS_WIN)
+    // On success, a nonzero is returned.
+    return ::SetEnvironmentVariable(ASCIIToWide(variable_name).c_str(),
+                                    NULL) != 0;
+#endif
+  }
 };
 
 }  // namespace
@@ -99,13 +114,13 @@ const char kHome[] = "HOME";
 
 EnvVarGetter::~EnvVarGetter() {}
 
-bool EnvVarGetter::HasEnv(const char* variable_name) {
-  return GetEnv(variable_name, NULL);
-}
-
 // static
 EnvVarGetter* EnvVarGetter::Create() {
   return new EnvVarGetterImpl();
 }
 
+bool EnvVarGetter::HasEnv(const char* variable_name) {
+  return GetEnv(variable_name, NULL);
+}
+
 }  // namespace base
diff --git a/base/env_var.h b/base/env_var.h
index 9622e74..43d3fd6 100644
--- a/base/env_var.h
+++ b/base/env_var.h
@@ -20,11 +20,14 @@ extern const char kHome[];
 
 }  // namespace env_vars
 
-// These are used to derive mocks for unittests.
 class EnvVarGetter {
  public:
   virtual ~EnvVarGetter();
 
+  // Static factory method that returns the implementation that provide the
+  // appropriate platform-specific instance.
+  static EnvVarGetter* Create();
+
   // Gets an environment variable's value and stores it in |result|.
   // Returns false if the key is unset.
   virtual bool GetEnv(const char* variable_name, std::string* result) = 0;
@@ -36,8 +39,8 @@ class EnvVarGetter {
   virtual bool SetEnv(const char* variable_name,
                       const std::string& new_value) = 0;
 
-  // Create an instance of EnvVarGetter
-  static EnvVarGetter* Create();
+  // Returns true on success, otherwise returns false.
+  virtual bool UnSetEnv(const char* variable_name) = 0;
 };
 
 }  // namespace base
diff --git a/base/env_var_unittest.cc b/base/env_var_unittest.cc
index d80d997..f05a9b4 100644
--- a/base/env_var_unittest.cc
+++ b/base/env_var_unittest.cc
@@ -24,9 +24,10 @@ TEST_F(EnvVarTest, HasEnvVar) {
 }
 
 TEST_F(EnvVarTest, SetEnvVar) {
+  scoped_ptr<base::EnvVarGetter> env(base::EnvVarGetter::Create());
+
   const char kFooUpper[] = "FOO";
   const char kFooLower[] = "foo";
-  scoped_ptr<base::EnvVarGetter> env(base::EnvVarGetter::Create());
   EXPECT_TRUE(env->SetEnv(kFooUpper, kFooLower));
 
   // Now verify that the environment has the new variable.
@@ -36,3 +37,21 @@ TEST_F(EnvVarTest, SetEnvVar) {
   EXPECT_TRUE(env->GetEnv(kFooUpper, &var_value));
   EXPECT_EQ(var_value, kFooLower);
 }
+
+TEST_F(EnvVarTest, UnSetEnvVar) {
+  scoped_ptr<base::EnvVarGetter> env(base::EnvVarGetter::Create());
+
+  const char kFooUpper[] = "FOO";
+  const char kFooLower[] = "foo";
+  // First set some environment variable.
+  EXPECT_TRUE(env->SetEnv(kFooUpper, kFooLower));
+
+  // Now verify that the environment has the new variable.
+  EXPECT_TRUE(env->HasEnv(kFooUpper));
+
+  // Finally verify that the environment variable was erased.
+  EXPECT_TRUE(env->UnSetEnv(kFooUpper));
+
+  // And check that the variable has been unset.
+  EXPECT_FALSE(env->HasEnv(kFooUpper));
+}
diff --git a/base/xdg_util_unittest.cc b/base/xdg_util_unittest.cc
index b821921..24671b1 100644
--- a/base/xdg_util_unittest.cc
+++ b/base/xdg_util_unittest.cc
@@ -4,11 +4,10 @@
 
 #include "base/xdg_util.h"
 
+#include "base/env_var.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-#include "base/env_var.h"
-
 using ::testing::_;
 using ::testing::Return;
 using ::testing::SetArgumentPointee;
@@ -20,6 +19,7 @@ class MockEnvVarGetter : public base::EnvVarGetter {
  public:
   MOCK_METHOD2(GetEnv, bool(const char*, std::string* result));
   MOCK_METHOD2(SetEnv, bool(const char*, const std::string& new_value));
+  MOCK_METHOD1(UnSetEnv, bool(const char*));
 };
 
 const char* kGnome = "gnome";