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

10 files changed, 98 insertions, 40 deletions

diff --git a/app/l10n_util_unittest.cc b/app/l10n_util_unittest.cc
index 1975d6c..58d0a98f 100644
--- a/app/l10n_util_unittest.cc
+++ b/app/l10n_util_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -15,6 +15,7 @@
 #include "app/test/data/resource.h"
 #endif
 #include "base/basictypes.h"
+#include "base/env_var.h"
 #include "base/file_util.h"
 #include "base/path_service.h"
 #include "base/stl_util-inl.h"
@@ -138,30 +139,32 @@ TEST_F(L10nUtilTest, GetAppLocale) {
   icu::Locale locale = icu::Locale::getDefault();
 
 #if defined(OS_POSIX) && !defined(OS_CHROMEOS)
+  scoped_ptr<base::EnvVarGetter> env(base::EnvVarGetter::Create());
+
   // Test the support of LANGUAGE environment variable.
   SetICUDefaultLocale("en-US");
-  ::setenv("LANGUAGE", "xx:fr_CA", 1);
+  env->SetEnv("LANGUAGE", "xx:fr_CA");
   EXPECT_EQ("fr", l10n_util::GetApplicationLocale(L""));
 
-  ::setenv("LANGUAGE", "xx:yy:en_gb.utf-8@quot", 1);
+  env->SetEnv("LANGUAGE", "xx:yy:en_gb.utf-8@quot");
   EXPECT_EQ("en-GB", l10n_util::GetApplicationLocale(L""));
 
-  ::setenv("LANGUAGE", "xx:zh-hk", 1);
+  env->SetEnv("LANGUAGE", "xx:zh-hk");
   EXPECT_EQ("zh-TW", l10n_util::GetApplicationLocale(L""));
 
   // We emulate gettext's behavior here, which ignores LANG/LC_MESSAGES/LC_ALL
   // when LANGUAGE is specified. If no language specified in LANGUAGE is valid,
   // then just fallback to the default language, which is en-US for us.
   SetICUDefaultLocale("fr-FR");
-  ::setenv("LANGUAGE", "xx:yy", 1);
+  env->SetEnv("LANGUAGE", "xx:yy");
   EXPECT_EQ("en-US", l10n_util::GetApplicationLocale(L""));
 
-  ::setenv("LANGUAGE", "/fr:zh_CN", 1);
+  env->SetEnv("LANGUAGE", "/fr:zh_CN");
   EXPECT_EQ("zh-CN", l10n_util::GetApplicationLocale(L""));
 
   // Make sure the follow tests won't be affected by LANGUAGE environment
   // variable.
-  ::unsetenv("LANGUAGE");
+  env->UnSetEnv("LANGUAGE");
 #endif  // defined(OS_POSIX) && !defined(OS_CHROMEOS)
 
   SetICUDefaultLocale("en-US");
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";
diff --git a/chrome/browser/locale_tests_uitest.cc b/chrome/browser/locale_tests_uitest.cc
index 54cf202..d5cb994 100644
--- a/chrome/browser/locale_tests_uitest.cc
+++ b/chrome/browser/locale_tests_uitest.cc
@@ -1,9 +1,10 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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 "chrome/test/ui/ui_test.h"
 
+#include "base/env_var.h"
 #include "build/build_config.h"
 
 class LocaleTestsBase : public UITest {
@@ -14,10 +15,11 @@ class LocaleTestsBase : public UITest {
  protected:
   void RestoreLcAllEnvironment() {
 #if defined(OS_LINUX)
+    scoped_ptr<base::EnvVarGetter> env(base::EnvVarGetter::Create());
     if (old_lc_all_) {
-      setenv("LC_ALL", old_lc_all_, 1);
+      env->SetEnv("LC_ALL", old_lc_all_);
     } else {
-      unsetenv("LC_ALL");
+      env->UnSetEnv("LC_ALL");
     }
 #endif
   };
diff --git a/chrome/browser/shell_integration_unittest.cc b/chrome/browser/shell_integration_unittest.cc
index 0514781..dbaa290 100644
--- a/chrome/browser/shell_integration_unittest.cc
+++ b/chrome/browser/shell_integration_unittest.cc
@@ -32,8 +32,7 @@ namespace {
 // Provides mock environment variables values based on a stored map.
 class MockEnvVarGetter : public base::EnvVarGetter {
  public:
-  MockEnvVarGetter() {
-  }
+  MockEnvVarGetter() {}
 
   void Set(const std::string& name, const std::string& value) {
     variables_[name] = value;
@@ -49,7 +48,12 @@ class MockEnvVarGetter : public base::EnvVarGetter {
   }
 
   virtual bool SetEnv(const char* variable_name, const std::string& new_value) {
-    NOTIMPLEMENTED();
+    ADD_FAILURE();
+    return false;
+  }
+
+  virtual bool UnSetEnv(const char* variable_name) {
+    ADD_FAILURE();
     return false;
   }
 
diff --git a/chrome/browser/zygote_host_linux.cc b/chrome/browser/zygote_host_linux.cc
index 75987a4..95db15a 100644
--- a/chrome/browser/zygote_host_linux.cc
+++ b/chrome/browser/zygote_host_linux.cc
@@ -11,6 +11,7 @@
 
 #include "base/command_line.h"
 #include "base/eintr_wrapper.h"
+#include "base/env_var.h"
 #include "base/linux_util.h"
 #include "base/logging.h"
 #include "base/path_service.h"
@@ -18,6 +19,7 @@
 #include "base/process_util.h"
 #include "base/string_number_conversions.h"
 #include "base/string_util.h"
+#include "base/scoped_ptr.h"
 #include "base/unix_domain_socket_posix.h"
 
 #include "chrome/browser/renderer_host/render_sandbox_host_linux.h"
@@ -38,11 +40,12 @@ static void SaveSUIDUnsafeEnvironmentVariables() {
     if (!saved_envvar)
       continue;
 
-    const char* const value = getenv(envvar);
-    if (value)
-      setenv(saved_envvar, value, 1 /* overwrite */);
+    scoped_ptr<base::EnvVarGetter> env(base::EnvVarGetter::Create());
+    std::string value;
+    if (env->GetEnv(envvar, &value))
+      env->SetEnv(saved_envvar, value);
     else
-      unsetenv(saved_envvar);
+      env->UnSetEnv(saved_envvar);
 
     free(saved_envvar);
   }
diff --git a/chrome/plugin/plugin_main_mac.mm b/chrome/plugin/plugin_main_mac.mm
index 56b7c95..119f986 100644
--- a/chrome/plugin/plugin_main_mac.mm
+++ b/chrome/plugin/plugin_main_mac.mm
@@ -1,17 +1,21 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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 "base/chrome_application_mac.h"
+#include "base/env_var.h"
+#include "base/scoped_ptr.h"
 #include "base/string_util.h"
 #include "chrome/common/plugin_carbon_interpose_constants_mac.h"
 #include "chrome/plugin/plugin_interpose_util_mac.h"
 
 #if !defined(__LP64__)
 void TrimInterposeEnvironment() {
-  const char* interpose_list =
-      getenv(plugin_interpose_strings::kDYLDInsertLibrariesKey);
-  if (!interpose_list) {
+  scoped_ptr<base::EnvVarGetter> env(base::EnvVarGetter::Create());
+
+  std::string interpose_list;
+  if (!env->GetEnv(plugin_interpose_strings::kDYLDInsertLibrariesKey,
+                   &interpose_list)) {
     NOTREACHED() << "No interposing libraries set";
     return;
   }
@@ -21,19 +25,19 @@ void TrimInterposeEnvironment() {
   // need to handle are:
   // 1) The whole string is "<kInterposeLibraryPath>", so just clear it, or
   // 2) ":<kInterposeLibraryPath>" is the end of the string, so trim and re-set.
-  int suffix_offset = strlen(interpose_list) -
+  int suffix_offset = strlen(interpose_list.c_str()) -
       strlen(plugin_interpose_strings::kInterposeLibraryPath);
+
   if (suffix_offset == 0 &&
-      strcmp(interpose_list,
+      strcmp(interpose_list.c_str(),
              plugin_interpose_strings::kInterposeLibraryPath) == 0) {
-    unsetenv(plugin_interpose_strings::kDYLDInsertLibrariesKey);
+    env->UnSetEnv(plugin_interpose_strings::kDYLDInsertLibrariesKey);
   } else if (suffix_offset > 0 && interpose_list[suffix_offset - 1] == ':' &&
-             strcmp(interpose_list + suffix_offset,
+             strcmp(interpose_list.c_str() + suffix_offset,
                     plugin_interpose_strings::kInterposeLibraryPath) == 0) {
-    std::string trimmed_list =
-        std::string(interpose_list).substr(0, suffix_offset - 1);
-    setenv(plugin_interpose_strings::kDYLDInsertLibrariesKey,
-           trimmed_list.c_str(), 1);
+    std::string trimmed_list = interpose_list.substr(0, suffix_offset - 1);
+    env->SetEnv(plugin_interpose_strings::kDYLDInsertLibrariesKey,
+                trimmed_list.c_str());
   } else {
     NOTREACHED() << "Missing Carbon interposing library";
   }
diff --git a/net/proxy/proxy_config_service_linux_unittest.cc b/net/proxy/proxy_config_service_linux_unittest.cc
index c43e207..30c3311 100644
--- a/net/proxy/proxy_config_service_linux_unittest.cc
+++ b/net/proxy/proxy_config_service_linux_unittest.cc
@@ -115,7 +115,12 @@ class MockEnvVarGetter : public base::EnvVarGetter {
   }
 
   virtual bool SetEnv(const char* variable_name, const std::string& new_value) {
-    NOTIMPLEMENTED();
+    ADD_FAILURE();
+    return false;
+  }
+
+  virtual bool UnSetEnv(const char* variable_name) {
+    ADD_FAILURE();
     return false;
   }