Mojo: nuke EnvironmentData

With this change, Mojo applications that link against mojo_environment_chromium
do not need to instantiate mojo::Environment. We rely on AtExitManager for all
finalization of singleton objects. This frees us up to use the familiar
base::Singleton and base::LazyInstance for any such state. Tests can use
ShadowingAtExitManager to clean the environment between test runs.

It becomes a link error to use mojo::Environment if you are not linking against
mojo_environment_standalone. I plan to follow this up with a change that buries
mojo::Environment for the case where you are linking against
mojo_environment_standalone. Ideally, this means no one will ever need to think
about mojo::Environment again.

Review URL: https://codereview.chromium.org/281353005

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

19 files changed, 24 insertions, 182 deletions

diff --git a/mojo/android/javatests/mojo_test_case.cc b/mojo/android/javatests/mojo_test_case.cc
index 470598d..f14dfd8 100644
--- a/mojo/android/javatests/mojo_test_case.cc
+++ b/mojo/android/javatests/mojo_test_case.cc
@@ -6,6 +6,7 @@
 
 #include "base/android/jni_android.h"
 #include "base/android/scoped_java_ref.h"
+#include "base/at_exit.h"
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
@@ -17,7 +18,7 @@
 namespace {
 
 struct TestEnvironment {
-  mojo::Environment environment;
+  base::ShadowingAtExitManager at_exit;
   base::MessageLoopForUI message_loop;
 };
 
diff --git a/mojo/apps/js/test/js_to_cpp_unittest.cc b/mojo/apps/js/test/js_to_cpp_unittest.cc
index 74bc7ed..fa34cc6 100644
--- a/mojo/apps/js/test/js_to_cpp_unittest.cc
+++ b/mojo/apps/js/test/js_to_cpp_unittest.cc
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/at_exit.h"
 #include "base/file_util.h"
 #include "base/files/file_path.h"
 #include "base/message_loop/message_loop.h"
@@ -12,7 +13,6 @@
 #include "mojo/apps/js/test/js_to_cpp.mojom.h"
 #include "mojo/common/common_type_converters.h"
 #include "mojo/common/test/test_utils.h"
-#include "mojo/public/cpp/environment/environment.h"
 #include "mojo/public/cpp/system/core.h"
 #include "mojo/public/cpp/system/macros.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -408,7 +408,7 @@ class JsToCppTest : public testing::Test {
   }
 
  private:
-  Environment environment;
+  base::ShadowingAtExitManager at_exit_;
   base::MessageLoop loop;
   base::RunLoop run_loop_;
 
diff --git a/mojo/common/BUILD.gn b/mojo/common/BUILD.gn
index 148835f..538299a 100644
--- a/mojo/common/BUILD.gn
+++ b/mojo/common/BUILD.gn
@@ -13,8 +13,6 @@ component("common") {
     "common_type_converters.h",
     "data_pipe_utils.cc",
     "data_pipe_utils.h",
-    "environment_data.cc",
-    "environment_data.h",
     "handle_watcher.cc",
     "handle_watcher.h",
     "message_pump_mojo.cc",
diff --git a/mojo/common/environment_data.cc b/mojo/common/environment_data.cc
deleted file mode 100644
index b523d27..0000000
--- a/mojo/common/environment_data.cc
+++ /dev/null
@@ -1,51 +0,0 @@
-// Copyright 2014 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 "mojo/common/environment_data.h"
-
-#include "base/stl_util.h"
-
-namespace mojo {
-namespace common {
-
-// static
-EnvironmentData* EnvironmentData::instance_ = NULL;
-
-EnvironmentData::EnvironmentData() {
-  DCHECK(!instance_);
-  instance_ = this;
-}
-
-EnvironmentData::~EnvironmentData() {
-  instance_ = NULL;
-  DataMap data_map;
-  data_map.swap(data_map_);
-  STLDeleteContainerPairSecondPointers(data_map.begin(), data_map.end());
-}
-
-// static
-EnvironmentData* EnvironmentData::GetInstance() {
-  return instance_;
-}
-
-void EnvironmentData::SetData(const void* key, scoped_ptr<Data> data) {
-  Data* old = NULL;
-  {
-    base::AutoLock auto_lock(data_lock_);
-    old = data_map_[key];
-    if (data)
-      data_map_[key] = data.release();
-    else
-      data_map_.erase(key);
-  }
-  delete old;
-}
-
-EnvironmentData::Data* EnvironmentData::GetData(const void* key) {
-  base::AutoLock auto_lock(data_lock_);
-  return data_map_.count(key) > 0 ? data_map_[key] : NULL;
-}
-
-}  // namespace common
-}  // namespace mojo
diff --git a/mojo/common/environment_data.h b/mojo/common/environment_data.h
deleted file mode 100644
index 4440df2..0000000
--- a/mojo/common/environment_data.h
+++ /dev/null
@@ -1,52 +0,0 @@
-// Copyright 2014 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.
-
-#ifndef MOJO_COMMON_ENVIRONMENT_DATA_H_
-#define MOJO_COMMON_ENVIRONMENT_DATA_H_
-
-#include <map>
-
-#include "base/memory/scoped_ptr.h"
-#include "base/synchronization/lock.h"
-#include "mojo/common/mojo_common_export.h"
-
-namespace mojo {
-namespace common {
-
-// EnvironmentData is used to store arbitrary key/value pairs in the
-// environment. The key/value pairs are owned by the Environment and deleted
-// when it is deleted.
-class MOJO_COMMON_EXPORT EnvironmentData {
- public:
-  class MOJO_COMMON_EXPORT Data {
-   public:
-    Data() {}
-    virtual ~Data() {}
-  };
-
-  EnvironmentData();
-  ~EnvironmentData();
-
-  static EnvironmentData* GetInstance();
-
-  void SetData(const void* key, scoped_ptr<Data> data);
-
-  Data* GetData(const void* key);
-
- private:
-  typedef std::map<const void*, Data*> DataMap;
-
-  static EnvironmentData* instance_;
-
-  base::Lock data_lock_;
-
-  DataMap data_map_;
-
-  DISALLOW_COPY_AND_ASSIGN(EnvironmentData);
-};
-
-}  // namespace common
-}  // namespace mojo
-
-#endif  // MOJO_COMMON_ENVIRONMENT_DATA_H_
diff --git a/mojo/common/handle_watcher.cc b/mojo/common/handle_watcher.cc
index 1affa90..4ae86db 100644
--- a/mojo/common/handle_watcher.cc
+++ b/mojo/common/handle_watcher.cc
@@ -9,13 +9,13 @@
 #include "base/atomic_sequence_num.h"
 #include "base/bind.h"
 #include "base/lazy_instance.h"
+#include "base/memory/singleton.h"
 #include "base/memory/weak_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "base/synchronization/lock.h"
 #include "base/threading/thread.h"
 #include "base/time/time.h"
-#include "mojo/common/environment_data.h"
 #include "mojo/common/message_pump_mojo.h"
 #include "mojo/common/message_pump_mojo_handler.h"
 #include "mojo/common/time_helper.h"
@@ -29,8 +29,6 @@ namespace {
 
 const char kWatcherThreadName[] = "handle-watcher-thread";
 
-const char kWatcherThreadManagerKey[] = "watcher-thread-manager";
-
 // TODO(sky): this should be unnecessary once MessageLoop has been refactored.
 MessagePumpMojo* message_pump_mojo = NULL;
 
@@ -176,6 +174,7 @@ class WatcherThreadManager {
   void StopWatching(WatcherID watcher_id);
 
  private:
+  friend struct DefaultSingletonTraits<WatcherThreadManager>;
   WatcherThreadManager();
 
   base::Thread thread_;
@@ -187,29 +186,12 @@ class WatcherThreadManager {
   DISALLOW_COPY_AND_ASSIGN(WatcherThreadManager);
 };
 
-struct WatcherThreadManagerData : EnvironmentData::Data {
-  scoped_ptr<WatcherThreadManager> thread_manager;
-};
-
 WatcherThreadManager::~WatcherThreadManager() {
   thread_.Stop();
 }
 
-static base::LazyInstance<base::Lock> thread_lookup_lock =
-    LAZY_INSTANCE_INITIALIZER;
-
 WatcherThreadManager* WatcherThreadManager::GetInstance() {
-  base::AutoLock auto_lock(thread_lookup_lock.Get());
-  WatcherThreadManagerData* data = static_cast<WatcherThreadManagerData*>(
-      EnvironmentData::GetInstance()->GetData(kWatcherThreadManagerKey));
-  if (!data) {
-    data = new WatcherThreadManagerData;
-    data->thread_manager.reset(new WatcherThreadManager);
-    EnvironmentData::GetInstance()->SetData(
-        kWatcherThreadManagerKey,
-        scoped_ptr<EnvironmentData::Data>(data));
-  }
-  return data->thread_manager.get();
+  return Singleton<WatcherThreadManager>::get();
 }
 
 WatcherID WatcherThreadManager::StartWatching(
diff --git a/mojo/common/handle_watcher_unittest.cc b/mojo/common/handle_watcher_unittest.cc
index 37fde23..39667814 100644
--- a/mojo/common/handle_watcher_unittest.cc
+++ b/mojo/common/handle_watcher_unittest.cc
@@ -6,12 +6,12 @@
 
 #include <string>
 
+#include "base/at_exit.h"
 #include "base/auto_reset.h"
 #include "base/bind.h"
 #include "base/run_loop.h"
 #include "base/test/simple_test_tick_clock.h"
 #include "mojo/common/time_helper.h"
-#include "mojo/public/cpp/environment/environment.h"
 #include "mojo/public/cpp/system/core.h"
 #include "mojo/public/cpp/test_support/test_utils.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -116,7 +116,7 @@ class HandleWatcherTest : public testing::Test {
   base::SimpleTestTickClock tick_clock_;
 
  private:
-  Environment environment_;
+  base::ShadowingAtExitManager at_exit_;
   base::MessageLoop message_loop_;
 
   DISALLOW_COPY_AND_ASSIGN(HandleWatcherTest);
@@ -318,8 +318,7 @@ TEST(HandleWatcherCleanEnvironmentTest, AbortedOnMessageLoopDestruction) {
   bool was_signaled = false;
   MojoResult result = MOJO_RESULT_OK;
 
-  Environment env;
-
+  base::ShadowingAtExitManager at_exit;
   MessagePipe pipe;
   HandleWatcher watcher;
   {
diff --git a/mojo/environment/environment.cc b/mojo/environment/environment.cc
index ab182a6..468f6a3 100644
--- a/mojo/environment/environment.cc
+++ b/mojo/environment/environment.cc
@@ -4,32 +4,16 @@
 
 #include "mojo/public/cpp/environment/environment.h"
 
-#include "mojo/common/environment_data.h"
-
 namespace mojo {
 
-class Environment::Data {
- public:
-  Data();
-  ~Data();
-
- private:
-  common::EnvironmentData data_;
-
-  DISALLOW_COPY_AND_ASSIGN(Data);
-};
-
-Environment::Data::Data() {
-}
-
-Environment::Data::~Data() {
-}
-
-Environment::Environment() : data_(new Environment::Data) {
+// These methods are intentionally not implemented so that there is a link
+// error if someone uses them in a Chromium-environment.
+#if 0
+Environment::Environment() {
 }
 
 Environment::~Environment() {
-  delete data_;
 }
+#endif
 
 }  // namespace mojo
diff --git a/mojo/mojo.gyp b/mojo/mojo.gyp
index aee37a4..5a79230 100644
--- a/mojo/mojo.gyp
+++ b/mojo/mojo.gyp
@@ -338,8 +338,6 @@
         'common/common_type_converters.h',
         'common/data_pipe_utils.cc',
         'common/data_pipe_utils.h',
-        'common/environment_data.cc',
-        'common/environment_data.h',
         'common/handle_watcher.cc',
         'common/handle_watcher.h',
         'common/message_pump_mojo.cc',
@@ -763,6 +761,9 @@
             'libmojo_system_java',
             'mojo_jni_headers',
           ],
+          'defines': [
+            'UNIT_TEST'  # As exported from testing/gtest.gyp:gtest.
+          ],
           'sources': [
             'android/javatests/mojo_test_case.cc',
             'android/javatests/mojo_test_case.h',
diff --git a/mojo/public/cpp/environment/environment.h b/mojo/public/cpp/environment/environment.h
index df293ae..f75e2f7 100644
--- a/mojo/public/cpp/environment/environment.h
+++ b/mojo/public/cpp/environment/environment.h
@@ -16,11 +16,6 @@ class Environment {
   ~Environment();
 
  private:
-  class Data;
-
-  // Environment implementation can use this to store state.
-  Data* data_;
-
   MOJO_DISALLOW_COPY_AND_ASSIGN(Environment);
 };
 
diff --git a/mojo/public/cpp/environment/lib/environment.cc b/mojo/public/cpp/environment/lib/environment.cc
index 1a2e19d..b84aa7c 100644
--- a/mojo/public/cpp/environment/lib/environment.cc
+++ b/mojo/public/cpp/environment/lib/environment.cc
@@ -8,7 +8,7 @@
 
 namespace mojo {
 
-Environment::Environment() : data_(NULL) {
+Environment::Environment() {
   RunLoop::SetUp();
 }
 
diff --git a/mojo/service_manager/service_manager_unittest.cc b/mojo/service_manager/service_manager_unittest.cc
index 77ffab9..2e86769 100644
--- a/mojo/service_manager/service_manager_unittest.cc
+++ b/mojo/service_manager/service_manager_unittest.cc
@@ -2,9 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/at_exit.h"
 #include "base/message_loop/message_loop.h"
 #include "mojo/public/cpp/application/application.h"
-#include "mojo/public/cpp/environment/environment.h"
 #include "mojo/public/interfaces/service_provider/service_provider.mojom.h"
 #include "mojo/service_manager/service_loader.h"
 #include "mojo/service_manager/service_manager.h"
@@ -231,7 +231,7 @@ class ServiceManagerTest : public testing::Test {
   }
 
  protected:
-  mojo::Environment env_;
+  base::ShadowingAtExitManager at_exit_;
   base::MessageLoop loop_;
   TestContext context_;
   scoped_ptr<TestClientImpl> test_client_;
diff --git a/mojo/services/dbus_echo/dbus_echo_service.cc b/mojo/services/dbus_echo/dbus_echo_service.cc
index 560daa6..fcf016c 100644
--- a/mojo/services/dbus_echo/dbus_echo_service.cc
+++ b/mojo/services/dbus_echo/dbus_echo_service.cc
@@ -45,7 +45,6 @@ int main(int argc, char** argv) {
                        false,    // Timestamp
                        false);   // Tick count
 
-  mojo::Environment env;
   mojo::embedder::Init();
 
   base::MessageLoopForIO message_loop;
diff --git a/mojo/services/view_manager/view_manager_connection_unittest.cc b/mojo/services/view_manager/view_manager_connection_unittest.cc
index 8cfc77a..e15fa06 100644
--- a/mojo/services/view_manager/view_manager_connection_unittest.cc
+++ b/mojo/services/view_manager/view_manager_connection_unittest.cc
@@ -5,6 +5,7 @@
 #include <string>
 #include <vector>
 
+#include "base/at_exit.h"
 #include "base/auto_reset.h"
 #include "base/bind.h"
 #include "base/memory/scoped_ptr.h"
@@ -16,7 +17,6 @@
 #include "mojo/public/cpp/application/application.h"
 #include "mojo/public/cpp/application/connect.h"
 #include "mojo/public/cpp/bindings/lib/router.h"
-#include "mojo/public/cpp/environment/environment.h"
 #include "mojo/service_manager/service_manager.h"
 #include "mojo/services/public/cpp/geometry/geometry_type_converters.h"
 #include "mojo/services/public/cpp/view_manager/util.h"
@@ -473,6 +473,7 @@ class ViewManagerConnectionTest : public testing::Test {
     connection2_ = NULL;
   }
 
+  base::ShadowingAtExitManager at_exit_;
   base::MessageLoop loop_;
   shell::ShellTestHelper test_helper_;
 
diff --git a/mojo/shell/android/mojo_main.cc b/mojo/shell/android/mojo_main.cc
index 3faf73f..c265632 100644
--- a/mojo/shell/android/mojo_main.cc
+++ b/mojo/shell/android/mojo_main.cc
@@ -14,7 +14,6 @@
 #include "base/message_loop/message_loop.h"
 #include "jni/MojoMain_jni.h"
 #include "mojo/public/cpp/application/application.h"
-#include "mojo/public/cpp/environment/environment.h"
 #include "mojo/service_manager/service_loader.h"
 #include "mojo/service_manager/service_manager.h"
 #include "mojo/shell/context.h"
@@ -34,10 +33,6 @@ LazyInstance<scoped_ptr<base::MessageLoop> > g_java_message_loop =
 LazyInstance<scoped_ptr<shell::Context> > g_context =
     LAZY_INSTANCE_INITIALIZER;
 
-
-LazyInstance<scoped_ptr<mojo::Environment> > g_env =
-    LAZY_INSTANCE_INITIALIZER;
-
 }  // namespace
 
 static void Init(JNIEnv* env, jclass clazz, jobject context) {
@@ -68,8 +63,6 @@ static void Start(JNIEnv* env, jclass clazz, jobject context, jstring jurl) {
     app_urls.push_back(GURL(base::android::ConvertJavaStringToUTF8(env, jurl)));
 #endif
 
-  g_env.Get().reset(new Environment);
-
   base::android::ScopedJavaGlobalRef<jobject> activity;
   activity.Reset(env, context);
 
diff --git a/mojo/shell/context.cc b/mojo/shell/context.cc
index 608fc53..b47967e 100644
--- a/mojo/shell/context.cc
+++ b/mojo/shell/context.cc
@@ -54,7 +54,7 @@ class Setup {
   DISALLOW_COPY_AND_ASSIGN(Setup);
 };
 
-static base::LazyInstance<Setup> setup = LAZY_INSTANCE_INITIALIZER;
+static base::LazyInstance<Setup>::Leaky setup = LAZY_INSTANCE_INITIALIZER;
 
 }  // namespace
 
diff --git a/mojo/shell/desktop/mojo_main.cc b/mojo/shell/desktop/mojo_main.cc
index 7fc116f..de2d0d4 100644
--- a/mojo/shell/desktop/mojo_main.cc
+++ b/mojo/shell/desktop/mojo_main.cc
@@ -7,7 +7,6 @@
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
-#include "mojo/public/cpp/environment/environment.h"
 #include "mojo/shell/child_process.h"
 #include "mojo/shell/context.h"
 #include "mojo/shell/init.h"
@@ -17,7 +16,6 @@
 
 int main(int argc, char** argv) {
   base::AtExitManager at_exit;
-  mojo::Environment env;
   base::CommandLine::Init(argc, argv);
   mojo::shell::InitializeLogging();
 
diff --git a/mojo/shell/shell_test_base.h b/mojo/shell/shell_test_base.h
index 639a0a7..84864b7 100644
--- a/mojo/shell/shell_test_base.h
+++ b/mojo/shell/shell_test_base.h
@@ -9,7 +9,6 @@
 
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
-#include "mojo/public/cpp/environment/environment.h"
 #include "mojo/public/cpp/system/core.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -44,8 +43,6 @@ class ShellTestBase : public testing::Test {
   Context* shell_context() { return shell_context_.get(); }
 
  private:
-  Environment environment_;
-
   // Only set if/when |InitMojo()| is called.
   scoped_ptr<base::MessageLoop> message_loop_;
   scoped_ptr<Context> shell_context_;
diff --git a/mojo/shell/shell_test_helper.h b/mojo/shell/shell_test_helper.h
index 04a4df5..ef8af89 100644
--- a/mojo/shell/shell_test_helper.h
+++ b/mojo/shell/shell_test_helper.h
@@ -8,7 +8,6 @@
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/run_loop.h"
-#include "mojo/public/cpp/environment/environment.h"
 #include "mojo/public/interfaces/service_provider/service_provider.mojom.h"
 #include "mojo/service_manager/service_loader.h"
 #include "mojo/shell/context.h"
@@ -42,8 +41,6 @@ class ShellTestHelper {
  private:
   class TestServiceProvider;
 
-  Environment environment_;
-
   scoped_ptr<Context> context_;
 
   scoped_ptr<ServiceManager::TestAPI> test_api_;